On Mon, Oct 28, 2013 at 02:04:20PM -0700, 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. :)

One answer to "is there any reason..." is "it will loop infinitely if
you set PERL_PATH=perl". :) However, we can work around that with
"command".

It also may cause problems due to the way one-shot variables are treated
when calling a function versus a command, but we do not seem to set any
variables for invocations perl (and I do not envision it happening
often).

And finally, the other reason I can think of is that we can't apply it
consistently. It only helps where a shell function would activate, which
makes the end result potentially more confusing (especially to somebody
who does not really grok shells and subprocesses). Still, it does not
introduce any _new_ cases that need it, but only helps with a subset of
the cases. So in that sense it is a strict improvement, as we can let
most uses go, but catch only the trickier cases in review.

So I'm on the fence on whether it is a good idea or not, but I wrote up
the patches to play with it. I also noticed that we do not consistently
use $PERL_PATH in some of the built scripts, so I included that fix,
too.

Note that I do not have a system with a broken perl. I simulated a very
broken perl, which is how I found all of the spots to fix. But whether
they are actual bugs that would trigger due to a Windows perl that
handles CRLF differently, I have no clue.

  [1/3]: use @@PERL@@ in built scripts
  [2/3]: t: provide a perl() function which uses $PERL_PATH
  [3/3]: t: use perl instead of "$PERL_PATH" where applicable

-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

Reply via email to