Re: [PATCH] t9100: fix breakage when SHELL_PATH is not /bin/sh

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Mon, 8 Feb 2016, Jeff King wrote:

> On Mon, Feb 08, 2016 at 08:31:54PM +0100, Johannes Schindelin wrote:
> 
> > On Mon, 8 Feb 2016, Jeff King wrote:
> > 
> > > Assuming your patch works on Windows
> > 
> > If it re-introduces that chmod +x, it won't.
> > 
> > Please note that my *original* patch actually only guarded the chmod +x,
> > but Junio suggested switching to write_script and since it passed the test
> > suite here, I though it would be safe.
> > 
> > I still think write_script is the better alternative.
> 
> I'm confused why it matters. write_script() unconditionally calls "chmod
> +x", doesn't it?

Hmpf, you're right. I'll check tomorrow what's going wrong.

> I just double-checked its definition in test-lib-function.sh; am I
> missing some Windows-specific magic that kicks in?

No Windows magic I know of. But actually, the patch could be simplified to

-- snip --
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 56acc1e..ee85cc7 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -30,7 +30,7 @@ test_expect_success \
 		echo "deep dir" >dir/a/b/c/d/e/file &&
 		mkdir bar &&
 		echo "zzz" >bar/zzz &&
-		write_script exec.sh </dev/null &&
+		write_script exec.sh /bin/sh </dev/null &&
 		svn_cmd import -m "import for git svn" . "$svnrepo" >/dev/null
 	) &&
 	rm -rf import &&
-- snap --

> > So why not just prefix it with `SHELL_PATH=/bin/sh`?
> 
> But then what is write_script buying us?

write_script is a semantically unambiguous way to specify what we *want*.
And it would allow us to handle chmod specifically for Windows *in one
place only*.

But as I said, I have to investigate what's going on.

Ciao,
Dscho