Re: [PATCH] git-new-workdir: add windows compatibility

Re: [PATCH] git-new-workdir: add windows compatibility

[Junio C Hamano]

Daniel Smith <dansmith65@gmail.com> writes:

> When running on Windows in MinGW, creating symbolic links via ln always
> failed.
>
> Using mklink instead of ln is the recommended method of creating links on
> Windows:
> http://stackoverflow.com/questions/18641864/git-bash-shell-fails-to-create-symbolic-links
>
> Script now branches on Windows to use mklink. This change should not affect
> unix systems.
>
> Signed-off-by: Daniel Smith <dansmith65@gmail.com>
>
> Has been tested on Windows 8.1 and OS X Yosemite.
> ---

Swap the "Has been tested..." and "Signed-off-by:" lines.

I'll defer to Windows folks if "mklink" is a sensible thing to use
or not; I have no first-hand experience with Windows, but only heard
that links are for admin user only or something like that, so I want
to hear from people whose judgement on Windows matters I trust.

>
> +iswindows () {
> + [[ -n "$WINDIR" ]];
> +}

Please don't add unnecessary bash-isms.  We have kept this script
usable without stepping out of POSIX.

	test -n "$WINDIR"

> -git_dir=$(cd "$git_dir" && pwd) || exit 1
> +if iswindows
> +then
> + git_dir=$(cd "$git_dir"; cmd.exe /c cd) || exit 1
> +else
> + git_dir=$(cd "$git_dir" && pwd) || exit 1
> +fi

Indentation of lines inside a new block is done with one more level
of HT in our scripts, not with just one SP.

> - ln -s "$git_dir/$x" "$new_workdir/.git/$x" || failed
> + if iswindows
> + then

Move these into a helper shell function, starting from here...

> + if test -d "$git_dir/$x"
> + then
> + # create directory symbolic link
> + isdir="/d"
> + fi
> + # convert path separator to backslash
> + targetPath=$(sed -e 's#^J:##' -e 's#/#\\#g' <<< "$git_dir/$x")
> + cmd.exe /c "mklink $isdir \"$new_workdir/.git/$x\" \"$targetPath\"" || failed

... up to here.  Also a few points about these new lines:

 * Use indentation when doing nested if/then/if/then/fi/fi block,
   i.e.

	if isWindows
        then
		if test -d "..."
                then
			isdir=/d
                fi
                target=..
                cmd.exe /c ...
	fi

 * "<<<" is a bash-ism, isn't it?

 * Use of "#" as s/// separator, when slash is not involved, looks
   ugly and makes it harder to read.

 * Is "J:" drive something special (unlike C: or D: drives)?

 * Can computation of targetPath fail?  IOW, shouldn't that line end
   with &&?

 * Share || failed between this part and POSIX part, i.e.

	if isWindows
        then
		ln_s_win "$new_workdir" "$x"
	else
		ln -s "$git_dir/$x" "$new_workdir'.git/$x"
	fi || failed

   where ln_s_win would be the "helper shell function" I suggested.

	ln_s_win () {
		if test -d "$git_dir/$2"
		then
                	isdir=/d
		fi
                target=$(printf "%s" "$git_dir/$2" | sed -e "...") &&
		cmd.exe /c "mklink $isdir ..."
	}

> + else
> + ln -s "$git_dir/$x" "$new_workdir/.git/$x" || failed
> + fi
>  done
>
>  # commands below this are run in the context of the new workdir

Thanks.

Re: [PATCH] git-new-workdir: add windows compatibility

[Johannes Schindelin]

Hi,

On 2015-05-26 06:03, Junio C Hamano wrote:
> Daniel Smith <dansmith65@gmail.com> writes:
> 
>> When running on Windows in MinGW, creating symbolic links via ln always
>> failed.
>>
>> Using mklink instead of ln is the recommended method of creating links on
>> Windows:
>> http://stackoverflow.com/questions/18641864/git-bash-shell-fails-to-create-symbolic-links
>>
>> Script now branches on Windows to use mklink. This change should not affect
>> unix systems.
>>
>> Signed-off-by: Daniel Smith <dansmith65@gmail.com>
>>
>> Has been tested on Windows 8.1 and OS X Yosemite.
>> ---
> 
> Swap the "Has been tested..." and "Signed-off-by:" lines.
> 
> I'll defer to Windows folks if "mklink" is a sensible thing to use
> or not; I have no first-hand experience with Windows, but only heard
> that links are for admin user only or something like that, so I want
> to hear from people whose judgement on Windows matters I trust.

The biggest problem with `mklink` is that it is only supported on Windows Vista and later, while I really like to keep Windows XP support in Git for Windows.

