Re: [PATCH] t/README: tests can use perl even with NO_PERL

2013-10-28 Thread Ben Walton
On Mon, Oct 28, 2013 at 9:04 PM, Jonathan Nieder  wrote:
> Jeff King wrote:
>
>> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
>> everywhere, and not simply do:
>>
>>   perl () {
>> "$PERL_PATH" "$@"
>>   }
>>
>> in test-lib.sh?
>
> Sounds like a nice potential improvement to me. :)

+1.

-Ben
-- 
---
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---
--
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] t/README: tests can use perl even with NO_PERL

2013-10-28 Thread Jonathan Nieder
Jeff King wrote:

> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
> everywhere, and not simply do:
>
>   perl () {
> "$PERL_PATH" "$@"
>   }
>
> in test-lib.sh?

Sounds like a nice potential improvement to me. :)

Thanks,
Jonathan
--
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] t/README: tests can use perl even with NO_PERL

2013-10-28 Thread Jeff King
On Mon, Oct 28, 2013 at 12:22:16PM -0700, Jonathan Nieder wrote:

> The git build system supports a NO_PERL switch to avoid installing
> perl bindings or other features (like "git add --patch") that rely on
> perl on runtime, but even with NO_PERL it has not been possible for a
> long time to run tests without perl.  Helpers such as
> 
>   nul_to_q () {
>   "$PERL_PATH" -pe 'y/\000/Q/'
>   }
> 
> use perl as a better tr or sed and are regularly used in tests without
> worrying to add a PERL prerequisite.
> 
> Perl is portable enough that it seems fine to keep relying on it for
> this kind of thing in tests (and more readable than the alternative of
> trying to find POSIXy equivalents).  Update the test documentation to
> clarify this.
> 
> Reported-by: Johannes Sixt 
> Signed-off-by: Jonathan Nieder 
> ---

Yeah, I think this accurately the conclusions we've come to informally
during review on the list (for a long time we did not even use
$PERL_PATH for such "vanilla" cases, but some people have a broken perl
in their PATH).

Your patch looks good, and I think Ben's patch does not need a PERL
prerequisite. However, it is supposed to use $PERL_PATH, which it does
not.

Speaking of which, is there any reason to use the ugly "$PERL_PATH"
everywhere, and not simply do:

  perl () {
"$PERL_PATH" "$@"
  }

in test-lib.sh?

-Peff
--
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] t/README: tests can use perl even with NO_PERL

2013-10-28 Thread Johannes Sixt
Am 28.10.2013 20:22, schrieb Jonathan Nieder:
> The git build system supports a NO_PERL switch to avoid installing
> perl bindings or other features (like "git add --patch") that rely on
> perl on runtime, but even with NO_PERL it has not been possible for a
> long time to run tests without perl.  Helpers such as
> 
>   nul_to_q () {
>   "$PERL_PATH" -pe 'y/\000/Q/'
>   }
> 
> use perl as a better tr or sed and are regularly used in tests without
> worrying to add a PERL prerequisite.
> 
> Perl is portable enough that it seems fine to keep relying on it for
> this kind of thing in tests (and more readable than the alternative of
> trying to find POSIXy equivalents).  Update the test documentation to
> clarify this.

Thank you for the clarification!

-- Hannes

--
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


[PATCH] t/README: tests can use perl even with NO_PERL

2013-10-28 Thread Jonathan Nieder
The git build system supports a NO_PERL switch to avoid installing
perl bindings or other features (like "git add --patch") that rely on
perl on runtime, but even with NO_PERL it has not been possible for a
long time to run tests without perl.  Helpers such as

nul_to_q () {
"$PERL_PATH" -pe 'y/\000/Q/'
}

use perl as a better tr or sed and are regularly used in tests without
worrying to add a PERL prerequisite.

Perl is portable enough that it seems fine to keep relying on it for
this kind of thing in tests (and more readable than the alternative of
trying to find POSIXy equivalents).  Update the test documentation to
clarify this.

Reported-by: Johannes Sixt 
Signed-off-by: Jonathan Nieder 
---
Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> + - PERL
>> +
>> +   Git wasn't compiled with NO_PERL=YesPlease.
>> +
>> +   Even without the PERL prerequisite, tests can assume there is a
>> +   usable perl interpreter at $PERL_PATH, though it need not be
>> +   particularly modern.
>> +
>> +   Wrap tests for commands implemented in Perl with this.
>
> Sounds like a sensible thing to address, but I first parsed it as
>
> Wrap (tests for (commands implemented in Perl)) with this.
>
> while I think the readers are expected to parse it as
>
>Wrap ((tests for commands) implemented in Perl) with this.

I meant the former --- that is, tests for commands like "git add -p"
should be skipped in a NO_PERL build.  Probably clearest to leave out
the third paragraph entirely, like this:

 t/README | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index 2167125..0a939eb 100644
--- a/t/README
+++ b/t/README
@@ -629,11 +629,18 @@ See the prereq argument to the test_* functions in the 
"Test harness
 library" section above and the "test_have_prereq" function for how to
 use these, and "test_set_prereq" for how to define your own.
 
- - PERL & PYTHON
+ - PYTHON
 
-   Git wasn't compiled with NO_PERL=YesPlease or
-   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
-   these.
+   Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
+   need Python with this.
+
+ - PERL
+
+   Git wasn't compiled with NO_PERL=YesPlease.
+
+   Even without the PERL prerequisite, tests can assume there is a
+   usable perl interpreter at $PERL_PATH, though it need not be
+   particularly modern.
 
  - POSIXPERM
 
-- 
1.8.4.1

--
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