git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.39k stars 2.55k forks source link

git bash should error on 'ln -s' if it is not enabled #3122

Open ernstklrb opened 3 years ago

ernstklrb commented 3 years ago

Setup

$ git --version --build-options
git version 2.28.0.windows.1
cpu: x86_64
built from commit: 77982caf269b7ee713a76da2bcf260c34d3bf7a7
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh

Microsoft Windows [Version 10.0.18363.1379]


 - What options did you set as part of the installation? Or did you choose the
   defaults? I do not know - it is installed by Admin.

One of the following:

type "C:\Program Files\Git\etc\install-options.txt" type "C:\Program Files (x86)\Git\etc\install-options.txt" type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt" $ cat /etc/install-options.txt

insert your machine's response here



 - Any other interesting things about your environment that might be related
   to the issue you're seeing?
none

### Details

 - Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

This report is about 'git bash' itself, not about git.

- What I see

Apparently, when creating symlinks is not enabled at install time, git bash will resort to copying the source to the target instead., if a `ln -s source target` command is issued. This is what I see from issuing `stat` on the source and the result.

- What I would like to see

In case symlinking has not been enabled, `ln -s` **should stop with an error** (instead of resorting to a semantically incompatible alternative behavior).
dscho commented 3 years ago

@ernstklrb you have a good point. MSYS2's Bash (which we use in Git for Windows) does not create symlinks by default.

However, there is a "knob" you can turn: setting the environment variable MSYS=winsymlinks:nativestrict (see Cygwin's documentation, MSYS is the MSYS2 equivalent of CYGWIN).

Now, we already have prior art for modifying the MSYS variable when launching Git Bash: https://github.com/git-for-windows/build-extra/commit/b73b4e360c41ea6d7a748f2df6c071df7a068e7e

Essentially, you can add the line MSYS=winsymlinks:nativestrict to /etc/git-bash.config (or modify an existing line setting MSYS, multiple options can be specified, separated by blanks).

Could you try that (as administrator, because /etc/git-bash.config is actually C:\Program Files\Git\etc\git-bash.config) and see whether that works as intended?

If so, we can add that setting automatically when support for symbolic links is enabled in Git for Windows' installer.

ernstklrb commented 3 years ago

I have tested with a fresh installation of git for windows, with "symbolic links" enabled in the installer. I get the following behavior:

Maybe a better message would have been "Operation not possible", but I guess that is outside of your scope

ernstklrb commented 3 years ago

Now I did a second test, again with a fresh installation (after uninstallation and completely removing c:\Program Files\Git).

This time without enabling symbolic links.

I get the same behavior as in the "with symlinks" case.

So I think you either need to add the MSYS-line in both cases (with and without symlinks enabled), or maybe just in this second case without symlinks enabled.

But it is not sufficient to do it just in the "enabled" case you referred to in your post.

dscho commented 3 years ago

ln: failed to create symbolic link 'c.txt': Operation not permitted

Can you verify that mklink can create symlinks in your setup (this should be the case if you enabled Developer Mode)?

dscho commented 3 years ago

I get the same behavior as in the "with symlinks" case.

Wait, I am confused. What was the "with symlinks" case? Or better: what was the behavior?

So I think you either need to add the MSYS-line in both cases (with and without symlinks enabled), or maybe just in this second case without symlinks enabled.

I think you're right on that. Unless we can guarantee that git-bash.config is removed (and potentially recreated) when reinstalling.

But that's not what I am trying to work out right now. I am trying to work out how to get Git Bash to expose the ln -s behavior that you want.

rimrul commented 3 years ago

Wait, I am confused. What was the "with symlinks" case?

If I undertand it correctly they're just saying ln -s behaves the same, wether you enable symlinks in the installer or not, it only depends on the MSYS environment variable.

dscho commented 3 years ago

Wait, I am confused. What was the "with symlinks" case?

If I undertand it correctly they're just saying ln -s behaves the same, wether you enable symlinks in the installer or not, it only depends on the MSYS environment variable.

Yes, that would make sense because Bash does not care about Git's config, nor does the MSYS2 runtime.

ernstklrb commented 3 years ago

ln: failed to create symbolic link 'c.txt': Operation not permitted

Can you verify that mklink can create symlinks in your setup (this should be the case if you enabled Developer Mode)?

So when I tried "MSYS=winsymlinks:nativestrict" first, I did not have permissions for doing mklink. This resulted in 'ln -s' erroring out, like I would expect.

Now, with permissions for mklink enabled as per your wiki entry, the behavior has changed: 'ln -s' in git bash will create a symlink.

dscho commented 3 years ago

Now, with permissions for mklink enabled as per your wiki entry, the behavior has changed: 'ln -s' in git bash will create a symlink.

Ouch. We should have edited https://github.com/git-for-windows/git/wiki/Symbolic-Links#allowing-non-administrators-to-create-symbolic-links a long time ago, I think, to prominently encourage Developer Mode, and only as a fall-back to do the permission dance. Unfortunately, I lack the time to edit this page...

ernstklrb commented 3 years ago

Improving the documentation reg. symbolic links might be good. Regardless, I think it is worth improving the default behavior in the non-developer-mode case to not silently copy instead of symlinking. Because

dscho commented 3 years ago

Improving the documentation reg. symbolic links might be good.

Would you be so kind?

dscho commented 3 years ago

I think it is worth improving the default behavior in the non-developer-mode case to not silently copy instead of symlinking.

That's a decision for the MSYS2 project, though...

ernstklrb commented 3 years ago

I think it is worth improving the default behavior in the non-developer-mode case to not silently copy instead of symlinking.

That's a decision for the MSYS2 project, though...

For git-bash, I guess, the git-for-windows project can decide how to handle the symlink issue.

I suggest you add MSYS=winsymlinks:nativestrict to /etc/git-bash.config on installation. (At least when /etc/git-bash.config does not yet exist.) Rather provoke a clear error message and avoid bad surprises later with "symlinks" that happen to be copies.

ernstklrb commented 3 years ago

Improving the documentation reg. symbolic links might be good.

Would you be so kind?

Maybe. I do not even understand the first paragraph: What is "Developer Mode", what is "symlink emulation", is "core.symlinks=true" a git config item? I only ever use symlinks in the shell "by hand". I do not track symlinks with git (which is a much more complex case in the context of git-for-windows I suspect).

dscho commented 3 years ago

I suggest you add MSYS=winsymlinks:nativestrict to /etc/git-bash.config on installation. (At least when /etc/git-bash.config does not yet exist.) Rather provoke a clear error message and avoid bad surprises later with "symlinks" that happen to be copies.

I'd rather only do that when the "Enable symbolic link support" checkbox is checked. And then only once I am sure that this works as intended.

I do not even understand the first paragraph: What is "Developer Mode",

You can switch on Developer Mode in Windows 10 (which is also a prerequisite to using WSL, if I remember correctly). Here is a link: https://www.howtogeek.com/292914/WHAT-IS-DEVELOPER-MODE-IN-WINDOWS-10/. Maybe you can find a more authoritative one, i.e. something official from Microsoft? It would be good to have a link in that wiki page.

what is "symlink emulation",

That is probably a left-over from olden times. Cygwin has a way to emulate symbolic links, but only Cygwin can interpret those.

is "core.symlinks=true" a git config item?

Yes.

I only ever use symlinks in the shell "by hand". I do not track symlinks with git (which is a much more complex case in the context of git-for-windows I suspect).

Git's source code itself has a symbolic link it tracks: RelNotes: https://github.com/git/git/blob/v2.31.0/RelNotes

But yeah, I agree, it is not a common thing on Windows.

ernstklrb commented 3 years ago

I suggest you add MSYS=winsymlinks:nativestrict to /etc/git-bash.config on installation. (At least when /etc/git-bash.config does not yet exist.) Rather provoke a clear error message and avoid bad surprises later with "symlinks" that happen to be copies.

I'd rather only do that when the "Enable symbolic link support" checkbox is checked. And then only once I am sure that this works as intended.

sure this should only be done after you are sure that this works as intended (on the prevalent versions of windows).

Still my main concern in this issue is that "unexpected" / "surprising" behavior of git bash should be avoided.

To me as a mostly linux user, silently getting a copy when issuing a ln -s command is surprising. And it is prone to lead to subtle problems if not noticed by the user immediately.

Apparently this only happens when neither in "Developer Mode", nor the user has the SeCreateSymbolicLinkPrivilege privilege.
Yet this is a relevant case, in particular to the user not educated about these things.
And to the user with no admin priviledge on their machine.

This is why I argue for "MSYS=winsymlinks:nativestrict" as a general default: it will avoid the surprise.

ernstklrb commented 3 years ago

After reading a bit about symlinks on windows (as a linux user), and looking at the current wiki text reg. symlinks, I have the impression that the interaction between (a) permissions of the windows user, (b) installation option "Enable symbolic link support", and (c) the git config item "core.symlinks"
makes the behavior of git-for-windows quite complex. Some of the complexity is possibly due to historic reasons (from a time when there were no (more or less) proper symlinks available on windows).

One specific problem here is that checking (b) actually does not enable symlink support, as long as (a) is not given.

How about removing (b) altogether, and instead resorting to notifying the user at runtime in case an attempt to create a symlink fails?

dscho commented 3 years ago

Still my main concern in this issue is that "unexpected" / "surprising" behavior of git bash should be avoided.

I understand that. And you can make a case for that at the project whose authority it is to change that behavior: MSYS2.

This is why I argue for "MSYS=winsymlinks:nativestrict" as a general default: it will avoid the surprise.

For you. For existing users who got used to the current behavior, you are causing surprise. And with over 3m downloads, I would be surprised if the number of those users were negligible.

the interaction [...] makes the behavior of git-for-windows quite complex.

I fully agree.

How about removing (b) altogether, and instead resorting to notifying the user at runtime in case an attempt to create a symlink fails?

I think that would be too late. At least in the installer, users have a chance to notice that their symlink support is disabled, and they can then go and enable Developer Mode before cloning and ending up with plain files (because Git does not actually copy the symlinked files, instead, when core.symlinks = false, it checks out files with the target path as contents.

Klaim commented 2 years ago

For you. For existing users who got used to the current behavior, you are causing surprise. And with over 3m downloads, I would be surprised if the number of those users were negligible.

I think a new option in the installer would help clarify that you can enable this at install time, and previous users will be informed and chose to enable it or not.

Note that I'm digging up this issue because as a build2 user and in particular when packaging projects using other buildsystems/package-managers I have to work on Windows with a ton of cloned git projects which use a lot of symlinks. Through the years I had a lot of trouble with the manipulations of symlinks on Windows mainly because git-on-windows was "hiding" the issue. Now I have some notes somewhere to check for when I have these issues, which basically are summarized here, and in build2's doc + the windows Developer mode to activate + the MSYs option to enable.

I also realized that even when you setup all this, cloning a repository which was created by people on linux and cloning on windows will not lead to symlink to folders to properly work, while recreating them manually locally will. This one is weird and I didn't find the exact reason yet so I'll report it another time.

All that to point that symlink working correctly is quite important, at least for package managers which work on windows.

dscho commented 2 years ago

I think a new option in the installer would help clarify that you can enable this at install time, and previous users will be informed and chose to enable it or not.

Sure, that makes sense. You could piggy-back on top of the work that went into the pseudo console support code and ensure that winsymlinks:nativestrict is written into /etc/git-bash.config.

However, that would only affect Git Bash users. Everybody else would call git.exe directly, and the shell script parts of Git would then not have that option turned on.

It is a bit tricky to get this right because we cannot simply set environment variables in Git's system config.

jcrben commented 1 year ago

@dscho

You can switch on Developer Mode in Windows 10 (which is also a prerequisite to using WSL, if I remember correctly)

You haven't needed it since 2017.

Security departments are probably still leery of enabling Developer Mode.

dscho commented 1 year ago

@jcrben could you back up that claim by a link to authoritative documentation as to which Windows version should no longer require Developer Mode?

jcrben commented 1 year ago

@dscho yep here's the blog post from Microsoft in 2017: https://devblogs.microsoft.com/commandline/developer-mode-no-longer-required-for-windows-subsystem-for-linux/

rimrul commented 1 year ago

@jcrben that post is about developer mode no longer being required for WSL, though and has no mention of Symlinks.

jcrben commented 1 year ago

@rimrul yep, you need Developer Mode for symlinks. I was responding to the comment that Developer Mode "is also a prerequisite to using WSL".

My point is that you shouldn't assume Developer Mode and therefore you shouldn't assume symlinks. Developer Mode is probably not a common thing on Windows workstations even for developers inside highly-secured corporate environments.

It's perhaps a bit easier to access to WSL than symlinks natively inside Windows.