cmd-build-with-buildah: enhance version handling, add yumrepos support, add autolocking #4249
Conversation
This drops a cmdlib.sh Python-baked function.
The `variant` variable is set by `prepare_build`, which we're purposely not calling here.
This is for coreos/fedora-coreos-tracker#1996. I initially wanted to just add another `LABEL` to our Containerfile and hook that up to another build arg, but the problem is that our Containerfile is shared with RHCOS/SCOS and there's no way to make `LABEL` directives conditional. I opened coreos/rpm-ostree#5454 which will fix this but for now to not block on this, let's just slap it on from the cosa side. Once it's folded back into the build process proper, then we can rever this. But the label not existing shouldn't really affect the majority of developers anyway.
I think it's still useful to have a `dev` marker in the version string for development builds. The versionary script recently learned a new `--dev` switch which will spit out a development version string. Let's tweak the logic so that: - by default, if `--versionary` is not passed, we call `versionary --dev` to get a development version string - if `--versionary` is passed, we _don't_ pass `--dev` to get a production version string - if `--version` is passed, it always wins This is a bit confusing of course. E.g. ideally the `--versionary` switch would be called `--prod-versionary` or something. But the intent is to replicate the UX that exists today with `cosa build` so that this becomes a drop-in replacement for it in the pipeline. Once we cut over and don't need to support both, we could tweak things.
![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 enhances version handling in cmd-build-with-buildah and adds support for yumrepos. The changes are generally good, introducing more explicit control over versioning and simplifying build existence checks. My review focuses on improving the robustness of the new logic, particularly around handling configuration values and file dependencies. I've suggested modifications to prevent silent failures with invalid configuration and to provide clearer error messages when required files are missing.
jlebon
force-pushed
the
pr/build-with-buildah-tweaks
branch
from
9ddd584 to
9908fb8
Compare
jlebon
changed the title
| # Apply autolock from another build for this version (or for another version if | ||
| # explicitly provided via --autolock) if no base lockfile exists. | ||
| lockfile="manifest-lock.${arch}.json" | ||
| if [ ! -f "src/config/${lockfile}" ] && { [ -n "${VERSION}" ] || [ -n "${AUTOLOCK_VERSION}" ]; }; then |
There was a problem hiding this comment.
Another way to do this that avoids the awkward {} but not sure it's better:
| if [ ! -f "src/config/${lockfile}" ] && { [ -n "${VERSION}" ] || [ -n "${AUTOLOCK_VERSION}" ]; }; then | |
| if [ ! -f "src/config/${lockfile}" ] && [ -n "${VERSION:-}${AUTOLOCK_VERSION:-}" ]; then |
There was a problem hiding this comment.
Agree it's awkward. Not sure it's better either. :) That one is mostly copy/pasted from cmd-build so I'll keep it as is for consistency.
| if [ -z "${VERSIONARY}" ]; then | ||
| VERSION=$(src/config/versionary --dev) | ||
| else | ||
| VERSION=$(src/config/versionary) | ||
| fi |
There was a problem hiding this comment.
I wonder if another option here in the future is just drop the versionary switch at all and add a --prod or --pipeline switch and we'd just add that when we run in the pipeline similar to how we add --versionary in the pipeline today.
There was a problem hiding this comment.
Yeah, absolutely. I mentioned this in the commit message. The --versionary switch is awkwardly named now, but trying to keep it "pipeline compatible" to minimize changes that would be needed there. But once we cut over, we could simplify things, yeah.
If there's a yumrepos source, hook that up to the build so that it has access to the repo contents and the contentset. Also just mount in `/etc/pki/ca-trust` because the repos may require a cert.
This is equivalent to `cosa build` which also runs `cosa prune` at the end.
This matches `cosa build`, but also this is required for autolocking, which makes use of this.
This matches `cosa build` support for it and is actively used by both RHCOS and FCOS (rawhide). Long-term I think this will become obsolete by hermetic builds in Konflux which will require us to always have in-tree lockfiles.
jlebon
force-pushed
the
pr/build-with-buildah-tweaks
branch
from
9908fb8 to
d2b49da
Compare
See individual commit messages