Skip to content

Conversation

dustymabe
Copy link
Member

In the build-with-buildah world there is no ostree-commit-object so there is no lightweight (i.e. partial import) way to do a db diff. We have to do a full import here unless we somehow start storing RPM NEVRAs in a label or annotation.

Also, we need to pass the version (id) to db diff because the commit checksum in the metadata doesn't seem to work when diffing a build-with-buildah image.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes RPM diffing for builds created with buildah. The changes involve using the build ID instead of the ostree commit hash for rpm-ostree db diff and forcing a full ostree import instead of a partial one, as ostree-commit-object is not available. The changes look correct and align with the description. I've added one suggestion to improve code clarity by renaming variables to more accurately reflect their contents.

In the build-with-buildah world there is no ostree-commit-object
so there is no lightweight (i.e. partial import) way to do a db diff.
We have to do a full import here unless we somehow start storing RPM
NEVRAs in a label or annotation.

Also, we need to pass the version (id) to `db diff` because the
commit checksum in the metadata doesn't seem to work when diffing
a build-with-buildah image.
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

There is both manifest-lock.generated.$arch.json (#4249) and commitmeta.json (#4293) nowadays that are part of the build dir and have the list of RPMs. So we could use those. The problem is that it wouldn't be rpm-ostree doing the diff but us. Not hard obviously, just toil.

Anyway, for now yeah this obviously works.

@jlebon
Copy link
Member

jlebon commented Sep 15, 2025

There is both manifest-lock.generated.$arch.json

How about just doing a git diff on that? Not as legible, but obviously much more lightweight.

@dustymabe
Copy link
Member Author

There is both manifest-lock.generated.$arch.json

These files don't get pulled when you buildfetch, which we could change but..

How about just doing a git diff on that? Not as legible, but obviously much more lightweight.

yeah, I think the legibility thing here is pretty important. You know I hate waste and here I am advocating for pulling a fat container down instead of a text file. That's how important I think the legibility is.

I guess we could implement a formatter here in this code to get us back to parity on the legibility front, but until then I'd like to ship this.

@dustymabe dustymabe merged commit 9ad8441 into coreos:main Sep 16, 2025
5 of 6 checks passed
@dustymabe dustymabe deleted the dusty-rpm-diffs branch September 16, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants