Issues with core.symlinks

[dra27]

The recent support for symbolic links is a great addition, though I've found two minor issues:

1. If core.symlinks is false, you get an error message adding a native symlink (i.e. created by mklink) from the working directory:

[master] C:\Scratch\Sym>git add Bar.txt
error: readlink("Bar.txt"): Function not implemented
error: unable to index file Bar.txt
fatal: adding files failed

Is that intentional behaviour? While core.symlinks = false means that virtual files should be created by Git, is it necessary for Git to pretend it doesn't know how to read native symbolic links?
2. If core.symlinks is true and the current user doesn't have SeCreateSymbolicLinkPrivilege (e.g. an administrator in an unelevated command prompt) then git checkout silently fails. Surely it should at least display a warning or preferably terminate with an error (as it would on a write failure, for example)? In both cases, the error could suggest changing core.symlinks to false although it's also possible to detect if the user could obtain the privilege by elevation and in that instance it would be a good second suggestion.

[dscho]

If core.symlinks is false, you get an error message adding a native symlink (i.e. created by mklink) from the working directory:

[master] C:\Scratch\Sym>git add Bar.txt
error: readlink("Bar.txt"): Function not implemented
error: unable to index file Bar.txt
fatal: adding files failed

Yes, this is intentional: core.symlinks does not quite enable symbolic links because Git's idea of symbolic links matches POSIX semantics exactly while our emulation does not. It is close, but not close enough to enable by default. For example, we cannot assume that plain developers have permission to create a symlink, as you pointed out in...

1. If core.symlinks is true and the current user doesn't have SeCreateSymbolicLinkPrivilege (e.g. an administrator in an unelevated command prompt) then git checkout silently fails. Surely it should at least display a warning or preferably terminate with an error (as it would on a write failure, for example)? In both cases, the error could suggest changing core.symlinks to false although it's also possible to detect if the user could obtain the privilege by elevation and in that instance it would be a good second suggestion.

Your suggestions make a lot of sense, but are you prepared to follow up and do something about them? In other words, would you please follow the advice in https://github.com/git-for-windows/git/wiki/How-to-participate#fix-bugs-or-add-features-in-the-git-code-itself and in https://github.com/git-for-windows/git/wiki/Building-Git?

[dra27]

If core.symlinks is false, you get an error message adding a native symlink (i.e. created by mklink) from the working directory:

