fix: make ci/container/container-run.sh work when the checkout is a git worktree - #11183
Closed
mbjorkqvist wants to merge 5 commits into
Closed
fix: make ci/container/container-run.sh work when the checkout is a git worktree#11183mbjorkqvist wants to merge 5 commits into
ci/container/container-run.sh work when the checkout is a git worktree#11183mbjorkqvist wants to merge 5 commits into
Conversation
In a linked git worktree `.git` is a file pointing at an admin directory
under the main checkout's `.git`, an absolute path outside the checkout.
`container-run.sh` mounted only the checkout, so that pointer dangled and
every `git` invocation inside the container failed. Mount the main
checkout's `.git` at the same absolute path so it resolves.
Inside the container all worktrees look prunable, because their
back-pointers name host paths that are not mounted. Reading is
unaffected, but `git gc` -- which git runs automatically after
commit/fetch -- prunes worktrees and would delete the host's admin
directories, so turn automatic gc off for this case.
Also stop the affected scripts from failing silently. `cd ""` returns 0
in bash and a command substitution inside `${X:-...}` does not trip
errexit, so `ci/scripts/rust-lint.sh` linted nothing and still exited 0;
`export X="$(...)"` swallows the substitution's exit status the same way.
Assign first, then `cd`/`export`, so `set -e` fires.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes local dev-container behavior when the repo is checked out as a linked Git worktree, ensuring Git operations inside the container work correctly and that Git failures in helper scripts are no longer silently ignored.
Changes:
- Prevent silent no-op behavior in scripts by assigning
REPO_ROOT/ROOT_DIRfirst (soset -ecorrectly stops on failinggit rev-parse) and thencd/export. - In
container-run.sh, detect linked-worktree checkouts and additionally bind-mount the main checkout’s Git admin directory at its absolute path so.gitfile pointers resolve inside the container. - Disable automatic
git gcinside the container for the linked-worktree case to avoid pruning/deleting host worktree admin directories.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ci/scripts/rust-lint.sh | Makes git-root detection fail fast (no silent “lint nothing, exit 0” behavior). |
| ci/container/get-image-tag.sh | Ensures git rev-parse failures are not masked before changing directories. |
| ci/container/container-run.sh | Adds extra bind-mount for linked-worktree Git admin dir and disables auto GC in that scenario. |
| ci/container/build-ic.sh | Ensures export VAR="$(...)" no longer masks failing git rev-parse/git rev-parse HEAD. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Setting `gc.auto=0` stops automatic gc from pruning the host's worktree admin directories, but leaves two gaps: an explicit `git gc` inside the container still prunes them, and no object maintenance ever happens for the host repository. `gc.worktreePruneExpire=never` disables exactly the dangerous step, in both the automatic and the explicit path, and leaves the rest of gc alone. The risk is not theoretical: gc prunes a worktree whose index has not been touched within `gc.worktreePruneExpire` (three months by default), and a machine that has accumulated worktrees typically has several of those. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mounting the admin directory at its host path only helps while the checkout's `.git` names that path absolutely. With `worktree.useRelativePaths` (git >= 2.48) it holds a path relative to the checkout instead, which inside the container resolves against the mount point of the checkout and dangles wherever the admin directory is mounted. There is nothing to mount that would fix it, so report it, point at `git worktree repair`, and refuse -- rather than starting a container in which git silently does not work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A linked worktree is the case that motivated the mount, but it is not the only one that reaches it: a `--separate-git-dir` clone and a submodule also have a `.git` file pointing outside the checkout, and they are equally broken without it -- so say that, instead of announcing "checkout is a git worktree" for all three. Also note what the mount gives away: for a worktree the container gets read-write access to the main checkout's whole git directory, not just to this checkout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`cd ""` does not move the shell, and this script is normally started with the repository root as the working directory, so the failure is not that nothing gets linted -- it is that a failure to locate the repository goes unnoticed and whatever directory the caller happened to be in gets linted in its place, with a successful exit either way. The fix is unchanged; only the comment was overstating the symptom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mbjorkqvist
changed the title
ci/container/container-run.sh work when the checkout is a git worktree
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
ci/scripts/rust-lint.sh:11
- In
rust-lint.sh,REPO_ROOTis still computed via${CI_PROJECT_DIR:-$(git rev-parse --show-toplevel)}. The PR description notes that command substitutions inside${VAR:-...}don’t reliably tripset -e, so a failinggit rev-parsecan still leaveREPO_ROOTempty andcd "$REPO_ROOT"becoming a silentcd "".
To make the failure reliably fatal, avoid $(...) inside ${...} here (e.g., branch on CI_PROJECT_DIR and run git rev-parse in a separate simple command when needed).
REPO_ROOT="${CI_PROJECT_DIR:-$(git rev-parse --show-toplevel)}"
cd "$REPO_ROOT"
Collaborator
Contributor
Author
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.
Running
./ci/container/container-run.shfrom a linked git worktree gave a container in which git did not work at all, and the ci scripts that locate the repository with git did not notice.The container could not use git. In a linked worktree
.gitis a file pointing at an admin directory under the main checkout's.git— an absolute path outside the checkout. Only the checkout was mounted, so that pointer dangled and everygitcall inside the container failed.container-run.shnow also mounts that directory at the same absolute path, which fixes every git-dependent script at once rather than one script at a time. Plain clones are unaffected. A worktree whose pointer is relative (git'sworktree.useRelativePaths) cannot be made to resolve this way, so that case is reported and refused instead of silently handed over.That mount makes all worktrees look prunable inside the container, since their back-pointers name host paths that are not mounted. Reading is unaffected, but
git gcprunes worktrees it considers stale as part of its job, and would delete the host's admin directories — including those of worktrees you are not even using here. Worktree pruning is therefore switched off for the container, rather than gc as a whole: the repository still gets its object maintenance, and an explicitgit gcis safe too.The failures were silent.
cd ""returns 0 in bash and does not move the shell, and a command substitution inside${X:-...}does not trip errexit, so a script whosegit rev-parse --show-toplevelfailed carried on against whatever directory it happened to be in and still exited 0;export X="$(...)"swallows the substitution's exit status the same way. The affected scripts now assign first and thencd/export, soset -efires. A sweep found no other instances.Only local developer tooling changes — CI sets
CI_PROJECT_DIRand never hit this. Note that the VS Code dev container (.devcontainer/devcontainer.json) is still affected: its mounts are static, so it cannot pick up the main checkout's git directory the way this script does.