Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3 new Windows GIX_TEST_IGNORE_ARCHIVES=1 failures with Git 2.48.1 #1849

Open
EliahKagan opened this issue Feb 19, 2025 · 3 comments
Open

3 new Windows GIX_TEST_IGNORE_ARCHIVES=1 failures with Git 2.48.1 #1849

EliahKagan opened this issue Feb 19, 2025 · 3 comments
Labels
acknowledged an issue is accepted as shortcoming to be fixed C-Windows Windows-specific issues help wanted Extra attention is needed

Comments

@EliahKagan
Copy link
Member

EliahKagan commented Feb 19, 2025

Current behavior 😯

When the test suite is run on Windows with GIX_TEST_IGNORE_ARCHIVES=1, there are three new failures starting in Git for Windows 2.48.1. I don't know if the problem is in Git for Windows, gitoxide, or gitoxide's test suite:

Those links go to the directly relevant sections of the test output in a gist that contains the full output. The directly relevant sections themselves are:

gix-diff-tests::diff index::renames_by_identity

        FAIL [  32.175s] gix-diff-tests::diff index::renames_by_identity
──── STDOUT:             gix-diff-tests::diff index::renames_by_identity

running 1 test
test index::renames_by_identity ... FAILED

failures:

failures:
    index::renames_by_identity

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 68 filtered out; finished in 32.15s

──── STDERR:             gix-diff-tests::diff index::renames_by_identity
thread 'index::renames_by_identity' panicked at gix-diff\tests\diff\index.rs:181:13:
assertion `left == right` failed: symlinks are only tracked by identity
  left: []
 right: ["link-1", "renamed-link-1"]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

gix-diff-tests::diff tree_with_rewrites::renames_by_identity

        FAIL [   0.081s] gix-diff-tests::diff tree_with_rewrites::renames_by_identity
──── STDOUT:             gix-diff-tests::diff tree_with_rewrites::renames_by_identity

running 1 test
test tree_with_rewrites::renames_by_identity ... FAILED

failures:

failures:
    tree_with_rewrites::renames_by_identity

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 68 filtered out; finished in 0.04s

──── STDERR:             gix-diff-tests::diff tree_with_rewrites::renames_by_identity
thread 'tree_with_rewrites::renames_by_identity' panicked at gix-diff\tests\diff\tree_with_rewrites.rs:276:13:
assertion `left == right` failed: symlinks are only tracked by identity
  left: []
 right: ["link-1", "renamed-link-1"]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

gix-merge::merge tree::run_baseline

        FAIL [  10.432s] gix-merge::merge tree::run_baseline
──── STDOUT:             gix-merge::merge tree::run_baseline

running 1 test
test tree::run_baseline ... FAILED

failures:

failures:
    tree::run_baseline

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 19 filtered out; finished in 10.41s

──── STDERR:             gix-merge::merge tree::run_baseline
failed to extract 'tests\fixtures\generated-archives\tree-baseline.tar': Ignoring archive at 'tests\fixtures\generated-archives\tree-baseline.tar' as GIX_TEST_IGNORE_ARCHIVES is set.
thread 'tree::run_baseline' panicked at tests\tools\src\lib.rs:587:17:
fixture script of "C:/Users/ek/scoop/apps/git/2.48.1\\bin\\bash.exe" "C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\tests\\fixtures\\tree-baseline.sh" failed: stdout: Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/non-tree-to-tree/.git/
[main (root-commit) 07922b1] init
 Author: author <author@example.com>
 1 file changed, 6 insertions(+)
 create mode 100644 a
[A 02ce137] 'A' changes 'a'
 Author: author <author@example.com>
 1 file changed, 1 insertion(+), 1 deletion(-)
[B 417f665] mv 'a' to 'a/sub/b', populate 'a/' with empty files
 Author: author <author@example.com>
 5 files changed, 6 deletions(-)
 delete mode 100644 a
 create mode 100644 a/d
 create mode 100644 a/e
 create mode 100644 a/sub/b
 create mode 100644 a/sub/c
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/tree-to-non-tree/.git/
[main (root-commit) 9fa918e] init
 Author: author <author@example.com>
 4 files changed, 6 insertions(+)
 create mode 100644 a/d
 create mode 100644 a/e
 create mode 100644 a/sub/b
 create mode 100644 a/sub/c
[A 4b22368] 'A' changes 'a/sub/b'
 Author: author <author@example.com>
 1 file changed, 1 insertion(+), 1 deletion(-)