[master] C:\Scratch\Sym>git add Bar.txt
error: readlink("Bar.txt"): Function not implemented
error: unable to index file Bar.txt
fatal: adding files failed```

Yes, this is intentional: core.symlinks does not quite enable symbolic links because Git's idea of symbolic links matches POSIX semantics exactly while our emulation does not.

I have recently been working on symlinks in another project (dra27/ocaml@eeece96 & dra27/ocaml@559747a) - is the semantics you refer to Windows' having separate directory and filename symlinks, or is there a further gotcha? (curiosity only!)

It is close, but not close enough to enable by default. For example, we cannot assume that plain developers have permission to create a symlink, as you pointed out in...

I can see why core.symlinks is not turned on for writing by default, but why does it ever have to be the case that Git refuses to read them (all users can read them, even if following them is fully disabled using fsutil behavior set SymlinkEvaluation, despite the unclear documentation of FILE_FLAG_BACKUP_SEMANTICS in MSDN)? The error message reads as though Git is in some way deficient, which it clearly isn't!

1.If core.symlinks is true and the current user doesn't have SeCreateSymbolicLinkPrivilege (e.g. an administrator in an unelevated command prompt) then git checkout silently fails. Surely it should at least display a warning or preferably terminate with an error (as it would on a write failure, for example)? In both cases, the error could suggest changing core.symlinks to false although it's also possible to detect if the user could obtain the privilege by elevation and in that instance it would be a good second suggestion.

Your suggestions make a lot of sense, but are you prepared to follow up and do something about them? In other words, would you please follow the advice in https://github.com/git-for-windows/git/wiki/How-to-participate#fix-bugs-or-add-features-in-the-git-code-itself and in https://github.com/git-for-windows/git/wiki/Building-Git?

Possibly - although probably not soon, alas! But it's helpful to know that the change would be considered useful, so I'll keep this on my ToDo list 😃

[dscho]

@dra27 I just invited you into the active team so I can assign this issue to you.

[dscho]

@dra27 did you have any chance to work on this?

[dscho]

@dra27 ping?

[kblees]

@dra27 Regarding your first point:

My utmost goal when adding symlink support was that with core.symlinks=false, git should behave exactly as before, so that we don't introduce regressions. The one instance where I deviated from this principle (stat() for symlinks failing with ELOOP instead of returning false data) already caused problems (#186).

So core.symlinks=false on Windows behaves as if symlinks weren't supported by the platform, with [l]stat() beeing a notable exception (as in earlier git versions).

[dscho]

@kblees I really appreciate your careful approach. IIRC we had a different approach before, and had to revert it wholesale because it caused real problems (#186 was a minor issue that was easily addressed, thanks to the excellent design of the core.symlinks support). If all Git for Windows contributions were that excellent, my job as maintainer would be as sweet as a lazy Sunday morning 😃

[dscho]

@dra27 everything okay with you?

[dra27]

@dscho - apologies, I missed the ping. Not had a chance to work on this yet - I'm afraid I really did mean "not soon". Should hopefully be able to get to this in late October.
@kblees - I completely see your point, but I think that the previous behaviour was a bug which should be fixed. The platform does support symlinks, so the assumption in Git that they didn't exist and an error message reading them is an error (but not, I hasten to add, an error in your patch to preserve that behaviour!)

[simonfox]

Not sure this is the best place for me to point this out, but I also have an issue with symlink'd folders that are specified in the .gitignore. The .gitignore lines for the symlinks aren't respected and the folders are shown as new items.

[dscho]

@simonfox please be precise what kind of "symlink'd folders" you talk about. The best way to be precise is to state explicitly the command you used to create the symlinks. This is unfortunately necessary because people refer to two, very different things as "symlinks" in Windows.

[simonfox]

@dscho sorry...
The symlinks are created as part of our build using gulp-symlink which uses nodejs fs.symlink function. gulp-symlink passes 'dir' for the value of the type parameter which I think creates a true symlink. Windows Explorer displays the folder as a shortcut...

[dscho]

The symlinks are created as part of our build using gulp-symlink which uses nodejs fs.symlink function.

You really had to send me on a wild goose chase to find out whether junction points are created or whether Vista-style symlinks requiring admin privileges are created instead, didn't you 😛

[dscho]

node.js parses the symlink type here, and it is used here. The dir type will be parsed into the UV_FS_SYMLINK_DIR flag which in turn indeed creates a Vista style symlink.

Such a symlink can be created using the /d flag to the mklink executable.

And I cannot reproduce what you are claiming at all: as adminstrator, I created such a symlink in /usr/src/build-extra:

mklink /d a1 portable

A git status correctly reports this as an untracked file. Adding the line /a1 to the .gitgnore file changes that: now git status reports a modified .gitignore file, but no untracked a1 file anymore. In other words, my test contradicts your claim that symlinks are not ignored correctly after adding them to the .gitignore file.

Are you sure you want to continue to be parsimonious with your description of your problem? I did sink quite a bit into this issue with my research, so I am no longer willing to do all the bidding myself, and will require you to volunteer much more, including a Minimal, Complete & Verifiable Example if I am to spend any more time on assisting you to resolve your issues.

[simonfox]

@dscho again, please accept my apologies, it wasn't my intention to muck you around.

I have determined the issue is caused by a trailing slash in the .gitignore. The reason it has come up for me as the behavior is different between git 1.9.5 which is what I was using until I raised this.

In 1.9.5 using either a1 or a1/ in .gitignore would both ignore a dir symlink created using gulp-symlink (which from your investigation is a Vista style symlink). We were using trailing slashes in our .gitignore and so after upgrading to 2.6.2.windows.1 the ignore lines were not respected. I can confirm that by removing the trailing slash, the symlinks are ignored.

If you require any further information or if you would like me to try anything else please let me know.

[dscho]

In 1.9.5 using either a1 or a1/ in .gitignore would both ignore a dir symlink created using gulp-symlink (which from your investigation is a Vista style symlink)

That was because we did not support symlinks at all back then. So your symlink was mistaken for a directory, and the behavior you saw is consistent with that.

Symlinks are not directories, however, so adding their name with a trailing slash to .gitignore tells Git to ignore the contents of a directory of that name. Since there is no such directory, the entry is essentially a no-op.

You will see the same behavior if you gitignore, say, git-for-windows/ and then put a file of that name into the directory.