Re: [PATCH 12/19] mingw: do not use symlinks with SVN in t9100

2016-01-25 Thread Johannes Schindelin
Hi Junio,

On Sun, 24 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > From: Karsten Blees 
> >
> > The SVN library does not seem to support symlinks, even if symlinks are
> > enabled in MSYS2 and Git. Use 'cp' instead of 'ln -s'.
> >
> > This partially fixes t/t9100-git-svn-basic.sh
> >
> > Signed-off-by: Karsten Blees 
> > Signed-off-by: Johannes Schindelin 
> > ---
> >  t/t9100-git-svn-basic.sh | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> For the purpose of SVN test, is it important that foo.link is a link
> to foo?  I am wondering what would be the fallout from making this
> change without "only on MINGW do this".

(I originally sent the following response as a reply to 13/19 by mistake.)

Your comment made me inspect the entire t9100 again, wondering why things
work when we copy the contents instead of symlinking them. And you know
what, even if I could have sworn that I verified for every patch in this
series that it is actually necessary to pass the test suite, it is *not*
necessary.

So I backed it out and it won't be part of v2 anymore.

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 12/19] mingw: do not use symlinks with SVN in t9100

2016-01-24 Thread Junio C Hamano
Johannes Schindelin  writes:

> From: Karsten Blees 
>
> The SVN library does not seem to support symlinks, even if symlinks are
> enabled in MSYS2 and Git. Use 'cp' instead of 'ln -s'.
>
> This partially fixes t/t9100-git-svn-basic.sh
>
> Signed-off-by: Karsten Blees 
> Signed-off-by: Johannes Schindelin 
> ---
>  t/t9100-git-svn-basic.sh | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

For the purpose of SVN test, is it important that foo.link is a link
to foo?  I am wondering what would be the fallout from making this
change without "only on MINGW do this".

On the other hand, perhaps such a change may make this particular
test meaningless for whatever reason.  Perhaps this test is about
git-svn handling a symbolic link correctly, and you wouldn't be
testing anything useful if you unconditionally changed "ln -s" to
"cp" on all platforms.  But if that is the case, I suspect that it
would make more sense to mark the test to be skipped using the
prerequiste, instead of pretending that this test passes.  While the
updated test may be testing that the same "works correctly on
symbolic links" as before on all the other platforms, on MINGW, it
is testing something completely different.

I cannot quite tell if that is the case.

> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> index 258d9b8..dbb440b 100755
> --- a/t/t9100-git-svn-basic.sh
> +++ b/t/t9100-git-svn-basic.sh
> @@ -25,7 +25,14 @@ test_expect_success \
>   (
>   cd import &&
>   echo foo >foo &&
> - ln -s foo foo.link
> + if test_have_prereq !MINGW
> + then
> + ln -s foo foo.link
> + else
> + # MSYS libsvn does not support symlinks, so always use 
> cp, even if
> + # ln -s actually works
> + cp foo foo.link
> + fi
>   mkdir -p dir/a/b/c/d/e &&
>   echo "deep dir" >dir/a/b/c/d/e/file &&
>   mkdir bar &&
--
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