Re: [PATCH 05/13] credential: gate new fields on capability

From: Jeff King <peff@peff.net>
Date: Mar 28, 2024, 10:20 UTC
Message-ID: <20240328102053.GA890906@coredump.intra.peff.net>
In-Reply-To: <20240324011301.1553072-6-sandals@crustytoothpaste.net>

On Sun, Mar 24, 2024 at 01:12:53AM +0000, brian m. carlson wrote:

Show 13 quoted lines

> @@ -35,6 +41,16 @@ test_expect_success 'setup helper scripts' '
>  	test -z "$pass" || echo password=$pass
>  	EOF
>  
> +	write_script git-credential-verbatim-cred <<-\EOF &&
> +	authtype=$1; shift
> +	credential=$1; shift
> +	. ./dump
> +	echo capability[]=authtype
> +	test -z "${capability##*authtype*}" || return
> +	test -z "$authtype" || echo authtype=$authtype
> +	test -z "$credential" || echo credential=$credential
> +	EOF

I think this "|| return" needs to be "|| exit 0" or similar. The Windows CI jobs fail with:

  --- a/expect-stderr
  +++ b/stderr
  @@ -2,3 +2,4 @@ verbatim-cred: get
   verbatim-cred: capability[]=authtype
   verbatim-cred: protocol=http
   verbatim-cred: host=example.com
  +D:\a\git\git\t\trash directory.t0300-credentials\git-credential-verbatim-cred: line 10: return: can only `return' from a function or sourced script

(actually if you count the line numbers, I think this particular case is the similar "|| return" added to the script later, but both should be fixed).

It doesn't show up elsewhere because only bash complains, but not dash. Even running the test script with bash isn't enough, because write_script uses $SHELL_PATH under the hood. But building with "make SHELL_PATH=/bin/bash test" shows the problem on other platforms.

-Peff