[B 7f7724f] rm -Rf a/ && add non-empty 'a'
 Author: author <author@example.com>
 5 files changed, 1 insertion(+), 6 deletions(-)
 create mode 100644 a
 delete mode 100644 a/d
 delete mode 100644 a/e
 delete mode 100644 a/sub/b
 delete mode 100644 a/sub/c
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/non-tree-to-tree-with-rename/.git/
[main (root-commit) 07922b1] init
 Author: author <author@example.com>
 1 file changed, 6 insertions(+)
 create mode 100644 a
[A 02ce137] 'A' changes 'a'
 Author: author <author@example.com>
 1 file changed, 1 insertion(+), 1 deletion(-)
[B 5109b47] mv 'a' to 'a/sub/b', populate 'a/' with empty files
 Author: author <author@example.com>
 4 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 a/d
 create mode 100644 a/e
 rename a => a/sub/b (100%)
 create mode 100644 a/sub/c
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/tree-to-non-tree-with-rename/.git/
[main (root-commit) 9fa918e] init
 Author: author <author@example.com>
 4 files changed, 6 insertions(+)
 create mode 100644 a/d
 create mode 100644 a/e
 create mode 100644 a/sub/b
 create mode 100644 a/sub/c
[A 4b22368] 'A' changes 'a/sub/b'
 Author: author <author@example.com>
 1 file changed, 1 insertion(+), 1 deletion(-)
[B 7abf1ed] rm -Rf a/ && add empty 'a' (which is like a rename from an empty deleted file)
 Author: author <author@example.com>
 4 files changed, 6 deletions(-)
 rename a/d => a (100%)
 delete mode 100644 a/e
 delete mode 100644 a/sub/b
 delete mode 100644 a/sub/c
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/simple/.git/
[main (root-commit) b8f70c8] initial
 Author: author <author@example.com>
 3 files changed, 7 insertions(+)
 create mode 100644 greeting
 create mode 100644 numbers
 create mode 100644 whatever
[side1 01b0ab9] modify-stuff
 Author: author <author@example.com>
 3 files changed, 3 insertions(+), 2 deletions(-)
rm 'whatever'
[side2 bac05a1] other-modifications
 Author: author <author@example.com>
 4 files changed, 2 insertions(+), 2 deletions(-)
 delete mode 100644 whatever
 create mode 100644 whatever/empty
[side3 d0d15aa] rename-numbers
 Author: author <author@example.com>
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename numbers => sequence (100%)
[side4 e8ad12b] other-content-modifications
 Author: author <author@example.com>
 2 files changed, 2 insertions(+), 1 deletion(-)
[unrelated (root-commit) e8ce47b] first-commit
 Author: author <author@example.com>
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 something-else
[tweak1 9c0e517] Renamed numbers
 Author: author <author@example.com>
 1 file changed, 1 insertion(+)
 rename numbers => "\316\221\317\205\317\204\316\254 \316\274\316\277\317\205 \317\206\316\261\316\257\316\275\316\277\316\275\317\204\316\261\316\271 \316\272\316\271\316\275\316\255\316\266\316\271\316\272\316\261" (70%)
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/rename-delete/.git/
[main (root-commit) 2dbefd6] original
 Author: author <author@example.com>
 4 files changed, 8 insertions(+)
 create mode 100644 foo
 create mode 100644 olddir/a
 create mode 100644 olddir/b
 create mode 100644 olddir/c
[A 5645da1] Modify foo, rename olddir to newdir
 Author: author <author@example.com>
 4 files changed, 1 insertion(+)
 rename {olddir => newdir}/a (100%)
 rename {olddir => newdir}/b (100%)
 rename {olddir => newdir}/c (100%)
[B f0d6c0a] Modify foo & rename foo -> olddir/bar
 Author: author <author@example.com>
 1 file changed, 1 insertion(+)
 rename foo => olddir/bar (71%)
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/rename-add/.git/
[main (root-commit) f93f0f3] original
 Author: author <author@example.com>
 1 file changed, 6 insertions(+)
 create mode 100644 foo
[A d3bb807] Modify foo, add bar
 Author: author <author@example.com>
 2 files changed, 1 insertion(+), 1 deletion(-)
 create mode 100644 bar
[B 2084a7d] rename foo to bar
 Author: author <author@example.com>
 1 file changed, 1 insertion(+)
 rename foo => bar (90%)
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/rename-add-exe-bit-conflict/.git/
[main (root-commit) 7b6e33c] original
 Author: author <author@example.com>
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100755 a
 create mode 100644 b
