Let the test suite pass when MSYS2's symlink support is turned on #5975
Conversation
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
Ever since fe53bbc (Git.pm: Always set Repository to absolute path if autodetecting, 2009-05-07), the t9700 test _must_ fail on Windows because of that age-old Unix paths vs Windows paths problem. The underlying root cause is that Git cannot run with a regular Win32 variant of Perl, the assumption that every path is a Unix path is just too strong in Git's Perl code. As a consequence, Git for Windows is basically stuck with using the MSYS2 variant of Perl which uses a POSIX emulation layer (which is a friendly fork of Cygwin) _and_ a best-effort Unix <-> Windows paths conversion whenever crossing the boundary between MSYS2 and regular Win32 processes. It is best effort only, though, using heuristics to automagically convert correctly in most cases, but not in all cases. In the context of this here patch, this means that asking `git.exe` for the absolute path of the `.git/` directory will return a Win32 path because `git.exe` is a regular Win32 executable that has no idea about Unix-ish paths. But above-mentioned commit introduced a test that wants to verify that this path is identical to the one that the Git Perl module reports (which refuses to use Win32 paths and uses Unix-ish paths instead). Obviously, this must fail because no heuristics can kick in at that layer. This test failure has not even been caught when Git introduced Windows support in its CI definition in 2e90484 (ci: add a Windows job to the Azure Pipelines definition, 2019-01-29), as all tests relying on Perl had to be disabled even from the start (because the CI runs would otherwise have resulted in prohibitively long runtimes, not because Windows is super slow per se, but because Git's test suite keeps insisting on using technology that requires a POSIX emulation layer, which _is_ super slow on Windows). To work around this failure, let's use the `cygpath` utility to convert the absolute `gitdir` path into the form that the Perl code expects. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When 0482c32 (apply: ignore working tree filemode when !core.filemode, 2023-12-26) fixed `git apply` to stop warning about executable files, it inadvertently changed the code flow also for symbolic links and directories. Let's narrow the scope of the special `!trust_executable_git` code path to apply only to regular files. This is needed to let t4115.5(symlink escape when creating new files) pass on Windows when symbolic link support is enabled in the MSYS2 runtime. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The `_wopen()` function would gladly follow a symbolic link to a non-existent file and create it when given above-mentioned flags. Git expects the `open()` call to fail, though. So let's add yet another work-around to pretend that Windows behaves like Linux. This is required to let t4115.8(--reject removes .rej symlink if it exists) pass on Windows when enabling the MSYS2 runtime's symbolic link support. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The test case 're-init to move gitdir symlink' wants to compare the contents of `newdir/.git`, which is a symbolic link pointing to a file. However, `git diff --no-index`, which is used by `test_cmp` on Windows, does not resolve symlinks; It shows the symlink _target_ instead (with a file mode of 120000). That is totally unexpected by the test case, which as a consequence fails, meaning that it's a bug in the test case itself. Co-authored-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Just like 0fdcfa2 (t0301: fixes for windows compatibility, 2021-09-14) explained, we should not call `mkdir -m<mode>` in the test suite because that would fail on Windows (because Windows has a much more powerful permission system that cannot be mapped into the simpler user/group/other read/write/execute model). There was one forgotten instance of this which was hidden by a `SYMLINK` prerequisite. Currently, this prevents this test case from being executed on Windows, but with the upcoming support for symbolic links, it would become a problem. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho force-pushed the pass-the-test-suite-with-symlinks-on-windows branch from 5b6cb7c to 7d6c171 Compare The 'symref transaction supports symlinks' test case is guarded by the `SYMLINK` prerequisite because `core.prefersymlinkrefs = true` requires symbolic links to be supported. However, the `preferSymlinkRefs` feature is not supported on Windows, therefore this test case needs the `MINGW` prerequisite, too. There's a couple more cases where we set this config key: - In a subsequent test in t0600, but there we explicitly set it to "false". So this would naturally be supported by Windows. - In t7201 we set the value to `yes`, but we never verify that the written reference is a symbolic link in the first place. I guess that we could rather remove setting the configuration value here, as we are about to deprecate support for symrefs via symbolic links in the first place. But that's certainly outside of the scope of this patch. - In t9903 we do the same, but likewise, we don't check whether the written file is a symbolic link. Therefore this seems to be the only instance where the tests actually need to be adapted. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The MSYS2 runtime (which inherits this trait from the Cygwin runtime, and which is used by Git for Windows' Bash to emulate POSIX functionality on Windows, the same Bash that is also used to run Git's test suite on Windows) has a mode where it can create native symbolic links on Windows. Naturally, this is a bit of a strange feature, given that Cygwin goes out of its way to support Unix-like paths even if no Win32 program understands those, and the symbolic links have to use Win32 paths instead (which Win32 programs understand very well). As a consequence, the symbolic link targets get normalized before the links are created. This results in certain quirks that Git's test suite is ill equipped to accommodate (because Git's test suite expects to be able to use Unix-like paths even on Windows). The test script t1006-cat-file.sh contains two prime examples, two test cases that need to skip a couple assertions because they are simply wrong in the context of Git for Windows. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In Git for Windows, the gitdir is canonicalized so that even when the gitdir is specified via a symbolic link, the `gitdir:` conditional include will only match the real directory path. Unfortunately, t1305 codifies a different behavior in two test cases, which are hereby skipped on Windows. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The device `/dev/null` does not exist on Windows, it's called `NUL` there. Calling `ln -s /dev/null my-symlink` in a symlink-enabled MSYS2 Bash will therefore literally link to a file or directory called `null` that is supposed to be in the current drive's top-level `dev` directory. Which typically does not exist. The test, however, really wants the created symbolic link to point to the NUL device. Let's instead use the `mklink` utility on Windows to perform that job, and keep using `ln -s /dev/null <target>` on non-Windows platforms. While at it, add the missing `SYMLINKS` prereq because this test _still_ would not pass on Windows before support for symbolic links is upstreamed from Git for Windows. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Git's test suite's relies on Unix shell scripting, which is understandable, of course, given Git's firm roots (and indeed, ongoing focus) on Linux. This fact, combined with Unix shell scripting's natural habitat -- which is, naturally... *drumroll*... Unix -- often has unintended side effects, where developers expect the test suite to run in a Unix environment, which is an incorrect assumption. One instance of this problem can be observed in the 'difftool --dir-diff handles modified symlinks' test case in `t7800-difftool.sh`, which assumes that all absolute paths start with a forward slash. That assumption is incorrect in general, e.g. on Windows, where absolute paths have many shapes and forms, none of which starts with a forward slash. The only saving grace is that this test case is currently not run on Windows because of the `SYMLINK` prerequisite. However, I am currently working towards upstreaming symbolic link support from Git for Windows to upstream Git, which will put a crack into that saving grace. Let's change that test case so that it does not rely on absolute paths (which are passed to the "external command" `ls` as parameters and are therefore part of its output, and which the test case wants to filter out before verifying that the output is as expected) starting with a forward slash. Let's instead rely on the much more reliable fact that `ls` will output the path in a line that ends in a colon, and simply filter out those lines by matching said colon instead. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Git for Windows has supported symbolic links for quite some time: In git-for-windows#156, this support was introduced already into Git for Windows v2.4.2.windows.1 in May 2015. However, the Git for Windows CI never ran the test suite with symbolic link support because the MSYS2 runtime (i.e. the POSIX emulation layer required to run Git's test suite because the latter is written in Unix shell script) does not support symbolic links right out of the box. This is for historical reasons: Symbolic link support was introduced in Windows 7, but these links could only be created by administrators by default, and it took until Windows 10 Build 14972 that at least in Developer Mode, non-administrators would be permitted to create them. The MSYS2 runtime _does_ have some sort of support for symbolic links, although with caveats: seeing as it expects the inputs as Unix-like paths, but the outputs need to be Win32 symbolic links pointing to Win32 paths, some normalization has to be performed in the process. This leads to sometimes surprising behavior e.g. when a link target like `a/b/..` is normalized to `a`. It has been a minute or three since the time when Windows versions without symbolic link support were common, therefore there are plans to turn on that support in the MSYS2 runtime on these Windows versions by default, see msys2/msys2-runtime#114 for more details about this. To prepare for this, I am working toward upstreaming Git for Windows' own support for symbolic links. And to prepare for that, in turn, I am hereby contributing preemptively the fixes required to eventually let Git's test suite pass when both MSYS2 runtime and Git support symbolic links. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
If the path we're trying to resolve points to a symbolic link, the `GetFinalPathW()` function won't resolve that link, so let's fall back to the slower-but-accurate method in such cases. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho force-pushed the pass-the-test-suite-with-symlinks-on-windows branch from 7d6c171 to 43f8b93 Compare Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
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.
Currently Git for Windows' test suite does not pass after configuring
MSYS=winsymlinks:nativestrict. This PR fixes it.This PR contains gitgitgadget#2009 (and will need to be adapted in lockstep with the changes the reviewers on the Git mailing list will undoubtedly suggest).