That is why Karsten Blees' symlink support (emulated via NTFS reparse points) which I just merged into Git for Windows yesterday is so careful about everything.

But git-new-workdir is in `contrib/`, anyway, and not installed in Git for Windows by default, therefore it is less critical: interested parties will have to be a bit knowledgeable in any case.

So I am fine with it!

Ciao,
Dscho

Re: [PATCH] git-new-workdir: add windows compatibility

[Paul Smith]

On Tue, 2015-05-26 at 11:53 +0200, Johannes Schindelin wrote:
> The biggest problem with `mklink` is that it is only supported on
> Windows Vista and later, while I really like to keep Windows XP
> support in Git for Windows.

No, the biggest problem with mklink is that you have to have
administrative privileges to use it... from wikipedia:

http://en.wikipedia.org/wiki/NTFS_symbolic_link

> The default security settings in Windows Vista/Windows 7 disallow
> non-elevated administrators and all non-administrators from creating
> symbolic links. This behavior can be changed running "secpol.msc" the
> Local Security Policy management console (under: Security Settings
> \Local Policies\User Rights Assignment\Create symbolic links). It can
> be worked around by starting cmd.exe with Run as administrator option
> or the runas command.

Except even that is not so simple, as various StackOverflow questions
and answers will show (I have to run so I can't look them up now).  I
did try to get this to work a year or so ago, and although I'm in no way
a Windows person (so maybe someone else would have better luck) I
couldn't get it to work.

Re: [PATCH] git-new-workdir: add windows compatibility

[Johannes Schindelin]

Hi Paul,

On 2015-05-26 14:20, Paul Smith wrote:
> On Tue, 2015-05-26 at 11:53 +0200, Johannes Schindelin wrote:
>> The biggest problem with `mklink` is that it is only supported on
>> Windows Vista and later, while I really like to keep Windows XP
>> support in Git for Windows.
> 
> No, the biggest problem with mklink is that you have to have
> administrative privileges to use it... from wikipedia:
> 
> http://en.wikipedia.org/wiki/NTFS_symbolic_link

It is even worse than that, as you pointed out below: administrators *can* permit non-administrators to create and use of symbolic links.

However, from a maintainer's perspective (which is my role in this thread), the compatibility problem is an even worse problem than the permissions.

>> The default security settings in Windows Vista/Windows 7 disallow
>> non-elevated administrators and all non-administrators from creating
>> symbolic links. This behavior can be changed running "secpol.msc" the
>> Local Security Policy management console (under: Security Settings
>> \Local Policies\User Rights Assignment\Create symbolic links). It can
>> be worked around by starting cmd.exe with Run as administrator option
>> or the runas command.
> 
> Except even that is not so simple, as various StackOverflow questions
> and answers will show (I have to run so I can't look them up now).  I
> did try to get this to work a year or so ago, and although I'm in no way
> a Windows person (so maybe someone else would have better luck) I
> couldn't get it to work.

For what it is worth, I tried my hand a couple of years ago at the project to move git-new-workdir to use the `.git` *file* and alternates mechanisms, but that does not work because you really need a separate `.git/HEAD`.

Ciao,
Johannes

Re: [PATCH] git-new-workdir: add windows compatibility

[Michael J Gruber]

Johannes Schindelin venit, vidit, dixit 26.05.2015 14:35:
> Hi Paul,
> 
> On 2015-05-26 14:20, Paul Smith wrote:
>> On Tue, 2015-05-26 at 11:53 +0200, Johannes Schindelin wrote:
>>> The biggest problem with `mklink` is that it is only supported on
>>> Windows Vista and later, while I really like to keep Windows XP
>>> support in Git for Windows.
>>
>> No, the biggest problem with mklink is that you have to have
>> administrative privileges to use it... from wikipedia:
>>
>> http://en.wikipedia.org/wiki/NTFS_symbolic_link
> 
> It is even worse than that, as you pointed out below: administrators *can* permit non-administrators to create and use of symbolic links.
> 
> However, from a maintainer's perspective (which is my role in this thread), the compatibility problem is an even worse problem than the permissions.
> 
>>> The default security settings in Windows Vista/Windows 7 disallow
>>> non-elevated administrators and all non-administrators from creating
>>> symbolic links. This behavior can be changed running "secpol.msc" the
>>> Local Security Policy management console (under: Security Settings
>>> \Local Policies\User Rights Assignment\Create symbolic links). It can
>>> be worked around by starting cmd.exe with Run as administrator option
>>> or the runas command.
>>
>> Except even that is not so simple, as various StackOverflow questions
>> and answers will show (I have to run so I can't look them up now).  I
>> did try to get this to work a year or so ago, and although I'm in no way
>> a Windows person (so maybe someone else would have better luck) I
>> couldn't get it to work.
> 
> For what it is worth, I tried my hand a couple of years ago at the project to move git-new-workdir to use the `.git` *file* and alternates mechanisms, but that does not work because you really need a separate `.git/HEAD`.
> 
> Ciao,
> Johannes
> 

Isn't that basically the approach that "git checkout --to" is taking? Is
that one "Windows proof"? I've lost track of its status, though.

Michael

Re: [PATCH] git-new-workdir: add windows compatibility

[Duy Nguyen]

On Fri, May 29, 2015 at 4:53 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Isn't that basically the approach that "git checkout --to" is taking? Is
> that one "Windows proof"?

It should be.

> I've lost track of its status, though.

It should be fine to use as long as you don't do checkout --to on a
submodule. Some progress was made on that direction, but I was busy
with other series and then didn't have time for git for a while.
-- 
Duy

Re: [PATCH] git-new-workdir: add windows compatibility

[Johannes Schindelin]

Hi Michael,

On 2015-05-29 11:53, Michael J Gruber wrote:
> Johannes Schindelin venit, vidit, dixit 26.05.2015 14:35:
>> For what it is worth, I tried my hand a couple of years ago at the project to move git-new-workdir to use the `.git` *file* and alternates mechanisms, but that does not work because you really need a separate `.git/HEAD`.
> 
> Isn't that basically the approach that "git checkout --to" is taking? Is
> that one "Windows proof"? I've lost track of its status, though.

I haven't tried `git checkout --to` on Windows yet, and sadly I won't be able to do that for another week...

Ciao,
Dscho

Re: [PATCH] git-new-workdir: add windows compatibility

[Karsten Blees]

Am 26.05.2015 um 06:03 schrieb Junio C Hamano:
> Daniel Smith <dansmith65@gmail.com> writes:
> 
>> When running on Windows in MinGW, creating symbolic links via ln always
>> failed.
>>
>> Using mklink instead of ln is the recommended method of creating links on
>> Windows:
>> http://stackoverflow.com/questions/18641864/git-bash-shell-fails-to-create-symbolic-links
>>
> 
> I'll defer to Windows folks if "mklink" is a sensible thing to use
> or not; I have no first-hand experience with Windows, but only heard
> that links are for admin user only or something like that, so I want
> to hear from people whose judgement on Windows matters I trust.
> 

mklink:
 - is not available on Windows XP
 - requires special permissions
 - does not work on network shares (unless enabled via 'fsutil behavior')
 - only works on file systems that support reparse points (e.g. NTFS, not FAT)

AFAICT, the MSys2 symlink() implementation is pretty smart to detect these conditions and fall back to deep copy (aka 'cp -a') if symlinks are not supported.

IOW, using 'ln -s' will hopefully "just work" in the upcoming Git for Windows 2, thus trying to fix it for MSys1 / Git for Windows 1.9x is probably a lost cause.

Karsten

Re: [PATCH] git-new-workdir: add windows compatibility

[Johannes Schindelin]

Hi Daniel,

On 2015-05-26 19:16, Daniel Smith wrote:
> Thanks to everyone for reviewing my proposed patch an providing valuable
> feedback. This was my first patch submission to a large open source project
> like Git and the whole process was a little daunting.

Heh, yeah, it can be quite overwhelming (and this mailing list is very "male", too). Sorry!

I hope, though, that it was obvious we all only wanted to be helpful?

> On Tue, May 26, 2015 at 9:48 AM, Karsten Blees <karsten.blees@gmail.com>
> wrote:
> 
>> AFAICT, the MSys2 symlink() implementation is pretty smart to detect these
>> conditions and fall back to deep copy (aka 'cp -a') if symlinks are not
>> supported.
>>
>> IOW, using 'ln -s' will hopefully "just work" in the upcoming Git for
>> Windows 2, thus trying to fix it for MSys1 / Git for Windows 1.9x is
>> probably a lost cause.
>>
> 
> In that case, I'll abandon this patch and wait for Git for Windows 2 to
> come out.

Speaking of which: Could you try `git-new-workdir` with Git for Windows 2.x (developers' preview)? https://git-for-windows.github.io/#download

It *might* be necessary to define the `MSYS` environment variable to `winsymlinks:nativestrict` before starting the Git Bash, to enable symlinking. It would be really good to know if that is the case so that I can do something about this in the Git for Windows installer.

Ciao,
Johannes