[A 90e5a2b] -x a
 Author: author <author@example.com>
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 a
[B 1dbf7de] mv b a; chmod +x a
 Author: author <author@example.com>
 1 file changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 b
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/rename-add-symlink/.git/
[main (root-commit) f93f0f3] original
 Author: author <author@example.com>
 1 file changed, 6 insertions(+)
 create mode 100644 foo
[A b65c259] Modify foo, add symlink bar
 Author: author <author@example.com>
 2 files changed, 1 insertion(+), 1 deletion(-)
 create mode 120000 bar
[B 2084a7d] rename foo to bar
 Author: author <author@example.com>
 1 file changed, 1 insertion(+)
 rename foo => bar (90%)
Initialized empty Git repository in C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/rename-add-same-symlink/.git/
[main (root-commit) a54e6e0] original
 Author: author <author@example.com>
 2 files changed, 1 insertion(+)
 create mode 120000 link
 create mode 100644 target
[A fa185d1] rename link to link-new
 Author: author <author@example.com>
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename link => link-new (100%)

stderr: Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'side1'
Switched to branch 'side2'
Switched to branch 'side3'
Switched to branch 'side4'
Switched to a new branch 'unrelated'
Switched to a new branch 'tweak1'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
Switched to branch 'B'
Switched to branch 'A'
error: Your local changes to the following files would be overwritten by checkout:
        link-new
Please commit your changes or stash them before you switch branches.
error: The following untracked working tree files would be overwritten by checkout:
        link
Please move or remove them before you switch branches.
Aborting

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

In that third test, navigating to C:/Users/ek/source/repos/gitoxide/gix-merge/tests/fixtures/generated-do-not-edit/tree-baseline/3163911692-windows/rename-add-same-symlink reveals that link is a symlink pointing to target and link-new is a regular file. The output of ls (i.e. Get-ChildItem, as this is in PowerShell) is:

    Directory: C:\Users\ek\source\repos\gitoxide\gix-merge\tests\fixtures\generated-do-not-edit\tree-baseline\316391169
2-windows\rename-add-same-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
la---           2/18/2025  8:36 PM              0 link -> target
-a---           2/18/2025  8:36 PM              0 link-new

The output of git status is:

On branch A
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        typechange: link-new
        deleted:    target

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        link

no changes added to commit (use "git add" and/or "git commit -a")

The relevant fixture script code for that test is:

git init rename-add-same-symlink
(cd rename-add-same-symlink
touch target
ln -s target link
git add .
git commit -m "original"
git branch A
git branch B
git checkout A
git mv link link-new
git commit -m "rename link to link-new"
git checkout B
ln -s target link-new
git add link-new
git commit -m "create link-new"
)

This made me wonder if this is no longer taking effect with the version of MSYS2 provided by Git for Windows that provides the Git Bash environment:

let mut msys_for_git_bash_on_windows = env::var_os("MSYS").unwrap_or_default();
msys_for_git_bash_on_windows.push(" winsymlinks:nativestrict");

But at least under a cursory examination that does not seem to be broken: I am able to create actual symlinks with ln -s when setting that environment variable and invoking the bash shell provided by Git for Windows, including when version 2.48.1 is installed, and it works whether the bash.exe I use is the "real" bash in (git root)/usr/bin, the Git for Windows wrapper in (git root)/bin, or the scoop wrapper that calls the (git root)/bin wrapper.

The run shown and linked above was from PowerShell 7.5. All the same failures, including those three, occur when running the tests on the same system from Git Bash. The same tests will typically pass or fail from either environment, since a bash.exe shipped with Git for Windows is used either way since #1712 fixed #1359; these new failures do not go against that expectation.

Modifying gix-testtools to use a path to bash.exe in which all directory separators are / does not affect the results. The test failures do not seem to suggest that this would, but I mention this in case it turns out to be relevant. (The reason I know this is that I discovered this bug while testing such a change on a feature branch, then when rerunning tests on the main branch discovered that it was not caused by that change.)

Expected behavior 🤔

Ideally no tests should fail, but the specific expectation that is broken here is that no tests other than the 15 tests known to fail on Windows with GIX_TEST_IGNORE_ARCHIVE=1 and documented in #1358--one of which is a performance test that does not always fail locally and in practice never fails on CI--should fail.

This expectation continues to hold when the test suite is run with Git for Windows 2.47.1(2). Running the tests in otherwise the same local environment matches CI. It is only with Git for Windows 2.48.1 that these three tests fail.

