diff mbox series

[1/2] t5410: use longer path for sample script

Message ID 20181024073705.GA31202@sigill.intra.peff.net (mailing list archive)
State New, archived
Headers show
Series run-command: fix Unix PATH lookup | expand

Commit Message

Jeff King Oct. 24, 2018, 7:37 a.m. UTC
t5410 creates a sample script "alternate-refs", and sets
core.alternateRefsCommand to just "alternate-refs". That
shouldn't work, as "." is not in our $PATH, and so we should
not find it.

However, due to a bug in run-command.c, we sometimes find it
anyway! Even more confusing, this bug is only in the
fork-based version of run-command. So the test passes on
Linux (etc), but fails on Windows.

In preparation for fixing the run-command bug, let's use a
more complete path here.

Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5410-receive-pack-alternates.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Johannes Schindelin Oct. 24, 2018, 8:53 a.m. UTC | #1
Hi Peff,

On Wed, 24 Oct 2018, Jeff King wrote:

> t5410 creates a sample script "alternate-refs", and sets
> core.alternateRefsCommand to just "alternate-refs". That
> shouldn't work, as "." is not in our $PATH, and so we should
> not find it.
> 
> However, due to a bug in run-command.c, we sometimes find it
> anyway! Even more confusing, this bug is only in the
> fork-based version of run-command. So the test passes on
> Linux (etc), but fails on Windows.
> 
> In preparation for fixing the run-command bug, let's use a
> more complete path here.
> 
> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Thank you for the fix! I can confirm that the patch works, and the commit
message is stellar, as per usual for your contributions.

BTW since this breaks every single one of our Continuous Builds on
Windows, I would be very much in favor of fast-tracking this to `master`.

Thanks,
Dscho

>  t/t5410-receive-pack-alternates.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh
> index 457c20c2a5..f00d0da860 100755
> --- a/t/t5410-receive-pack-alternates.sh
> +++ b/t/t5410-receive-pack-alternates.sh
> @@ -23,7 +23,7 @@ test_expect_success 'with core.alternateRefsCommand' '
>  			--format="%(objectname)" \
>  			refs/heads/public/
>  	EOF
> -	test_config -C fork core.alternateRefsCommand alternate-refs &&
> +	test_config -C fork core.alternateRefsCommand ./alternate-refs &&
>  	git rev-parse public/branch >expect &&
>  	printf "0000" | git receive-pack fork >actual &&
>  	extract_haves <actual >actual.haves &&
> -- 
> 2.19.1.1094.gd480080bf6
> 
>
Junio C Hamano Oct. 25, 2018, 1:15 a.m. UTC | #2
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Wed, 24 Oct 2018, Jeff King wrote:
>
>> t5410 creates a sample script "alternate-refs", and sets
>> core.alternateRefsCommand to just "alternate-refs". That
>> shouldn't work, as "." is not in our $PATH, and so we should
>> not find it.
>> 
>> However, due to a bug in run-command.c, we sometimes find it
>> anyway! Even more confusing, this bug is only in the
>> fork-based version of run-command. So the test passes on
>> Linux (etc), but fails on Windows.
>> 
>> In preparation for fixing the run-command bug, let's use a
>> more complete path here.
>> 
>> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> Signed-off-by: Jeff King <peff@peff.net>
>> ---
>
> Thank you for the fix! I can confirm that the patch works, and the commit
> message is stellar, as per usual for your contributions.
>
> BTW since this breaks every single one of our Continuous Builds on
> Windows, I would be very much in favor of fast-tracking this to `master`.
>
> Thanks,
> Dscho

I should note to the public that this one, and the companion patch
2/2, owe greatly to you and Peff's efforts.

Thanks, both.

>
>>  t/t5410-receive-pack-alternates.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh
>> index 457c20c2a5..f00d0da860 100755
>> --- a/t/t5410-receive-pack-alternates.sh
>> +++ b/t/t5410-receive-pack-alternates.sh
>> @@ -23,7 +23,7 @@ test_expect_success 'with core.alternateRefsCommand' '
>>  			--format="%(objectname)" \
>>  			refs/heads/public/
>>  	EOF
>> -	test_config -C fork core.alternateRefsCommand alternate-refs &&
>> +	test_config -C fork core.alternateRefsCommand ./alternate-refs &&
>>  	git rev-parse public/branch >expect &&
>>  	printf "0000" | git receive-pack fork >actual &&
>>  	extract_haves <actual >actual.haves &&
>> -- 
>> 2.19.1.1094.gd480080bf6
>> 
>>
diff mbox series

Patch

diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh
index 457c20c2a5..f00d0da860 100755
--- a/t/t5410-receive-pack-alternates.sh
+++ b/t/t5410-receive-pack-alternates.sh
@@ -23,7 +23,7 @@  test_expect_success 'with core.alternateRefsCommand' '
 			--format="%(objectname)" \
 			refs/heads/public/
 	EOF
-	test_config -C fork core.alternateRefsCommand alternate-refs &&
+	test_config -C fork core.alternateRefsCommand ./alternate-refs &&
 	git rev-parse public/branch >expect &&
 	printf "0000" | git receive-pack fork >actual &&
 	extract_haves <actual >actual.haves &&