-
Notifications
You must be signed in to change notification settings - Fork 42
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
Implement a winsymlinks
mode that prefers native symlinks, falling back to the deep copy mode
#114
base: msys2-3.5.3
Are you sure you want to change the base?
Implement a winsymlinks
mode that prefers native symlinks, falling back to the deep copy mode
#114
Conversation
e69c9f4
to
cb31af7
Compare
This adds the missing documentation (in case we ever publish the information about the MSYS variable on msys2.org). Signed-off-by: Johannes Schindelin <[email protected]>
…llback When native symlinks are available, it is a shame to create deep copies by default. However, since there are many scenarios where symlinks are not available (e.g. when running on FAT, or on older Windows versions, or when Developer Mode is not enabled), we've got to have a fallback. In the regular Cygwin world, it is legitimate to fall back to WSL symlinks and/or to the system file emulation (where a file is created that is marked with the "system" attribute and with content that adheres to a specific, magic form that is recognized specifically by the Cygwin runtime). However, in the world of MSYS2, the assumption is that the result of the operation should be as interoperable with regular Win32 programs as possible. Hence the default to "deepcopy". As a "best of both worlds" mode, let's implement one that tries to create native symlinks by default, and if that fails, uses the "deepcopy" method as a fallback. This addresses msys2#113. Signed-off-by: Johannes Schindelin <[email protected]>
cb31af7
to
b8e842e
Compare
Cygwin not only had, it still has symlinks like that - except the docs suggest that the
The "shortcut" style links are something different. The new Developer Mode feature is OK, but it's got a big warning on it when you enable it and enterprises therefore don't want to enable it:
https://stackoverflow.com/questions/74613686/what-does-the-warning-in-developer-mode-mean I set |
This mode is incompatible with regular Win32 programs; They won't understand those symlinks and misinterpret them all the time. That is an okay stance to take for Cygwin, which wants you to stay within its ecosystem. It is not an okay stance for MSYS2 which wants to integrate with native Win32 programs as much as possible.
Yes, developer mode comes with a lot of liberties that developing code requires. Nevertheless, it is the mode in which Windows users can create symbolic links in modern Windows without requiring an elevated process (which would increase the security risk a lot more than Developer Mode). In any case, the mode I introduce here, and which I propose to promote to eventually be the default, retains backwards-compatibility in a fashion, by falling back to MSYS2's current behavior if symbolic links cannot be created. |
I am thinking, would it make sense to try to generalize this? maybe something like |
Can you think of any use case where a different config/order would be preferred by users? |
Not necessarily, it's just now we have a lot of seemingly-poorly-defined fallback cases:
I guess now that I write it out, that's not too bad, everything that falls back eventually falls back to sysfile. Perhaps deepcopy should fall back to |
@@ -2154,6 +2158,12 @@ symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice) | |||
__seterrno (); | |||
__leave; | |||
} | |||
/* With deepcopy fall back? Let's do that, then */ | |||
if (res == -1 && wsym_type == WSYM_native_or_deepcopy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that if target isspecial
res will be -2
, so will still fall back to default. This is probably reasonable, since deepcopy would just fall back to sysfile in that case. default will let it try wsl before falling back to sysfile. I'm thinking it might be best to make deepcopy fall back to default instead of sysfile anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
making deepcopy fall back to wsl instead of sysfile breaks one git test which tries to commit a symlink to /dev/null. The result of doing so is almost certainly not helpful (I bet it commits the sysfile, not a symlink), but when it's a wsl symlink it fails with Function not implemented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when it's a wsl symlink it fails with
Function not implemented
That's my fault, as I have not managed to spend time on upstreaming Git for Windows' symlink support: git/git@master...dscho:git:support-symlinks-on-windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a quick look at those changes, and I didn't see any handling of wsl symlinks in there, so I don't think that would help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. I misremembered. A git grep IO_REPARSE_TAG_LX_SYMLINK
indeed comes up empty...
My rationale was: deepcopy exists only as a Win32-compatible fall-back if symlink support does not exist, and symlink support predates WSL (and both require Developer Mode to be enabled IIRC). But then, this entire PR is about moving off of deepcopy in the first place, preparing for a time when MSYS2 drops support for Windows versions where symlinks can only be created when running elevated. FWIW @jeremyd2019 I really like the refactoring you did, moving the deepcopy code into its own function, I'd just suggest to do it as a preparatory, separate commit instead of squashing it in. |
Not to derail into something off-topic - not sure if you're saying WSL2 requires Developer Mode - it doesn't. Probably easier to get WSL2 to a level that meets enterprise security than Developer Mode, altho that's no simple thing either. There's been a trend for tools to just use junctions instead since they don't require admin privileges - you can see a bit of context at kubernetes-sigs/krew#843 - obv junctions just link a folder, but in many cases that can suffice |
@jcrben It had been a trend, but thankfully it's no longer all that common. See e.g. this insightful comment for an explanation. Here's an excerpt:
tl;dr Windows junctions are nowhere close to being a valid symlink replacement. |
@dscho well, in some ways using junctions is becoming more common - fnm switched to junctions in 2021, volta just did in 2024, jbangdev did it in 2024 at my urging, kubectl plugin manager krew may also do it. avoiding a complex and scary step of enabling Developer Mode is pretty nice. wish we could get symlinks on Windows without enabling Developer Mode. See also: Why do you have to be an admin to create a symlink in Windows? |
It surprises new MSYS2 users no end that
ln -s
does not create symbolic links at all, but deep copies (and with an exit code indicating success!). This did not at all match the expectations of those users who were familiar with Unix' concept of symbolic links and thought that they could rely on MSYS2 providing those, too, or fail with a non-zero exit code.Historical reasons are at play here: When MSYS2 was started (or was that already the behavior of MSys? I forget...), symbolic links were not supported on Windows, at least not really: you had to have administrator privileges to create them (but not to delete them... 🤷) in Windows Vista, and before that, Windows simply had no idea about symbolic links.
So what about Cygwin? Well, Cygwin had something like support for symbolic links, using
.lnk
files for the emulation. The only problem? You had to stay within Cygwin's walled garden to make use of them. All non-Cygwin applications would react with a less or more unpleasant "huh?!?" when encountering those "symbolic links".That's why MSYS2 chose to deep-copy by default. At least that way
./configure
would still work for those projects that required symbolic links. This was instrumental in getting MSYS2's package ecosystem off the ground.Even when a Windows 10 update introduced support for creating symbolic links without elevation as long as Windows was run in Developer Mode, the created symbolic links are not completely what Unix/Linux/macOS users may be used to, as Windows discerns between directory symlinks and file symlinks.
Be that as it may, now that we've dropped Windows 7 and Windows 8 support, it may be a good time to start switching the default to creating actual symbolic links by default.
Since we still support Windows 8.1 (and a couple of Windows 10 versions that do not allow creating symbolic links in non-elevated operations, even in Developer Mode), we cannot simply switch to a mode where the MSYS2 runtime creates symbolic links when asked for, but we have to have a mode where the MSYS2 runtime first checks whether that is possible with the Windows version on which it is running, and if not, falls back to the deep-copy.
This PR does precisely that: implement that mode, but does not yet flip the default away from
deepcopy
. The reason is that I want this to be tested by volunteers (myself included) first, and once it is deemed robust and stable enough, flip the default tonativeordeepcopy
.This addresses #113.