The reason they are not yet failing on CI in the test-fixtures-windows job--which since #1663 will reliably detect any such new failures--appears to be that the runner image being used has Git for Windows 2.47.1(2). It seems likely that those new failures will occur on CI once it is upgraded to 2.48.1.

Git behavior

A change in Git is involved in triggering this, but I don't know what the change is, nor if the change is itself correct.

git-for-windows/git#5411, which pertains to the 2.48.1 release of Git for Windows, links to git-for-windows/git#5376, which mentions that, for 2.48, Git for Windows integrated some patches from the Git mailing list before they were integrated into the upstream Git. I'm not sure to what extent Git for Windows 2.48.1 has Windows-nonspecific patches that didn't make it upstream yet, though. (The Git for Windows 2.48.1 release page does not list such changes.) Even if Git for Windows 2.48.1 does contain such changes, I do not assume that they would be the cause of this.

Steps to reproduce 🕹

This is mostly detailed above, but the specific commands I used to run the tests in PowerShell were:

gix clean -xde
$env:GIX_TEST_IGNORE_ARCHIVES = '1'
cargo nextest run --workspace --no-fail-fast

And in Git Bash, where the results were the same:

gix clean -xde
GIX_TEST_IGNORE_ARCHIVES=1 cargo nextest run --workspace --no-fail-fast

I ran the tests on the main branch at 2efce72 on my main Windows 10 development machine. After initially running them with Git for Windows 2.48.1 installed, I downgraded Git for Windows to 2.47.1(2) and verified that the new failures did not occur, then upgraded it back to 2.48.1 and verified again that they failed again and in the same way. Both versions were installed using scoop, from the scoop main bucket. This packages Portable Git. I do not think that the use of scoop is important here--the trigger seems to be Git for Windows 2.48.1--but I mention it just in case. I also ran the tests without GIX_TEST_IGNORE_ARCHIVES=1 to verify that these tests only fail when that is set.

To check if the problem was specific to Windows, I verified that are still no failures on Arch Linux with Git 2.48.1-1 or Git 2.48.1-2, nor on Ubuntu 24.04 LTS with Git 2.48.1-0 from the git-core PPA, including running the tests both with and without GIX_TEST_IGNORE_ARCHIVES=1 on each. Before each test run, I ran gix clean -xde (not just before the test runs shown above).

@Byron Byron added help wanted Extra attention is needed acknowledged an issue is accepted as shortcoming to be fixed C-Windows Windows-specific issues labels Feb 20, 2025
@Byron
Copy link
Member

Byron commented Feb 20, 2025

Thanks a lot for the exhaustive evaluation of the issue (and for detecting it ahead of time in the first place).

I understand that the tests are failing because the test-scripts don't create symlinks consistently anymore, even though nothing seems to have changed about the bash.exe shipping with Git. With it, it also is indeed possible to create a symlink when done by hand with the respective "MSYS" (winsymlinks:nativestrict) environment variable set.

Quite puzzling.

@EliahKagan
Copy link
Member Author

I understand that the tests are failing because the test-scripts don't create symlinks consistently anymore

What puzzled me was that far more test fixture scripts would have failed if this was fully broken.

I've looked into it further. It turns out that it is specifically renaming symlinks that is broken, which we are doing in the working tree with git mv (and which fortunately seems only to happen in working trees with git mv; operations like switching branches perform replacement rather than renaming).

MSYS is not broken and ln -s still works when the MSYS environment variable has a suitable value, and git will still check out symlinks, stage them, commit them, etc. But Git 2.48.1 has a Windows-specific regression where renaming a symlink dereferences it and renames the target!

This is caused by a new and--other than this bug--improved renaming implementation, which uses CreateFileW to open the file being renamed, but which inadvertently dereferences symlinks and opens their targets due to the absence of the FILE_FLAG_OPEN_REPARSE_POINT flag in the dwFlagsAndAttributes argument. I've reported the bug in git-for-windows/git#5436 and proposed a fix in git-for-windows/git#5437.

@Byron
Copy link
Member

Byron commented Feb 21, 2025

MSYS is not broken and ln -s still works when the MSYS environment variable has a suitable value, and git will still check out symlinks, stage them, commit them, etc. But Git 2.48.1 has a Windows-specific regression where renaming a symlink dereferences it and renames the target!

This is fascinating, thanks for figuring this out and sharing!

Amazing that you managed to fix it as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed C-Windows Windows-specific issues help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants