Let nonstandard worktree fixtures work even if Git < 2.37.2 #1334
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.
This reorders the operations in the nonstandard worktree fixtures to work even when Git is at a lower version than 2.37.2. This makes
root_enters_directory_with_dot_git_in_reconfigured_worktree_tracked
pass on more systems, including common development systems.When fixture scripts are run on a system whose Git version is lower than 2.37.2, including currently maintained downstream versions with backported security (but not most other) fixes on some popular OSes (e.g. Ubuntu 22.04.4 LTS with 2.34.1-1ubuntu1.10), the
gix-dir
nonstandard worktree fixtures had silently failed to stage one of two intended files. They appeared to succeed, but created a repo with only theseemingly-outside
file, and not theinside
file.This is due to the bug git/git@27128996b8 fixed in Git. The nonstandard worktree fixtures had needed Git to support the first condition noted there, where
core.worktree
is the parent of the default worktree, in order to stage both files.The change made here overcomes the problem by staging before rather than after moving
.git
and settingcore.worktree
. Thegit add
command used to stage is adjusted to remain equivalent in its effect. (Committing can be, and is, still done last.)CI hadn't suffered/shown the problem, since while GHA ubuntu-latest runners are currently Ubuntu 22.04, they have many newer software versions installed on them, currently including Git 2.43.2.
The original test failure
Prior to this change, the failure on affected systems (those with insufficiently recent git) is in the assertion about the value of
entries
:Details of how I ran the tests are included below.
Rationale
Since the fix is straightforward, I think it is worth applying here in gitoxide's test fixtures, even though the actual bug involved in triggering the failure is not in any code carried by gitoxide but rather in Git.
However, if that is not judged to be worthwhile, then I recommend the issue be noted somewhere, preferably in a comment in or near the relevant fixtures or the one test that is currently failing on affected systems. In that case, this PR could be modified to add such a comment instead of making the change to the fixture script. Or it could be closed and I could open a new one.
Some test fixtures in gitoxide have associated generated archives that may need to be updated under some circumstances. I believe that is inapplicable here, because no generated archive associated with the
gix-dir
test fixture is tracked. (In addition, the fixture changes here do not change the way the repository created by the fixture is set up, so even if generated archives were tracked for it, I don't think there would be any need to regenerate them.)Verification
I performed a number of tests, building various Git versions and testing them with the fixture shell script fragment to verify that the problem is caused by the bug cited. Having narrowed down where the problem was in Git, I also verified that building Git from the bugfix's parent commit triggered the problem in an actual gitoxide test run, while building Git from the bugfix commit averted it; and I ran that full test with a few released versions of Git as well. I took these precautions:
cargo clean
norgit clean -dXf
are sufficient to remove it (I believe becausegit
sees these are nested repositories it shouldn't touch).gitoxide
was followed bygit lfs install
andgit lfs pull
in the cloned directory, and these commands were always done with the systemgit
, even when it was a different version that I then placed earlier in the path to be used in the tests.cargo nextest run --all --no-fail-fast
from the root of the gitoxide working tree. (The exception to this is shown above, where for demonstration I did another clone and reran just the affected test, to show its failure clearly.)No test ever failed except the one being investigated.
Some notes from that, and a script I used in otherwise manual bisection, can be seen in this gist. But I think the only actually important information is the result of manually trying out the fixtures on system and current versions of Git, with the version of the fixture before the change versus after. So I've reproduced that as follows. These are on an Ubuntu 22.04 system (x86_64).
Git 2.44.0 built from source, with the old (current) version of the fixture
Git 2.44.0 built from source, with the new (proposed) version of the fixture
Git 2.34.1 provided by the system, with the old (current) version of the fixture
In this case, and none of the others, the fixture fails to stage one of the two files, leading to the spurious test failure this PR seeks to rectify.
Git 2.34.1 provided by the system, with the new (proposed) version of the fixture