On Sun, Aug 16, 2015 at 05:08:56PM -0400, Andrew Dunstan wrote:
> On 08/16/2015 02:23 PM, Noah Misch wrote:
> >>-sub tapcheck
> >>+sub tap_check
> >> {
> >>- InstallTemp();
> >>+ die "Tap tests not enabled in configuration"
> >>+ unless $config->{tap_tests};
> >I endorse Heikki's argument for having omitted the configuration flag:
> >http://www.postgresql.org/message-id/55B90161.5090506@iki.fi
>
>
> That argument is not correct. None of the tap tests can be run via make if
> --enable-tap-tests is not set. This doesn't just apply to the check-world
> target as Heikki asserted. Have a look at the definitions of prove_check and
> prove_installcheck in src/Makefile.global.in if you don't believe me.
While that one piece of Heikki's argument was in error, I didn't feel that it
affected the conclusion. Please reply to the aforementioned email to dispute
the decision; some involved parties may not be following this thread.
> >>+ # Reset those values, they may have been changed by another test.
> >>+ # XXX is this true?
> >>+ local %ENV = %ENV;
> >> $ENV{PERL5LIB} = "$topdir/src/test/perl;$ENV{PERL5LIB}";
> >> $ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
> >>+ $ENV{TESTDIR} = "$dir";
> >The comment pertained only to the TESTDIR environment variable. Adding the
> >local %ENV is unnecessary. I think you can just delete the comment; there's
> >nothing noteworthy about setting a different TESTDIR value for each suite.
> >The PERL5LIB and PG_REGRESS updates need happen just once per bincheck, though
> >keeping them here seems good for the benefit of future TAP targets.
>
> The local decl is clearly needed. Otherwise repeated invocations of the
> function will result in repeated prepending of $topdir/src/test/perl to
> PERL5LIB. It's incredibly cheap anyway. And as you say, this is a much
> better place to do that than in bincheck. If you prefer, I could dispense
> with the local and instead only set to PERL5LIB conditionally. It's just a
> matter of style.
With the comment gone, the way you've done this section is fine.
