windows test robsustness #1444
Merged
windows test robsustness #1444
Conversation
This file contains 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
Byron
force-pushed
the
fix-windows-tests
branch
2 times, most recently
from
c3b3931 to
3d2e015
Compare
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.
Namely: Debug, PartialEq, Eq
|
Now that I have done this, I am pretty sure the other issues that happen with archives ignored are very similar, so should be more than fixable. I spent some time today to actually be able to have a half-decent dev-cycle on Windows so I can iterate on these and do minor code edits without ripping out my hair, so I am definitely considering to finally solve #1358 . |
This was referenced Jul 6, 2024
EliahKagan
reviewed
Jul 6, 2024
Comment on lines
+593
to
+594
| let mut msys_for_git_bash_on_windows = std::env::var("MSYS").unwrap_or_default(); | ||
| msys_for_git_bash_on_windows.push_str(" winsymlinks:nativestrict"); |
There was a problem hiding this comment.
I decided to check to make sure this behaves correctly even when another winsymlinks option (rather than other possibly symlink-unrelated options) is already listed in $MSYS.
This code is correct! 😸 I say that based on this experiment:
ek@Glub MINGW64 ~/tmp
$ touch target
ek@Glub MINGW64 ~/tmp
$ MSYS=winsymlinks:lnk ln -s target lnk-symlink
ek@Glub MINGW64 ~/tmp
$ MSYS=winsymlinks:nativestrict ln -s target nativestrict-symlink
ek@Glub MINGW64 ~/tmp
$ MSYS='winsymlinks:lnk winsymlinks:nativestrict' ln -s target lnk-nativestrict-symlink
ek@Glub MINGW64 ~/tmp
$ MSYS='winsymlinks:nativestrict winsymlinks:lnk' ln -s target nativestrict-lnk-symlink
ek@Glub MINGW64 ~/tmp
$ cmd //c dir
Volume in drive C is OS
Volume Serial Number is B203-10FB
Directory of C:\Users\ek\tmp
07/06/2024 07:22 PM <DIR> .
07/06/2024 07:22 PM <DIR> ..
07/06/2024 07:21 PM <SYMLINK> lnk-nativestrict-symlink [target]
07/06/2024 07:21 PM 525 lnk-symlink.lnk
07/06/2024 07:22 PM 525 nativestrict-lnk-symlink.lnk
07/06/2024 07:21 PM <SYMLINK> nativestrict-symlink [target]
07/06/2024 07:19 PM 0 target
5 File(s) 1,050 bytes
2 Dir(s) 157,514,899,456 bytes free
The later option overrides the earlier one, as one would expect. This doesn't seem to be documented for CYGWIN (in Cygwin) or MSYS, but I think it's reasonable to rely on. The alternative of removing prior winsymlinks options seems much less good to me, because that effect could be unintuitive when inspecting the environment and could also create the false impression that other options such as error_start would be removed, even though they would not be.
There was a problem hiding this comment.
Thanks a lot for trying it out! Just this morning I was thinking about this, wondering if it's working as expected :).
This was referenced Sep 5, 2024
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 3, 2024
As far as I know, gix-archive has no limitations related to Unix executable bits, on any platform. On Windows, the *filesystem* does not support these, and `chmod +x` commands in fixtures run in Git Bash appear to succeed but actually have no effect. But +x metadata can be staged and committed in Git repositories. When that is done, gix-archive can use those metadata just as it does on a Unix-like system. This fixes the tests to reflect this ability. Changes: 1. Add a `git update-index --chmod=+x` command to the gix-archive `basic.sh` fixture for `dir/subdir/exe`, so that even if the `chmod +x` command has no effect, the executable bit is set. This only affects `dir/subdir/exe`. It does not affect `extra-exe`, since that is never staged. On Windows, `extra-exe` can never have any associated executable mode bits. 2. Update the `basic_usage_internal` test to assert that `dir/subdir/exe` is `EntryKind::BlobExecutable` on all platforms, i.e., no longer `EntryKind::Blob` on Windows. Without this change, the change in (1) causes the test to fail. This also refactors to remove the `expected_exe_mode` constant, since its value is now only used in one place (for `extra-exe`), and to remove `expected_link_mode`, which has unconditionally been another name for `EntryKind::Link` since 93e088a (GitoxideLabs#1444). 3. Update the `basic_usage_tar` test to assert that the mode stored for `prefix/dir/subdir/exe` is 493 (0o755) on all platforms, i.e., no longer 420 (0o644) on Windows. This is analogous to (2), and without this the `basic_usage_tar` test fails due to the changes in (1). As in (2), this includes refactoring: `expected_exe_mode` is removed now that the choice between 420 (0o644) and 493 (0o755) is only made in one place (for `prefix/extra-exe`), and `expected_symlink_type` is removed, since it has unconditionally been another name for `EntryType::Symlink` since 93e088a (GitoxideLabs#1444). For future reference, with (1) but before (2), the failure is: --- STDERR: gix-archive::archive from_tree::basic_usage_internal --- Archive at 'tests\fixtures\generated-archives\basic.tar' not found, creating fixture using script 'basic.sh' thread 'from_tree::basic_usage_internal' panicked at gix-archive\tests\archive.rs:36:13: assertion `left == right` failed left: [(".gitattributes", Blob, Sha1(45c160c35c17ad264b96431cceb9793160396e99)), ("a", Blob, Sha1(45b983be36b73c0788dc9cbcb76cbb80fc7bb057)), ("symlink-to-a", Link, Sha1(2e65efe2a145dda7ee51d1741299f848e5bf752e)), ("dir/b", Blob, Sha1(ab4a98190cf776b43cb0fe57cef231fb93fd07e6)), ("dir/subdir/exe", BlobExecutable, Sha1(e69de29bb2d1d6434b8b29ae775ad8c2e48c5391)), ("extra-file", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-exe", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-dir-empty", Tree, Sha1(0000000000000000000000000000000000000000)), ("extra-dir/symlink-to-extra", Link, Sha1(0000000000000000000000000000000000000000))] right: [(".gitattributes", Blob, Sha1(45c160c35c17ad264b96431cceb9793160396e99)), ("a", Blob, Sha1(45b983be36b73c0788dc9cbcb76cbb80fc7bb057)), ("symlink-to-a", Link, Sha1(2e65efe2a145dda7ee51d1741299f848e5bf752e)), ("dir/b", Blob, Sha1(ab4a98190cf776b43cb0fe57cef231fb93fd07e6)), ("dir/subdir/exe", Blob, Sha1(e69de29bb2d1d6434b8b29ae775ad8c2e48c5391)), ("extra-file", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-exe", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-dir-empty", Tree, Sha1(0000000000000000000000000000000000000000)), ("extra-dir/symlink-to-extra", Link, Sha1(0000000000000000000000000000000000000000))] And with (1) but before (3), the failure is: --- STDERR: gix-archive::archive from_tree::basic_usage_tar --- thread 'from_tree::basic_usage_tar' panicked at gix-archive\tests\archive.rs:116:13: assertion `left == right` failed left: [("prefix/.gitattributes", Regular, 56, 420), ("prefix/a", Regular, 3, 420), ("prefix/symlink-to-a", Symlink, 0, 420), ("prefix/dir/b", Regular, 3, 420), ("prefix/dir/subdir/exe", Regular, 0, 493), ("prefix/extra-file", Regular, 21, 420), ("prefix/extra-exe", Regular, 0, 420), ("prefix/extra-dir-empty", Directory, 0, 420), ("prefix/extra-dir/symlink-to-extra", Symlink, 0, 420)] right: [("prefix/.gitattributes", Regular, 56, 420), ("prefix/a", Regular, 3, 420), ("prefix/symlink-to-a", Symlink, 0, 420), ("prefix/dir/b", Regular, 3, 420), ("prefix/dir/subdir/exe", Regular, 0, 420), ("prefix/extra-file", Regular, 21, 420), ("prefix/extra-exe", Regular, 0, 420), ("prefix/extra-dir-empty", Directory, 0, 420), ("prefix/extra-dir/symlink-to-extra", Symlink, 0, 420)] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 3, 2024
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.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 4, 2024
As in 470c76e, this updates tests to reflect the ability of Git repositories to represent Unix-style executable permissions in an index and commits, and the ability of gitoxide to operate on this, on all platforms, including Windows where the filesystem itself does not support this kind of executable permissons. Changes: 1. Adds a `git update-index --chmod=+x` command in the basic.sh fixture for gix-worktree-steam, so `dir/subtree/exe` is marked executable in the repository, even on Windows where the filesystem itself does not support Unix-style executable permissions and where, in Git Bash, `chmod +x` has no effect. This does not affect `extra-exe`, which is never staged. Since `extra-exe` is just an extra file in the working tree, it cannot be marked `+x` on Windows. 2. Updates the `paths_and_modes` assertion in `will_provide_all_information_and_respect_export_ignore` to assert the correct mode, with `+x` set, in the staged/committed file `dir/subtree/exe`. The `extra-exe` part of the assertion is unchanged in meaning, but the `expected_exe_mode` would only be used in that one place now, so this removes it and replaces it with the conditional expression for its value based on whether we are using Windows. While doing that refactoring, this also removes the `expected_link_mode` constant, which has unconditionally aliased `EntryKind::Link` since 93e088a (GitoxideLabs#1444). These changes are directly analogous to (1) and (2) in 470c76e. Here, they are made to the gix-worktree-stream tests, while in 470c76e they were made to the gix-archive tests (along with another related change, in (3), for which there is nothing anlogous to be done here). For reference, with *this* commit's change (1) described above but without *this* commit's change (2), the failure is: --- STDERR: gix-worktree-stream::stream from_tree::will_provide_all_information_and_respect_export_ignore --- Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.45s thread 'from_tree::will_provide_all_information_and_respect_export_ignore' panicked at gix-worktree-stream\tests\stream.rs:119:9: assertion `left == right` failed left: [(".gitattributes", Blob, Sha1(45c160c35c17ad264b96431cceb9793160396e99)), ("a", Blob, Sha1(45b983be36b73c0788dc9cbcb76cbb80fc7bb057)), ("bigfile", Blob, Sha1(4995fde49ed64e043977e22539f66a0d372dd129)), ("symlink-to-a", Link, Sha1(2e65efe2a145dda7ee51d1741299f848e5bf752e)), ("dir/.gitattributes", Blob, Sha1(81b9a375276405703e05be6cecf0fc1c8b8eed64)), ("dir/b", Blob, Sha1(ab4a98190cf776b43cb0fe57cef231fb93fd07e6)), ("dir/subdir/exe", BlobExecutable, Sha1(e69de29bb2d1d6434b8b29ae775ad8c2e48c5391)), ("dir/subdir/streamed", Blob, Sha1(08991f58f4de5d85b61c0f87f3ac053c79d0e739)), ("extra-file", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-bigfile", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-exe", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-dir-empty", Tree, Sha1(0000000000000000000000000000000000000000)), ("extra-dir/symlink-to-extra", Link, Sha1(0000000000000000000000000000000000000000))] right: [(".gitattributes", Blob, Sha1(45c160c35c17ad264b96431cceb9793160396e99)), ("a", Blob, Sha1(45b983be36b73c0788dc9cbcb76cbb80fc7bb057)), ("bigfile", Blob, Sha1(4995fde49ed64e043977e22539f66a0d372dd129)), ("symlink-to-a", Link, Sha1(2e65efe2a145dda7ee51d1741299f848e5bf752e)), ("dir/.gitattributes", Blob, Sha1(81b9a375276405703e05be6cecf0fc1c8b8eed64)), ("dir/b", Blob, Sha1(ab4a98190cf776b43cb0fe57cef231fb93fd07e6)), ("dir/subdir/exe", Blob, Sha1(e69de29bb2d1d6434b8b29ae775ad8c2e48c5391)), ("dir/subdir/streamed", Blob, Sha1(08991f58f4de5d85b61c0f87f3ac053c79d0e739)), ("extra-file", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-bigfile", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-exe", Blob, Sha1(0000000000000000000000000000000000000000)), ("extra-dir-empty", Tree, Sha1(0000000000000000000000000000000000000000)), ("extra-dir/symlink-to-extra", Link, Sha1(0000000000000000000000000000000000000000))] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 4, 2024
As detailed in d715e4a, tests involving symlinks have been expected to work even on Windows since GitoxideLabs#1444, but some tests remain to be updated to be run on Windows or to include all symlink-related asertions on Windows. This includes 4 tests in `gix-dir/tests/walk/mod.rs`, which were ignored on Windows even though they are able to pass. This commit enables them on Windows. It also updates the associated fixture scripts to no longer say that symlink test can't run on Windows. But no changes to the fixture scripts are required for the tests to pass. (While doing so, I've made a small change, adding quoting to a here document delimiter to make clear that no expansions are intended to occur in the here document text. But this change is purely for clarity; nothing was broken in connection with that heredoc.)
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 4, 2024
As detailed in d715e4a, most tests involving symlinks have been expected to work even on Windows since GitoxideLabs#1444, but some tests remain to be updated to be run on Windows or to include all symlink-related asertions on Windows. This includes 4 tests in `gix-dir/tests/walk/mod.rs`, which were ignored on Windows even though they are able to pass. This commit enables them on Windows. It also updates the associated fixture scripts to no longer say that symlink test can't run on Windows. But no changes to the fixture scripts are required for the tests to pass. (While doing so, I've made a small change, adding quoting to a here document delimiter to make clear that no expansions are intended to occur in the here document text. But this change is purely for clarity; nothing was broken in connection with that heredoc.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Allow windows tests to work even if symlinks are available.
Fixes #1443.
Tasks
MSYS=winsymlinks:nativestrictto make these tests pass