Re: [PATCH] t/lib-terminal: make TTY a lazy prerequisite

From: Jeff King <peff@peff.net>
Date: Mar 15, 2014, 01:55 UTC
Message-ID: <20140315015529.GB9979@sigill.intra.peff.net>
In-Reply-To: <xmqqpplofnba.fsf@gitster.dls.corp.google.com>

On Fri, Mar 14, 2014 at 03:05:45PM -0700, Junio C Hamano wrote:

Show 12 quoted lines

> > Actually, we don't need to do this, as of 94221d2 (t: use perl instead
> > of "$PERL_PATH" where applicable, 2013-10-28). If only the author of
> > that commit were here to correct me...
> 
> Yuck. I forgot all about that, too.
> 
> I wonder if that commit (actually the one before it) invites subtle
> bugs by tempting us to say
> 
> 	sane_unset VAR &&
> 	VAR=VAL perl -e 0 &&
>         test "${VAR+isset}" != "isset"

I dunno. A more subtle case is:

  write_script foo <<-\EOF
  perl ...
  EOF

which uses the real "perl" and not the function. So it's not as airtight as I would like, but I think it may be a net win, as the common case can just use "perl".

Hmph. It seems like I raised both of those concerns initially:

  http://article.gmane.org/gmane.comp.version-control.git/236879

We can revisit it if you want. I think the only options besides leaving it or reverting it would be to put "perl" into bin-wrappers as a wrapper script. That's fine for the tests, but I suspect it might annoy people who use bin-wrappers to run git straight out of the build directory without installing.

Show 5 quoted lines

> > -- >8 --
> > Subject: t/lib-terminal: make TTY a lazy prerequisite
> [...]
> 
> Thanks.

By the way, I checked for other cases that could use the same treatment by grepping for test_expect_* in t/lib-*.sh. Most of them are inside functions, so presumably the scripts call them at the appropriate time.

The exceptions are:

  1. lib-read-tree-m-3way.sh; this one has a whole battery of tests
     that sourced into t1000 and t4002. It could be split into functions
     and modernized, but it's probably not worth the effort. It's not
     causing ordering problems, and it's not likely to get used
     elsewhere.

  2. lib-pager.sh; this one is weird, as it is really about setting the
     "$less" variable to git's default pager. And then the prereq is
     really just checking that said pager is syntactically simple, I
     think, so we can override it by writing to a file with the same
     name. At least that's my impression; frankly I found it a bit
     confusing to read.

     Converting it to a lazy prereq wouldn't work because we care about
     its side effect of setting the "less" variable.  There are no
     ordering issues with it currently, so I'm inclined to leave it.

-Peff