From: Jeff King <peff@peff.net>
Date: Mon, 11 Apr 2016 17:32:25 GMT
Subject: Re: Hardcoded #!/bin/sh in t5532 causes problems on Solaris
Message-ID: <20160411173224.GE4011@sigill.intra.peff.net>
In-Reply-To: <xmqqmvp2ti20.fsf@gitster.mtv.corp.google.com>

On Sat, Apr 09, 2016 at 05:37:43PM -0700, Junio C Hamano wrote:

> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index b79f442..d96d0e4 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -555,10 +555,9 @@ test_expect_success 'rebase a detached HEAD' '
>  test_expect_success 'rebase a commit violating pre-commit' '
>  
>  	mkdir -p .git/hooks &&
> -	PRE_COMMIT=.git/hooks/pre-commit &&
> -	echo "#!/bin/sh" > $PRE_COMMIT &&
> -	echo "test -z \"\$(git diff --cached --check)\"" >> $PRE_COMMIT &&
> -	chmod a+x $PRE_COMMIT &&
> +	write_script .git/hooks/pre-commit <<-\EOF &&
> +	test -z "$(git diff --cached --check)"
> +	EOF

Looks good and is the minimal change. I kind of wonder if the example
would be more clear, though, as just:

  write_script .git/hooks/pre-commit <<-\EOF &&
  exit 1
  EOF
  echo whatever >file1 &&
  ...

I don't think we ever actually need the pre-commit check to pass, as we
simply override it with --no-verify. But I dunno. Maybe people find it
easier to read with a pseudo-realistic example (it took me a minute to
realize the trailing whitespace in the content was important).

It could also stand to clean up its hook with test_when_finished. The
next test resorts to "rm -rf" on the hooks directory at the beginning.
Yuck.

-Peff