9 tests rely on commands like ln -s making copies instead of symlinks on Windows
#1443
Comments
EliahKagan
changed the title
ln -s making copies instead of symlinks on Windowsln -s et al. making copies instead of symlinks on Windows
EliahKagan
changed the title
ln -s et al. making copies instead of symlinks on Windowsln -s making copies instead of symlinks on Windows
|
Thanks a lot for reporting!
It's true, the intend is for it to actually create a symlink, even on Windows. It was pretty clear that this didn't happen though, so I started to rely on using archives instead. These are also known to not create symlinks on Windows, I think, and it's strange that 9 failing testsI looked into one of the failing tests, In theory, testing with archives should be the same as testing without, and I'd hope that this Testing without archives on WindowsThere is a way to partition tests and only run a subset of them in a deterministic fashion. To keep Windows runtimes in check and not let them pass 25 minutes, one could use this with a controlling script that runs random, non-repeating partitions until the time runs out. The partitioning should help to over time run all tests, instead of running only the first X tests each time. https://nexte.st/docs/ci-features/partitioning/ Maybe this is something you could establish once all tests are fixed. |
|
I think the reason for these 9 tests failing is that they don't have archives to begin with, so the scripts are always executed and thus affected by |
I'm relieved that the reason--even if not the needed fix--is straightforward.
It seems to me that pre-generated archives actually have an advantage beyond working around known platforms limitations and speeding up test runs: they provide some protection against unknown platform-specific problems that would cause fixtures to generate the wrong thing, especially if the tests are being run fairly regularly both with and without using the committed archives.
I wonder if there is also a way to effectively compare the archives that get generated (though what kinds of variations are correct would differ across tests that use them). |
This will only reliably work on with developer setups, but that seems fair to assume. If this causes problems, it's fine to make it opt-in as well.
I think I forgot to mention that the idea would be to use nextest as runner but without using test archives to force actually executing the scripts to assure they work. Maybe, instead of complicating the thing with partitioning, the job could also be run but setup so nobody has to wait for it. Ideally, failures get communicated by email but otherwise are not blocking. |
…deLabs#1443) This will only reliably work on with developer setups, but that seems fair to assume. If this causes problems, it's fine to make it opt-in as well.
A few of the gix-worktree-state checkout tests have been checking if the filesystem supports symlinks, while one was skipped (marked ignored) on Windows based on the idea that symlinks would not be created, and also had an assertion that assumed symlinks would not be successfully created. This commit changes such tests so that they run on all platforms, including Windows, and so that, on all platforms, they assert that the filesystem supports symlinks, and assert that expected symlinks are created after attempts to do so. (The reason to assert that the filesystem supports symlinks is so that if this is not detected, either due to a failure or detection or due to the filesystem really not supporting symlinks, the test failures will be clear, rather than having a later assertion fail for unclear reasons.) Since GitoxideLabs#1444, tests involving symlinks are expected to work even on Windows. That PR included changes to the way fixtures were run, and to other parts of the test suite, to cause symlinks to be created in cases where they had previously had not (GitoxideLabs#1443). A number of tests had been assumed not to work due to limitations of Windows, MSYS, or Git: - Although Windows will not allow all users to create symlinks under all configurations, the test suite expects to be run on a Windows system configured to permit this. - Although `ln -s` and similar commands in MSYS environments, including Git Bash, do not by default attempt to create actual symlinks, this does happen when symlink creation is enabled using the `MSYS` environment variable, as done in 0899c2e (GitoxideLabs#1444). (This situation differs from that of Unix-style executable bits, which do not have filesystem support on Windows. While `chmod +x` and `chmod -x` commands do not take effect on Windows, which slightly limits the ability to test such metadata and requires that a number of fixtures set the mode directly in the ndex, with symlinks there is no such inherent restriction. Provided that the `MSYS` environment variable is set to allow it, which gix-testtools takes care of since GitoxideLabs#1444, and that Windows permits the user running the test suite to create symlinks, which is already needed to properly run the test suite on Windows, the same `ln -s` commands in fixture scripts that work on Unix-like systems will also work on Windows.) - Although `git` commands will not check out symlinks as actual symlinks on Windows unless `core.symlinks` is set to `true`, this is not typically required for the way symlinks are used in the gitoixde test suite. Instead, we usually test the presence of expected symlink metadata in repository data structures such as an index and trees, as well as the ability of gitoxide to check out symlinks. (We do not intentionally test the ability to run `ln -s` in Git Bash, but this is needed in order to create a number of the repositories for testing. Having `git` check out symlinks is not typically needed for this.) In addition, since we are requiring that Windows test environments permit the user running the test suite to create symlinks, any failures that arise in the future due to greater sensitivity to `core.symlinks` (see GitoxideLabs#1353 for context) could be worked around by setting that configuration variable for the tests, either in gix-testtools via `GIT_CONFIG_{COUNT,KEY,VALUE}` or in the specifically affected fixture scripts. While GitoxideLabs#1444 updated a number of tests to reflect the ability to create symlinks in fixture scripts and the wish to test them on all platforms including Windows, some tests remain to be updated. This commit covers the gix-worktree-statte checkout tests. This does not cover even their associated fixtures, which can already create symlinks (given the above described conditions), but that should be updated so they can set intended executable permissions (see above on `chmod`). This will be done separately.
Current behavior 😯
Background
As detailed in #1442,
ln -sin Git Bash (or other MSYS2 environments) in Windows does not ordinarily attempt to create symlinks, but instead copies files. Its default behavior of copying files is independent of whether the current user has the ability to create symlinks, and also independent of thecore.symlinksGit configuration variable. The behavior is also not directly related to Git's behavior of creating regular files whencore.symlinksis not set totrue, because when Git creates a regular file instead of a symlink, that file still represents a symlink in the index and contains its target path, rather than being a copy of the target.ln -sis used in about 14 of gitoxide's fixture scripts.Although
ln -sdoes not typically attempt to create symlinks, it will attempt to do so--and succeed if the user has the ability to create them without UAC elevation--if the default behavior of MSYS2's emulatedsymlinksystem call is customized in theMSYSenvironment variable. As noted in git-for-windows/git#3122 (comment) and #1442, one way to do this is to setMSYS=winsymlinks:nativestrict.One should be careful about doing this automatically, because the user may already have it set; like the
CYGWINvariable that it is based on, it supports a number of options, not all of which relate to symlinks, which can be set together in its value by separating them with spaces (or tabs). Therefore, if in the future gitoxide test fixtures automatically enablewinsymlinks:nativestrict, they should probably do so in a way that preserves unrelated options in$MSYSif present. However, such subtleties can usually be ignored in manual testing on a system whereMSYSis known to be unset.Test failures with
MSYS=winsymlinks:nativestrictOriginally my goal was to find out if any tests documented as failing in #1358 were failing due to
ln -sor other Unix-style commands (e.g.,tarto unpack an archive with symlinks) not really creating symlinks, such that they would pass withMSYS=winsymlinks:nativestrict. It turns out that there was exactly one such test; see #1358 (comment).However, the bigger effect is that there are nine new failures! This is to say that some tests have been depending on
ln -s, or other commands that would ordinarily create symlinks, not attempting to do so.Failures when using pre-generated archives
Using pre-generated archives by not defining
GIX_TEST_IGNORE_ARCHIVES, I ran:This produced 9 failures:
See this gist for the full test output of this run. Other than the information in this issue description, that gist contains the most important information relevant to this issue. The next subsection covers the effect of setting both
MSYSandGIX_TEST_IGNORE_ARCHIVES, but for the purpose of showing that their effects are mostly independent, so it may be of less interest.Failures when ignoring pre-generated archives
I independently tried it with:
This gave 24 failures:
But most of those failures are those that occur without setting the
MSYSenvironment variable and were reported in #1358. The new failures are the tests that failed above withoutGIX_TEST_IGNORE_ARCHIVES.See this other gist for the full test output from this run without using pre-generated archives.
Not necessarily specific to
ln -sThese results are not necessarily specific to
ln -s. This does not necessarily produce the same effect as setting the environment variable only forln -scalls, because the MSYS behavior it is affecting is not specifically the behavior ofln -s, but rather the behavior of any executable that links tomsys-2.0.dlland makes use of the POSIXsymlinkcall that it emulates. Some more details on this distinction are given in #1442.Note that fully native executables, including
git, do not link tomsys-2.0.dll, so whether they attempt to create symlinks is unaffected by theMSYSenvironment variable. In the MSYS2 documentation, this is correlated with the "C library" column in this table.Expected behavior 🤔
When
ln -screates symbolic links, that should not cause any tests to fail on any system. If other POSIX commands such astarare also involved in this, the same applies: when such a command ordinarily would create symbolic links but may not always do so on Windows, its ability to do so on Windows should not break the tests.If special-casing is in place to work around the difficulty creating symlinks on Windows by treating their presence as unexpected, then this disparity with other platforms should be eliminated when possible. If not possible, the special-casing should be made more robust, so it does not assume the absence of features that may be present in reasonable configurations on non-broken systems.
If commands such as
ln -sare used in test fixtures, or invoked by other means in testing, in any situations where they are really not intended to create symbolic links, then either a different command should be used, or the necessary configuration change should be made, such as removing symlink-related options from the value of$MSYSwhen present. My guess, however, is that calls toln -sin gitoxide's test suite are not deliberately intended to copy a file rather than create a symlink.Git behavior
Not applicable.
Steps to reproduce 🕹
Test commands are all shown above. I ran
gix clean -xdebetween tests.The Windows 10 system I tested on has "developer mode" enabled, which permits me to create symlinks without UAC elevation; this is important to the results in that it demonstrates that the problem is not the inability of
ln -sto create symlinks when it tries, but rather that these nine failures seem to be due to it actually creating them.The text was updated successfully, but these errors were encountered: