Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mock/py/mock-hermetic-repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def prepare_image(image_specification, bootstrap_data, outputdir):
Store the tarball into the same directory where the RPMs are
"""
pull_cmd = ["podman", "pull"]
if "pull_digest" in bootstrap_data:
if bootstrap_data.get("pull_digest"):
image_specification += "@" + bootstrap_data["pull_digest"]
if "architecture" in bootstrap_data:
pull_cmd += ["--arch", bootstrap_data["architecture"]]
Expand Down
6 changes: 4 additions & 2 deletions mock/py/mockbuild/podman.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,15 @@ def inspect_hermetic_metadata(self):
"""
Get the image metadata needed for the subsequent hermetic build.
"""
get_query = '{"pull_digest": "{{ .Digest }}", "id": "{{.Id}}", "architecture": "{{ .Architecture }}"}'
get_query = '{"id": "{{.Id}}", "architecture": "{{ .Architecture }}"}'
getLog().info("Reading image %s from %s", get_query, self.image)
cmd = ["podman", "image", "inspect", "--format", get_query, self.image]
Comment on lines +254 to 256

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The log message on line 255 is confusing because it includes the raw query format string, making the log output difficult to interpret. Since get_query is only used once after this change, you can improve readability by inlining it into the podman command and writing a more descriptive log message. Additionally, prefer lazy '%s' formatting for log entries, as demonstrated in the suggestion, over f-strings or direct string concatenation for better performance and consistency. This aligns with the style guide's emphasis on readability.

Suggested change
get_query = '{"id": "{{.Id}}", "architecture": "{{ .Architecture }}"}'
getLog().info("Reading image %s from %s", get_query, self.image)
cmd = ["podman", "image", "inspect", "--format", get_query, self.image]
getLog().info("Reading image metadata from %s", self.image)
References
  1. The style guide states in line 4: 'Readability: Code should be easy to understand for all team members.' The current log message is not easily understandable and thus violates this principle. (link)
  2. Prefer lazy '%s' formatting for log entries over f-strings.

res = subprocess.run(cmd, env=self.buildroot.env,
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
check=True)
return json.loads(res.stdout.decode("utf-8").strip())
res = json.loads(res.stdout.decode("utf-8").strip())
res['pull_digest'] = None
return res


def __repr__(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Image has one Digest and many RepoDigests. So, we can hit situation when these
two values don't match and podman will refuse to load such tarball later with
error like described [here](https://github.com/containers/podman/issues/27323).

We should move to different mechanism (probably skopeo) in the near future.
Loading