Junio C HamanoDaniel Smith <dansmith65@gmail.com> writes:
Show 14 quoted lines
> 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"
Show 7 quoted lines
> -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...
Show 8 quoted lines
> + 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 || failedwhere 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 ..."
}Show 6 quoted lines
> + 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.
