Checkout fails when Windows symlinks have strangely named targets #1420
Labels
acknowledged
an issue is accepted as shortcoming to be fixed
Comments
This was referenced Jun 26, 2024
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this issue
Jun 26, 2024
These are effectively special cases of dangling symlinks. They should be treated the same as ordinary dangling symlinks, but the error kind isn't NotFound for these, so currently they are not created. The new tests should pass once that is fixed. See GitoxideLabs#1420.
LuaKT
pushed a commit
to LMonitor/gitoxide
that referenced
this issue
Aug 20, 2024
These are effectively special cases of dangling symlinks. They should be treated the same as ordinary dangling symlinks, but the error kind isn't NotFound for these, so currently they are not created. The new tests should pass once that is fixed. See GitoxideLabs#1420.
LuaKT
pushed a commit
to LMonitor/gitoxide
that referenced
this issue
Aug 20, 2024
When the metadata of a symlink's target cannot be obtained, even if the error is something other than `NotFound`, this falls back to creating file symbolic links. This only affects scenarios where either the checkout would fail entirely or where the symlink would have been treated as a collision and skipped (even though it was not really a collision, since only its target had an error). Other cases are not affected, and all exisitng scenarios where directory symlink would be created will still create directory symlinks. This builds on 31d02a8 (GitoxideLabs#1363) by supporting dangling symlinks even when the target filenames are unusual, such as when the name is invalid or reserved. Windows permits such symlinks to be created, and going ahead and creating the matches the Git behavior. This should also support other errors beisdes `NotFound`. For example, some permissions-related errors, in some cases where traversal or acccess (even to access metadata) are not allowed, would fail to create a symlink. This should address that as well. This works by using `Path::is_dir()` in the standard library, which automatically converts all errors (not just `NotFound`) into `false`. The logic here is thus quite similar to what was already present, just more tolerant, even though the code itself is shorter and simpler. This fixes GitoxideLabs#1420, and also fixes GitoxideLabs#1421.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Current behavior 😯
Windows permits symbolic links to specify targets with unusual names, such as invalid filenames like
???and reserved legacy DOS device names likeCON. Cloning a repository on Windows withgix clonethat contains such a symlink fails, due to the error that occurs when attempting to look up metadata for such a target, which is done to figure out if a file symlink or directory symlink should be created.gix clonehas supported creating dangling symlinks on Windows since 31d02a8 (#1363), which fixed #1354 by special-casingNotFounderrors and falling back to creating a file symlink:https://github.com/Byron/gitoxide/blob/8d89b865c84d1fb153d93343d1ce4e1d64e53541/gix-fs/src/symlink.rs#L44-L48
Most cases of dangling symlinks are covered by
NotFounderrors. This includes cases where there is no target file at the destination, where there is no destination (e.g., the target path has nonexistent intermediate components), and possibly some other cases where errors prevent the target from being visible. However, this does not cover all errors. When the target is an invalid or reserved name, checkout fails.Furthermore, it is not only those specific symlinks that are refused; checkout as a whole fails, and
gix cloneleaves no cloned directory behind. This can be seen when cloning a repository with a symlink to???:And when cloning a repository with a symlink to
CON:More detailed reproduction is presented below.
This is only relevant when
gixoperates with a configuration that allows it to create symlinks. When it detects that it cannot create any symlinks at all, or when it is invoked with-c core.symlinks=false, it creates regular files instead of symlinks, so the behavior described here does not occur. Note thatgixwill usually attempt to create symlinks, though, and as of this writing this bug can be observed even whencore.symlinksis set tofalseglobally, due to #1353.Expected behavior 🤔
Option 1: Create the symlinks
The strange symlink, which is typically a dangling symlink (see below for details), should probably just be created. This is simple, should be the simplest kind of fix to implement, and would make
gixbehave more closely to Git. As detailed in the next section on Git behavior, Git creates these without complaint.It should be feasible to implement this by using an expression like
orig_abs.is_dir()in place of the entire existingmatchlogic, becausePath::is_dirperforms anfs::metadatacheck, as we are currently doing explicitly, and then converts all errors tofalse, rather than justNotFoundas we are currently doing. I intend to open a PR for this shortly.(Such a change would leave the behavior invariant with respect to what kind of symlink is created, in cases where a symlink is already being created. Since
fs::metadatarather thanfs::symlink_metadatais used, this behavior is to attempt to fully dereference the target if possible. If the target is a symlink whose ultimate target does not exist, or cannot be reached—such as due to the wrong kind of symlink, i.e. file vs. directory, appearing somewhere in the chain—then that is treated as an error. So a symlink to a directory symlink that dangles is currently created as a file symlink. This is not necessarily the best possible behavior, but I believe it should be considered independent of this bug and, if it is to be changed, then that can be done separately. Note that Git will sometimes do this as well, and always does so in the straightforward scenario that the ultimate target directory is not also being created.)Option 2: Refuse with a specific error and check out other files
However, if for some reason it is intentional that symlink creation be refused, then
gixshould:I think there is probably no justification for refusing to create symlinks, even unusual ones. I recommend Option 1.
A note on the semantics of such symlinks
Symlinks whose targets are invalid filenames or reserved device names are, for the most part, special cases of dangling symlinks. It seems to me that the behavior of creating dangling symlinks, and making them file symlinks on Windows where the type is unknown because the target does not exist, should apply here.
The following two symlink demonstrations are done in
cmd.exe. Usually PowerShell should be preferred, butcmd.exeis easier to use for symlink demonstrations because itsmklinkbuiltin creates symlinks and itsdircommand shows their exact symlink type (file or directory).It is intuitive that a symlink to an invalid path like
???is dangling, since no file named???exists. This shows that I can create the symlink, and attempting to create the target file by writing through the symlink fails:It is less intuitive that a symlink to a reserved DOS device name like
CONwould be dangling, since in most file access APIs, such names, except in\\?\paths, are treated as though they exist in every directory. So isn't a symlink toCONa non-dangling symlink to theCONdevice?Although I would not be confident that no application or API treats it that way, the answer is generally no--instead, it refers to a file named
CONin the current directory on the actual filesystem, which typically does not exist, but which can be created by writing through the symlink:Git behavior
Git creates the symlinks to the strangely named targets, without complaint. Since these are dangling symlinks, Git creates them as file symlinks. I tested with:
Cloning the repository containing a symlink to
???:Cloning the repository containing a symlink to
CON:I showed the output of the
dirbuiltin ofcmd.exebecause it distinguishes file and directory symlinks, thereby verifying that Git creates them as file symlinks. If they had been directory symlinks,<SYMLINKD>would have been shown instead of<SYMLINK>.The details of how those repositories were prepared are included below.
Steps to reproduce 🕹
1. Create test repositories or examine the ones I made
Either create a repository with a symlink to
???and a repository with a symlink toCON, or use the preexisting test repositories I created,symlink-to-qmarksandsymlink-to-con. To verify that both cases are broken, these need to be two separate repositories, because the current behavior is to fail the checkout entirely and leave no cloned files.I created both repositories on an Ubuntu system (and WSL is definitely sufficient), though it could be done in native Windows. Note that
ln -smay not be sufficient to create symlinks on native Windows, even thoughlnexists in Git Bash; this may create copies instead.A repository like
symlink-to-qmarkscan be created in a Unix-like system like this:A repository like
symlink-to-concan be created in a Unix-like system analogously:They can then be uploaded to remotes and used to test
gix clone. I made them roughly like that, but I also added more commits with things like readme files, so it would be clear what they were.All subsequent steps should be done on Windows. I will use SSH URLs here because the output with
gix --traceis shorter and more readable, but I have also tested this with HTTPS URLs to verify that the problem is not specific to any transport (though it would be very strange if it were, especially since the specific cause of the bug is known, as detailed above).2. Optionally, clone the repositories with
git cloneto verify Git behaviorThis is detailed in "Git behavior" above, including specific commands to clone the repositories and to inspect the checked out contents.
This shows that, with Git, the clone succeeds, and the specific dangling symlinks (with strangely named targets) are created.
Before proceeding to test with
gix, remove the created directories (or, alternatively, do all subsequent steps in a different directory) to avoid clashes confusing the experiment. They can be removed in PowerShell withrm -r -fo:3. Clone the repositories with
gix cloneto observe the bugOutput without
--traceis shown above in "Current behavior" demonstrating that:gix clone git@github.com:EliahKagan/symlink-to-qmarks.gitfails and reports that the failure is "Caused by":gix clone git@github.com:EliahKagan/symlink-to-con.gitfails and reports that the failure is "Caused by":Inspection reveals that
symlink-to-qmarksandsymlink-to-condirectories do not exist after the operation. Thus the clone as a whole failed, rather than just refusing to check out specific files.4. Optionally, clone the repositories with
gix --trace cloneWhen passing
--trace, the output is very long for HTTPS URLs, which is why SSH URLs have been used throughout, so this output is reasonably short and can be compared directly to non---traceoutput (and to Git behavior).Cloning the repository with a symlink to
???with--tracelooks like:Cloning the repository with a symlink to
CONwith--tracelooks like:Solution
My forthcoming PR will have tests that fail before the bugfix and pass afterwards. I have already written those tests, verified that they fail with no change to the code under test, written the fix, and verify that the tests pass with it. My pull request is not ready yet because:
But when that is ready, running its own tests against the current version of the code will also work as verification, as would running the new fixtures manually to produce test repositories and then testing with those repositories.
The text was updated successfully, but these errors were encountered: