Skip to content
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

ln -s copies instead of links #171

Closed
dansmith65 opened this issue May 29, 2015 · 14 comments
Closed

ln -s copies instead of links #171

dansmith65 opened this issue May 29, 2015 · 14 comments

Comments

@dansmith65
Copy link

v2.4.2.windows.1 (64 bit, portable install, if that matters), run from an elevated command prompt on Windows 8.1

ln -s src dest will create a copy of dest and name it src

whereas

cmd.exe /c "mklink /d dest src" will create a directory junction

I'm mentioning this because my understanding is this version is supposed to be able to intelligently choose the best method of creating a symbolic link on Windows, but it doesn't seem to be doing that in my case.

@dansmith65
Copy link
Author

Running export MSYS=winsymlinks:nativestrict before creating the link seems to resolve the issue.

@dscho
Copy link
Member

dscho commented May 29, 2015

Good point! I guess we should set this environment variable (unless it is already set) in git-bash.exe, eh?

@dansmith65
Copy link
Author

Personally, that's the behavior I would expect. The only caveat is that it will fail with a permissions error if not run as an administrator.

@kblees
Copy link

kblees commented May 29, 2015

IMO symlinks have too many caveats on Windows to enable this by default. So users should enable this explicitly if they know what they are doing.

@dansmith65
Copy link
Author

Would etc\profile.d\env.sh be an appropriate place to define that value?

I assume that runs when git-bash.exe is run?

@kblees
Copy link

kblees commented May 29, 2015

What about cmd //c setx MSYS winsymlinks:nativestrict? I.e. store it permanently in you personal environment variables. Then you don't lose it if you update Git.

@dscho
Copy link
Member

dscho commented May 29, 2015

Personally, I would be in favor of adding this as a checkbox in the "experimental options" page of the installer. Not quite sure about the best way to implement it (and I'll only get a chance to work on this in more than a week from now, anyway, so feel free to come up with a Pull Request).

@nalla
Copy link

nalla commented Jun 1, 2015

BTW: The release notes clearly states how to enable the symlink support: git-for-windows/build-extra@1999a58#diff-b1c84271dad23463aa793956bdbbcb6b. So this is not really an issue.

Fiddling with the environment within the installer might be not such a good idea. What if someone wants to restrict symlink support for a specific set of repos? If we leave enabling of that feature to the developer, she can fine-tune the setting to her liking.

@dscho
Copy link
Member

dscho commented Jun 9, 2015

Personally, I would be in favor of adding this as a checkbox in the "experimental options" page of the installer. Not quite sure about the best way to implement it (and I'll only get a chance to work on this in more than a week from now, anyway, so feel free to come up with a Pull Request).

@dansmith65 BTW I kind of thought that you would take ownership of this issue. Would be a nice gesture if you looked at the wiki page how to make an installer, then at the commit adding the experimental options and then came up with a Pull Request that adds another check box (off by default) to set the MSYS environment variable appropriately.

Ideally, this environment variable would be set in /etc/profile.d/env.sh but you should make sure first that that works as expected, i.e. that MSYS can be set after Git Bash is started -- it could be too late because the msys2-runtime is already started at that point.

@kblees
Copy link

kblees commented Jun 9, 2015

I don't think this should be exposed as an installer option. Windows symlinks just have too many pitfalls to explain in an installer page. Its also not an "experimental" feature in the sense that these problems will disappear with enough testing.

IMO a Wiki page / FAQ entry would be the best option (similar to long paths).

@dscho
Copy link
Member

dscho commented Jun 9, 2015

IMO a Wiki page / FAQ entry would be the best option (similar to long paths).

Heh... If my experience taught me that people read the Wiki or the FAQ, I would agree. Wholeheartedly! Alas, my experience tells me that people do not read the Wiki...

So I am actually in favor of renaming the "experimental features" page and adding both the "symlink emulation" and "long paths" checkboxes (both off by default, of course). In the description we can always link to the Wiki...

@kblees
Copy link

kblees commented Jun 10, 2015

In contrast to fscache, symlinks neither work on all platforms nor for all users nor on all file systems. Exposing this in the installer gives blindfolded admins the option to shoot their users in the foot.

Until now I was under the impression that we need to set the MSYS variable to enable symlinks in scripted commands. However, this does not seem to be the case (none of git's scripted commands use ln -s). And symlink support in Git is enabled via the core.symlinks option.

Therefore I'm closing this issue as not git related. How to enable symlinks in MSYS has long been answered (even by the original poster). Requests to make winsymlinks:nativestrict the default should be directed to the MSYS or even CYGWIN project.

@kblees kblees closed this as completed Jun 10, 2015
@dscho
Copy link
Member

dscho commented Jun 10, 2015

In contrast to fscache, symlinks neither work on all platforms nor for all users nor on all file systems. Exposing this in the installer gives blindfolded admins the option to shoot their users in the foot.

Which is why the checkbox would be guarded by the condition that the Windows version must be new enough (and if it is easy to implement in InnoSetup, also the condition that the NTFS junction permissions are not limited to administrators). If either of those conditions would not be met, we would hide that checkbox.

none of git's scripted commands use ln -s

This is a really good point, thanks for mentioning it! I just verified that even subtree (which is in contrib/, but we bundle it) does not use ln -s. Only git new-workdir (in contrib/) would use it, but we do not bundle that.

Note, however, that git difftool as well as git svn do expect symbolic links to work just fine.

Having said all that, I am fine with leaving this ticket closed. If anybody is motivated to add that installer page, I would be happy to accept that contribution, of course ;-)

@dansmith65
Copy link
Author

@dscho Personally, I use the portable version, so I'm not inclined to write the code to add this option to the installer.

Knowing I can manually set the MSYS environment variable is good enough for me.

jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Aug 30, 2019
…reamable version

I'm creating this PR for a fourth time (see git-for-windows#163, git-for-windows#171, and git-for-windows#178 for earlier versions). This version is tracking my progress to create something that can be sent as an RFC upstream, but also can be used to start the sparse feature branch. This is based on v2.23.0.

Note: I currently have conflicts with the virtual filesystem feature, and I'll resolve those with a merge commit when I'm ready. I'm just creating this for tracking progress at the moment, but can also be a place for early feedback.

* git sparse-checkout disable to disable the sparse-checkout feature and return to a full checkout.
* git sparse-checkout init --cone, to initialize in cone mode.
* git sparse-checkout add when core.sparseCheckout=cone.

This includes performance improvements with the hashset-based matching algorithm, but I'll leave further enhancements as smaller steps on top. Here are a few things I want to try:

Track the maximum depth of a prefix pattern, so we know to not run hashes for deeper paths.
Use the "known exclude" bit in cone mode to stop hashing paths we know will not be included.
Use the "known include" bit in cone mode to stop hashing paths we know will be included. This is more difficult than "known exclude" because we need to distinguish directories from files when doing path matches so we don't give a directory a "known include" when it isn't a recursive pattern match.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants