Skip to content

git: fix locking issue in pull_checkout_branch#51854

Merged
becker33 merged 1 commit intospack:developfrom
alecbcs:fix/git-branch-pull
Jan 30, 2026
Merged

git: fix locking issue in pull_checkout_branch#51854
becker33 merged 1 commit intospack:developfrom
alecbcs:fix/git-branch-pull

Conversation

@alecbcs
Copy link
Copy Markdown
Member

@alecbcs alecbcs commented Jan 15, 2026

After #51779, I started experiencing git locking issues like the following when attempting to update my packages repository.

> spack repo update
remote: Enumerating objects: 214, done.
remote: Counting objects: 100% (168/168), done.
remote: Compressing objects: 100% (70/70), done.
remote: Total 127 (delta 46), reused 84 (delta 18), pack-reused 0 (from 0)
error: cannot lock ref 'refs/remotes/upstream/develop': unable to resolve reference 'refs/remotes/upstream/develop'

Previously we'd used

git fetch <remote> <branch>

which is shorthand for:

git fetch <remote> refs/heads/<branch>:refs/remotes/<remote>/<branch>

this was modified to the following to support RHEL7 (although we're unsure why this started creating the reference on old versions of git?)

git fetch <remote> <branch>:refs/remotes/<remote>/<branch>

This syntax bypasses a users configured refspecs and says "take whatever <remote> gives me for <branch> and write it directly to refs/remotes/<remote>/<branch> which causes locking errors with some user / repository git configurations. After updating the command syntax to be the explicit form of what we'd been using before I am no longer experiencing any issues.

@alecbcs alecbcs requested review from becker33 and haampie and removed request for becker33 January 15, 2026 19:21
@haampie
Copy link
Copy Markdown
Member

haampie commented Jan 15, 2026

Can you confirm it works with RHEL 7?

@alecbcs
Copy link
Copy Markdown
Member Author

alecbcs commented Jan 15, 2026

@haampie how did you test your previous PR with RHEL7? We no longer have any RHEL7 machines at LLNL, and the available docker containers for centos7 don't have a preinstalled git. (And the package repos don't exist anymore.)

@haampie
Copy link
Copy Markdown
Member

haampie commented Jan 15, 2026

This one might have it: ghcr.io/spack/centos7

@alecbcs
Copy link
Copy Markdown
Member Author

alecbcs commented Jan 15, 2026

Yep! Thanks for the suggestion @haampie. This works with ghcr.io/spack/centos7:

[root@38ee19cd0872 ~]# git --version
git version 1.8.3.1

[root@38ee19cd0872 ~]# git clone --depth=2 --branch=fix/git-branch-pull https://github.com/alecbcs/spack.git
Cloning into 'spack'...
remote: Enumerating objects: 2359, done.
remote: Counting objects: 100% (2359/2359), done.
remote: Compressing objects: 100% (1592/1592), done.
remote: Total 2359 (delta 311), reused 1319 (delta 241), pack-reused 0 (from 0)
Receiving objects: 100% (2359/2359), 5.35 MiB | 0 bytes/s, done.
Resolving deltas: 100% (311/311), done.

[root@949de0168f12 ~]# ./spack/bin/spack repo update
remote: Enumerating objects: 19877, done.
remote: Counting objects: 100% (19877/19877), done.
remote: Compressing objects: 100% (10649/10649), done.
remote: Total 19877 (delta 1364), reused 13945 (delta 1235), pack-reused 0 (from 0)
Receiving objects: 100% (19877/19877), 10.17 MiB | 17.64 MiB/s, done.
Resolving deltas: 100% (1364/1364), done.
==> builtin: Updated sucessfully.

As a sanity check I also checked the original implementation prior to #51779 failed as expected when rebased against the same base as this PR.

[root@b2929718ea79 ~]# git clone --depth=2 --branch=test/git-branch-before-rhel7-fix https://github.com/alecbcs/spack.git
Cloning into 'spack'...
remote: Enumerating objects: 2359, done.
remote: Counting objects: 100% (2359/2359), done.
remote: Compressing objects: 100% (1593/1593), done.
remote: Total 2359 (delta 311), reused 1318 (delta 240), pack-reused 0 (from 0)
Receiving objects: 100% (2359/2359), 5.35 MiB | 0 bytes/s, done.
Resolving deltas: 100% (311/311), done.
[root@b2929718ea79 ~]# ./spack/bin/spack repo update
remote: Enumerating objects: 19877, done.
remote: Counting objects: 100% (19877/19877), done.
remote: Compressing objects: 100% (10649/10649), done.
remote: Total 19877 (delta 1364), reused 13945 (delta 1235), pack-reused 0 (from 0)
Receiving objects: 100% (19877/19877), 10.17 MiB | 16.74 MiB/s, done.
Resolving deltas: 100% (1364/1364), done.
error: pathspec 'develop' did not match any file(s) known to git.
==> Error: Failed to update repository builtin

@haampie
Copy link
Copy Markdown
Member

haampie commented Jan 16, 2026 •
edited
Loading

OK, curious, I thought that git fetch remote x:y meant x as a remote name, y as a local name.

Do you know why tests did not catch this? Are we not testing spack repo update? Feels like this should've been caught. Can you add a test accordingly? (Or is it related to the tests using a local path git repo? In that case I think file:// should be used consistently to get the same effect.)

@alecbcs alecbcs added v1.0.3 PRs to backport for v1.0.3 v1.1.1 labels Jan 17, 2026
@alecbcs
Copy link
Copy Markdown
Member Author

alecbcs commented Jan 26, 2026 •
edited
Loading

The locking issue appears to be a bug internal in git that behaves differently across platforms, making it difficult to reproduce reliably within Spack. For example, I can consistently trigger the bug on a macOS Tahoe machine, but it occurs only sporadically (1 in 100ish executions) on a macOS Sequoia laptop. (Even when both systems use the same Spack-built version of git.)

The issue also appears to depend on the timing between the fetch and checkout commands. Since Spack executes these commands back-to-back faster than a human, it seems like something is mid-cleanup and causes the current implementation to fail. Whereas if you add a second or two delay it succeeds.

My guess is that it's a thread race that's been made worse by filesystem speed / driver improvements in the OS.)

Overall, I agree that we should in general do a better job testing spack repo update, but I don't think it would have helped us find this specific bug. I'd like to tackle improved unit tests in another PR and get this one merged as a hotfix before the patch release comes out.

@becker33 becker33 added v1.1.2 and removed v1.1.1 labels Jan 27, 2026
@becker33 becker33 merged commit 53aab9f into spack:develop Jan 30, 2026
31 of 32 checks passed
@becker33 becker33 mentioned this pull request Feb 2, 2026
Merged
becker33 pushed a commit that referenced this pull request Feb 2, 2026
@alecbcs @becker33
fix locking issue in pull_checkout_branch (#51854)
1ac19b6 
Signed-off-by: Alec Scott <[email protected]>
becker33 pushed a commit that referenced this pull request Feb 2, 2026
@alecbcs @becker33
fix locking issue in pull_checkout_branch (#51854)
3784322 
Signed-off-by: Alec Scott <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Feb 19, 2026
@alecbcs @becker33
fix locking issue in pull_checkout_branch (#51854)
db0b4f5 
Signed-off-by: Alec Scott <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
@alecbcs alecbcs deleted the fix/git-branch-pull branch March 26, 2026 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@becker33 becker33 becker33 approved these changes

@haampie haampie Awaiting requested review from haampie

Assignees

No one assigned

Labels

v1.0.3 PRs to backport for v1.0.3 v1.1.2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alecbcs @haampie @becker33