Move the build environment under package/, behind an entry point - #1222
Closed
yiyi-wang-rv wants to merge 1 commit into
Closed
yiyi-wang-rv wants to merge 1 commit into
yiyi-wang-rv wants to merge 1 commit into
Conversation
The build RECIPE (package/debian/build-package) has always lived under package/,
versioned with the source it builds. The build ENVIRONMENT did not: it was
.github/workflows/Dockerfile, reachable only from a GitHub workflow. And the
knowledge of how to DRIVE it -- which LLVM version pairs with which Ubuntu
release, which build args the Dockerfile needs, how to map the calling user into
the container, where to mount the checkout -- was written down nowhere
executable, only spread across two composite actions.
Moving CI to the internal forge deletes .github/ outright, which ends that
arrangement whether or not we plan for it. So the environment moves next to the
recipe, behind two entry points a forge workflow, the release repository and a
developer's shell can all call the same way:
package/build-env/build-deb noble
package/build-env/test-deb noble
They share common.sh, which holds the distro table: the LLVM version that pairs
with each Ubuntu release now lives in one place rather than being restated by
every caller, so adding a release is one edit here and one in the CI matrix.
package/debian/* is untouched and the produced deb is the same. The composite
actions keep working -- only their `dockerfile:` default moves -- so the GitHub
path is unchanged until the migration deletes it.
Two real bugs turned up while writing this down, both fixed here.
The checkout must not mount at /opt/workspace. dpkg-buildpackage writes its
output to the PARENT of the directory it builds in, and build-package moves it
back from there; with the checkout at /opt/workspace that parent is /, which the
non-root build user cannot write. The GitHub path gets away with it by accident
-- it checks out into k-<distro>/ and mounts the parent, so .. lands on the
runner workspace. Anything mounting the checkout directly, which is the obvious
thing to do and what rv-release currently does, fails from inside
dpkg-buildpackage naming neither the mount nor the reason. The Dockerfile now
creates /opt/workspace owned by the build user and build-deb mounts the checkout
at /opt/workspace/source.
Passing the caller's account NAME into the image collides with the base image.
ubuntu:noble ships an `ubuntu` account at uid/gid 1000 and `users` is gid 100,
so groupadd/useradd fails several minutes into the build with a message that
never mentions a build arg. Only the ids ever mattered -- they exist so files
written to the bind mount come back owned by the caller -- so the Dockerfile
names the account itself and callers pass ids alone, with -o so the ids can be
shared rather than deleting a system account something else uses.
Verified: the jammy image builds as uid 1000 / gid 100, which fails on master;
inside it the account resolves to a real home, lit is on PATH, and both the bind
mount and its PARENT are writable, which is the fix above. Argument handling and
engine selection checked against a stub engine, from a subdirectory as well as
the root. shellcheck -x clean.
Not verified: a full deb build, which needs submodules and a long C++ compile.
package/debian/* is unchanged, so what this commit alters is the environment and
the invocation, and those are covered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The build recipe (
package/debian/build-package) has always lived underpackage/, versionedwith the source it builds. The build environment did not — it was
.github/workflows/Dockerfile, reachable only from a GitHub workflow. And the knowledge of howto drive it was written down nowhere executable at all: which LLVM version pairs with which
Ubuntu release, which build args the Dockerfile needs, how to map the calling user into the
container, where to mount the checkout. That lived partly in two composite actions and partly in
the head of whoever wrote the next caller.
Moving CI to the internal forge deletes
.github/outright, which ends that arrangement whetheror not we plan for it. So rather than move the same shape sideways, this puts the environment
next to the recipe and gives it two entry points that a forge workflow, the release repository,
and a developer's shell can all call identically:
What moved
.github/workflows/Dockerfilepackage/build-env/Dockerfile.github/actions/{with-docker,test-package}package/build-env/build-debpackage/build-env/test-debpackage/build-env/common.shBoth composite actions keep working — their
dockerfile:default is repointed, and nothing elseabout the GitHub path changes.
package/debian/build-packageandtest-packageare untouched.Two real bugs found on the way, both fixed here
1. The checkout must not mount at
/opt/workspace.dpkg-buildpackagewrites its output tothe parent of the directory it builds in, and
build-packagethen moves it back. Mount thecheckout at
/opt/workspaceand that parent is/, which the non-root build user cannot write.The existing GitHub path gets away with it by accident: it checks out into a
k-<distro>/subdirectory and mounts the parent, so
..lands on the runner workspace. Anything that mountsthe checkout directly — which is the obvious thing to do, and what the release repository
currently does — fails from inside
dpkg-buildpackage, naming neither the mount nor the reason.Reproduced both ways, and fixed by making the layout explicit: the
Dockerfilecreates/opt/workspaceowned by the build user, andbuild-debmounts the checkout at/opt/workspace/source.2. Passing the caller's account name into the image collides with the base image.
ubuntu:nobleships anubuntuaccount at uid/gid 1000, andusers(gid 100) is the primarygroup of many developer accounts. Either one makes the
Dockerfile'sgroupadd/useraddfail —several minutes into the build, with a message that never mentions a build arg.
Only the ids ever mattered: they exist so files written to the bind mount come back owned by
the caller. So the
Dockerfilenow names the account itself (builder), callers pass ids alone,and
-olets those ids be shared with an existing account rather than deleting a system accountsomething else is using.
with-dockerpasses ids too, and runs the container with a numeric--user.This is why CI builds today but the same Dockerfile fails on a developer machine: the runner
accounts happen to sit outside the ranges that collide.
Not in this PR
.github/. That belongs to the forge migration. This only makes the migrationpossible without taking the deb build down with it.
package/debian/*is untouched; the produced deb is the same./home/user/.tmp-mavenoddity in the Dockerfile — itADDs to a literal/home/user/that is nobody's home. It works by side effect (the warm cache lands in
~/.m2), and untanglingit is a separate change from moving the file.
How to verify
CI exercises the same Dockerfile through the unchanged composite actions, so a green
test.ymlplus
clang-format-check.ymlcovers the move itself.What I checked locally: the jammy image builds as uid 1000 / gid 100 — the collision that fails
on
master— and inside it the account resolves to a real home,litis onPATH, and boththe bind mount and its parent are writable. Argument handling and engine selection were checked
against a stub engine, run from a subdirectory as well as the repo root.
shellcheck -xclean.Not checked locally: a full deb build, which needs submodules and a long C++ compile.
package/debian/*is unchanged, so what this PR alters is the environment and the invocation.🤖 Generated with Claude Code