Open
Conversation
Detect bare repos (both core.bare=true with .git/ directory and true bare clones without .git) in find_nixpkgs_root() by falling back to git cat-file when the filesystem walk finds no nixos/release.nix. Fix resolve_git_dir() to fall back to git rev-parse --git-dir when no .git file or directory exists. Guard the wip command with an early exit in bare repos, since there is no working tree to diff against.
Move find_nixpkgs_root, is_bare_repository, resolve_git_dir, fetch_refs, and locked_open out of buildenv.py and review.py into a dedicated nixpkgs.py module. Rename tests/test_bare.py to tests/test_nixpkgs.py to match.
Add a session-scoped _isolate_git fixture that sets GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM to /dev/null, preventing settings like commit.gpgsign or gpg.format from breaking test commits. Also sets git identity vars centrally, removing the per-command GIT_ENV dict from test_nixpkgs.py.
Nothing imports from test modules, so the privacy convention doesn't apply. Also hoist the shutil import to the top of the file.
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.
Supporting bare nixpkgs repositories
The problem
I keep my nixpkgs checkout as a bare repo with worktrees:
I learned this git workflow for large repos with many workstreams (like nixpkgs)! from @Ericson2314. You clone once, set
core.bare = true, and thengit worktree addfor each branch you're working on. No files are checked out at the root; the worktrees live as siblingdirectories alongside
.git/. It's pretty awesome.Today, when I
cd ~/nixpkgsand runnixpkgs-review pr 12345, it dies immediately with "Has to be executed from nixpkgs repository". The tool can't tell it's inside nixpkgs because there's nonixos/release.nixon disk at the root level -- it only exists inside the worktrees.Running from inside a worktree (
cd ~/nixpkgs/master && nixpkgs-review pr ...) works fine, but it's annoying to have to pick an arbitrary worktree just to launch a review that creates its own worktree anyway.What was broken
Two functions assumed a working tree exists:
find_nixpkgs_root()walks up from CWD looking fornixos/release.nixon the filesystem. In a bare repo there are no checked-out files, so it walks all the way to/and gives up.resolve_git_dir()looks for a.gitfile or directory. My layout has.git/as a directory so this one actually works for me, but a "true" bare clone (git clone --bare) has no.gitentry at all -- the git database (HEAD, objects, refs) lives directly in the directory. That case would also fail.What this PR does
find_nixpkgs_root()now falls back to git when the filesystem walk fails. It asksgit rev-parse --is-bare-repositorywhether we're in a bare repo, then checksgit cat-file -e HEAD:nixos/release.nixto confirm the repo actually contains nixpkgs (not just any bare repo). If both pass, it returns CWD as the root.resolve_git_dir()gets a similar fallback: when there's no.giton disk, it triesgit rev-parse --git-dir, which works in true bare clones where the git database is the directory itself.wipis guarded with an early exit in bare repos. Thewipcommand captures the dirty working tree viagit diff, but a bare repo has no working tree to diff. Rather than letting it silently produce an empty diff and exit, it now tells you to useprorrevinstead.The non-bare case is unchanged -- the filesystem walk still runs first, so there's no subprocess overhead for the common case.
Refactoring
Along the way,
find_nixpkgs_root,resolve_git_dir,fetch_refs, and friends were extracted frombuildenv.pyandreview.pyinto a newnixpkgs.pymodule, since they're all about locating and interacting with the nixpkgs git repo rather than about the build environment or the review workflow.The test suite also got a session-scoped fixture that sets
GIT_CONFIG_GLOBALandGIT_CONFIG_SYSTEMto/dev/null, so the user's git config (things likecommit.gpgsign = true) can't break test commits. Previously -- at least on my system -- this required passing those env vars on the command line every time I ranpytest.AI-assisted
As you likely can tell, my friend Claude helped with this implementation. I've reviewed it and think it's pretty slop-free, but please let me know your own tastes on the matter.