cmd-build-with-buildah: update meta.json with pkg/advisory diffs #4327
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request re-introduces the functionality to update meta.json with package and advisory diffs into cmd-build-with-buildah. The changes are logical and well-implemented, including refactoring cmd-diff to support JSON output and adding more diffing capabilities. My review includes a suggestion to update outdated help text and a minor code style improvement for better maintainability.
dustymabe
force-pushed
the
dusty-add-pkgdiff
branch
2 times, most recently
from
c440444 to
bddff63
Compare
dustymabe
force-pushed
the
dusty-add-pkgdiff
branch
from
bddff63 to
8dcc708
Compare
|
#4253 has merged now so this one should be unblocked. |
dustymabe
force-pushed
the
dusty-add-pkgdiff
branch
from
8dcc708 to
22e11cd
Compare
Turns out this behavior wasn't really required and there was some preference to leave it the other way [1]. This reverts commit 9ffb64f. [1] coreos#4253 (comment)
It's not needed.
dustymabe
force-pushed
the
dusty-add-pkgdiff
branch
from
22e11cd to
b687e51
Compare
This used to be done in the cmd-build path and some things depend on it, so let's add it back here. This also prints a diff so we'll see it in the logs in the pipeline, which can be useful.
dustymabe
force-pushed
the
dusty-add-pkgdiff
branch
from
b687e51 to
6cba969
Compare
Now that we can generate an rpmdiff from commitmeta.json alone [1] we can stop downloading the ostree ociarchive on every build. [1] coreos/coreos-assembler#4327
| --strict Only allow installing locked packages when using lockfiles. | ||
| --parent-build=VERSION This option does nothing and is provided for backwards compatibility. | ||
| --parent-build=VERSION The version that represents the parent to this build. Used for RPM diffs | ||
| that get added to the meta.json |
There was a problem hiding this comment.
Cool, and with this, we now no longer have any "no-op compat" switches!
| # For the logs, print the RPM diff | ||
| /usr/lib/coreos-assembler/cmd-diff \ | ||
| --rpms ${PARENT_BUILD:+--from=$PARENT_BUILD} | ||
|
|
||
| # For meta.json let's record the RPM diff and advisory diff information | ||
| /usr/lib/coreos-assembler/cmd-diff \ | ||
| --rpms-json ${PARENT_BUILD:+--from=$PARENT_BUILD} | | ||
| jq '{"pkgdiff": .pkgdiff, "advisories-diff": .advisories}' > "${tempdir}/diff.json" | ||
| /usr/lib/coreos-assembler/cmd-meta --build="${VERSION}" \ | ||
| --skip-validation --artifact-json "${tempdir}/diff.json" | ||
| # Run a 'dump' now to perform schema validation since we skipped it above. | ||
| /usr/lib/coreos-assembler/cmd-meta --dump --build="${VERSION}" > /dev/null | ||
|
|
There was a problem hiding this comment.
Hmm, I think this probably should instead live in cosa import. That's the command that actually adds a new build. Then cosa build-with-buildah would just proxy the --parent-build switch like it does today for --skip-prune.
And also that way, we can populate meta.json with that info from the very start instead of as a post operation.
There was a problem hiding this comment.
I initially had started working to do just what you propose and I still wish I had a local stash, but I don't see it.
Let me explain why I backed off of that approach.
- I have an effort that will persist container-storage of the supermin VM across runs
- At one point I was going to have cosa import run inside the supermin VM too
- Prior to 77dbc4e
cosa diffwas going to be heavyweight (i.e. have to import the ociarchive of the parent in order to generate the diff) - I didn't want to do the heavyweight ociarchive import inside the supermin VM
- The only way to have the ociarchive import happen outside the supermin VM (if
cosa importwas going to happen inside the supermin VM, was to run it in build-with-buildah.
Fast forwarding to today. The diff should be lightweight regardless of where it is run so we should be able to do it in cosa import no problem.
This used to be done in the cmd-build path and some things depend on it, so let's add it back here. This also prints a diff so we'll see it in the logs in the pipeline, which can be useful.
Requires some changes in #4253