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

2015-05-25 Thread Junio C Hamano
Daniel Smith  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 
>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-26 Thread 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-26 Thread 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-26 Thread Johannes Schindelin
Hi,

On 2015-05-26 06:03, Junio C Hamano wrote:
> Daniel Smith  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 
>>
>> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-26 Thread Karsten Blees
Am 26.05.2015 um 06:03 schrieb Junio C Hamano:
> Daniel Smith  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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-27 Thread 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 
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-29 Thread 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-29 Thread Duy Nguyen
On Fri, May 29, 2015 at 4:53 PM, Michael J Gruber
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-29 Thread 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html