GNU bug report logs - #8887
Automake patches for custom test drivers' support break coreutils testsuite (was: Re: [PATCH v4 1/3] parallel-tests: add auxiliary script 'pt-driver', refactor)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8887 in the body.
You can then email your comments to 8887 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: "Ralf Wildenhues" <Ralf.Wildenhues <at> gmx.de>
Cc: bug-coreutils <at> gnu.org, automake-patches <at> gnu.org
Subject: Automake patches for custom test drivers' support break coreutils
	testsuite (was: Re: [PATCH v4 1/3] parallel-tests: add
	auxiliary script 'pt-driver', refactor)
Date: Fri, 17 Jun 2011 23:09:22 +0200

[Adding bug-coreutils]

Reference:
 <http://lists.gnu.org/archive/html/automake-patches/2011-06/msg00093.html>

On Friday 17 June 2011, Ralf Wildenhues wrote:
> I generally like the direction this is taking.  The point of best
> separation between which code goes into Makefile.in and which into
> the driver scripts can be fine-tuned when we have more than one such
> script.
> 
> Actually, yes, before deciding on this for real I really do want to see
> a nontrivial other driver script.  There is no point in hardcoding
> too much in several driver scripts if it all needs to be the same
> anyway.
> 
> Please measure the time overhead your changes introduce into the current
> code, for a trivial testsuite (say, 50 tests running 'true'), and a
> nontrivial one like Automake's and one with faster tests.
>
I've tried the coreutils testsuite and ... Ouch!  That gets broken
by my patches :-(

That's due to the overly complex TESTS_ENVIRONMENT employed by
conreutils' tests/Makefile.am:

 TESTS_ENVIRONMENT = \
  . $(srcdir)/lang-default;                     \
  tmp__=$${TMPDIR-/tmp};                        \
  test -d "$$tmp__" && test -w "$$tmp__" || tmp__=.;    \
  . $(srcdir)/envvar-check;                     \
  TMPDIR=$$tmp__; export TMPDIR;                \
  shell_or_perl_() {                            \
    if grep '^\#!/usr/bin/perl' "$$1" > /dev/null; then                 \
      if $(PERL) -e 'use warnings' > /dev/null 2>&1; then               \
        grep '^\#!/usr/bin/perl -T' "$$1" > /dev/null && T_=T || T_=;   \
        $(PERL) -w$$T_ -I$(srcdir) -MCoreutils -MCuSkip                 \
              -M"CuTmpdir qw($$f)" -- "$$1";    \
      else                                      \
        echo 1>&2 "$$tst: configure did not find a usable version of Perl," \
          "so skipping this test";              \
        (exit 77);                              \
      fi;                                       \
    else                                        \
      $(SHELL) "$$1";                           \
    fi;                                         \
  };                                            \
  export                                        \
  VERSION='$(VERSION)'                          \
  LOCALE_FR='$(LOCALE_FR)'                      \
  LOCALE_FR_UTF8='$(LOCALE_FR_UTF8)'            \
  abs_top_builddir='$(abs_top_builddir)'        \
  abs_top_srcdir='$(abs_top_srcdir)'            \
  abs_srcdir='$(abs_srcdir)'                    \
  built_programs="`cat .built-programs`"        \
  host_os=$(host_os)                            \
  host_triplet='$(host_triplet)'                \
  srcdir='$(srcdir)'                            \
  top_srcdir='$(top_srcdir)'                    \
  CONFIG_HEADER='$(abs_top_builddir)/$(CONFIG_INCLUDE)' \
  CU_TEST_NAME=`basename '$(abs_srcdir)'`,`echo $$tst|sed 's,^\./,,;s,/,-,g'` \
  CC='$(CC)'                                    \
  AWK='$(AWK)'                                  \
  EGREP='$(EGREP)'                              \
  EXEEXT='$(EXEEXT)'                            \
  MAKE=$(MAKE)                                  \
  PACKAGE_BUGREPORT='$(PACKAGE_BUGREPORT)'      \
  PACKAGE_VERSION=$(PACKAGE_VERSION)            \
  PERL='$(PERL)'                                \
  PREFERABLY_POSIX_SHELL='$(PREFERABLY_POSIX_SHELL)' \
  REPLACE_GETCWD=$(REPLACE_GETCWD)              \
  ; test -d /usr/xpg4/bin && PATH='/usr/xpg4/bin$(PATH_SEPARATOR)'"$$PATH"; \
  PATH='$(abs_top_builddir)/src$(PATH_SEPARATOR)'"$$PATH" \
  ; shell_or_perl_ 9>&2

In order to work with the upcoming new Automake testsuite harness, coreutils
have two possibilities:
 1. move the `shell_or_perl_' subroutine's functionality into a real acript,
    and define the LOG_COMPILER to point to it; or
 2. add a `.pl' extension to the perl test scripts, and define PL_LOG_COMPILER
    appropriately (might be a little tricky, considering the hops that the
    `shell_or_perl_' subroutine goes through in order to get the flags and
    imports right).

I should have have an FSF copyright assignement in place for coreutils too,
so I can volounteer to write a fix for this situation, if no one wants to
beat me.

Regards,
  Stefano

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>, bug-coreutils <at> gnu.org,
	automake-patches <at> gnu.org
Subject: Re: Automake patches for custom test drivers' support break coreutils
	testsuite
Date: Sat, 18 Jun 2011 08:41:06 +0200

Stefano Lattarini wrote:
> [Adding bug-coreutils]
>
> Reference:
>  <http://lists.gnu.org/archive/html/automake-patches/2011-06/msg00093.html>
>
> On Friday 17 June 2011, Ralf Wildenhues wrote:
>> I generally like the direction this is taking.  The point of best
>> separation between which code goes into Makefile.in and which into
>> the driver scripts can be fine-tuned when we have more than one such
>> script.
>>
>> Actually, yes, before deciding on this for real I really do want to see
>> a nontrivial other driver script.  There is no point in hardcoding
>> too much in several driver scripts if it all needs to be the same
>> anyway.
>>
>> Please measure the time overhead your changes introduce into the current
>> code, for a trivial testsuite (say, 50 tests running 'true'), and a
>> nontrivial one like Automake's and one with faster tests.
>>
> I've tried the coreutils testsuite and ... Ouch!  That gets broken
> by my patches :-(

Thank you for trying with coreutils before committing them.

> That's due to the overly complex TESTS_ENVIRONMENT employed by
> conreutils' tests/Makefile.am:
>
>  TESTS_ENVIRONMENT = \
>   . $(srcdir)/lang-default;                     \
>   tmp__=$${TMPDIR-/tmp};                        \
>   test -d "$$tmp__" && test -w "$$tmp__" || tmp__=.;    \
>   . $(srcdir)/envvar-check;                     \
>   TMPDIR=$$tmp__; export TMPDIR;                \
>   shell_or_perl_() {                            \
>     if grep '^\#!/usr/bin/perl' "$$1" > /dev/null; then                 \
>       if $(PERL) -e 'use warnings' > /dev/null 2>&1; then               \
>         grep '^\#!/usr/bin/perl -T' "$$1" > /dev/null && T_=T || T_=;   \
>         $(PERL) -w$$T_ -I$(srcdir) -MCoreutils -MCuSkip                 \
>               -M"CuTmpdir qw($$f)" -- "$$1";    \
>       else                                      \
>         echo 1>&2 "$$tst: configure did not find a usable version of Perl," \
...
>
> In order to work with the upcoming new Automake testsuite harness, coreutils
> have two possibilities:
>  1. move the `shell_or_perl_' subroutine's functionality into a real acript,
>     and define the LOG_COMPILER to point to it; or
>  2. add a `.pl' extension to the perl test scripts, and define PL_LOG_COMPILER
>     appropriately (might be a little tricky, considering the hops that the
>     `shell_or_perl_' subroutine goes through in order to get the flags and
>     imports right).

1) sounds preferable.

> I should have have an FSF copyright assignement in place for coreutils too,

Confirmed.

> so I can volounteer to write a fix for this situation, if no one wants to
> beat me.

I  won't say "no" ;-)
Thanks for volunteering.

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>, bug-coreutils <at> gnu.org,
	automake-patches <at> gnu.org
Subject: Re: Automake patches for custom test drivers' support break coreutils
	testsuite
Date: Sat, 18 Jun 2011 10:23:42 +0200

On Saturday 18 June 2011, Jim Meyering wrote:
> Stefano Lattarini wrote:
> > [Adding bug-coreutils]
> >
> > Reference:
> >  <http://lists.gnu.org/archive/html/automake-patches/2011-06/msg00093.html>
> >
> > On Friday 17 June 2011, Ralf Wildenhues wrote:
> >> I generally like the direction this is taking.  The point of best
> >> separation between which code goes into Makefile.in and which into
> >> the driver scripts can be fine-tuned when we have more than one such
> >> script.
> >>
> >> Actually, yes, before deciding on this for real I really do want to see
> >> a nontrivial other driver script.  There is no point in hardcoding
> >> too much in several driver scripts if it all needs to be the same
> >> anyway.
> >>
> >> Please measure the time overhead your changes introduce into the current
> >> code, for a trivial testsuite (say, 50 tests running 'true'), and a
> >> nontrivial one like Automake's and one with faster tests.
> >>
> > I've tried the coreutils testsuite and ... Ouch!  That gets broken
> > by my patches :-(
> 
> Thank you for trying with coreutils before committing them.
>
Note that they'll be commmited to a dedicated branch before being merged
into master, so even if you're using the developement version of automake
with coreutils you should be safe even in case of other breakage (that
is, until the merge to master takes place).

> > That's due to the overly complex TESTS_ENVIRONMENT employed by
> > conreutils' tests/Makefile.am:
> >
> >  TESTS_ENVIRONMENT = \
> >   . $(srcdir)/lang-default;                     \
> >   tmp__=$${TMPDIR-/tmp};                        \
> >   test -d "$$tmp__" && test -w "$$tmp__" || tmp__=.;    \
> >   . $(srcdir)/envvar-check;                     \
> >   TMPDIR=$$tmp__; export TMPDIR;                \
> >   shell_or_perl_() {                            \
> >     if grep '^\#!/usr/bin/perl' "$$1" > /dev/null; then                 \
> >       if $(PERL) -e 'use warnings' > /dev/null 2>&1; then               \
> >         grep '^\#!/usr/bin/perl -T' "$$1" > /dev/null && T_=T || T_=;   \
> >         $(PERL) -w$$T_ -I$(srcdir) -MCoreutils -MCuSkip                 \
> >               -M"CuTmpdir qw($$f)" -- "$$1";    \
> >       else                                      \
> >         echo 1>&2 "$$tst: configure did not find a usable version of Perl," \
> ...
> >
> > In order to work with the upcoming new Automake testsuite harness, coreutils
> > have two possibilities:
> >  1. move the `shell_or_perl_' subroutine's functionality into a real acript,
> >     and define the LOG_COMPILER to point to it; or
> >  2. add a `.pl' extension to the perl test scripts, and define PL_LOG_COMPILER
> >     appropriately (might be a little tricky, considering the hops that the
> >     `shell_or_perl_' subroutine goes through in order to get the flags and
> >     imports right).
> 
> 1) sounds preferable.
>
But it has a serious drawback: the redirection `9>&2' placed at the end
of TESTS_ENVIRONMENT will be rendered useless by the final exec done
in the new `shell_or_perl' script (at least for with shells using the
`cloexec' flag on fds > 2); this will bring back the problems fixed by
commit `v8.12-82-g6b68745' :-(

So I now think we should go with solution (2).

> > I should have have an FSF copyright assignement in place for coreutils too,
> 
> Confirmed.
> 
> > so I can volounteer to write a fix for this situation, if no one wants to
> > beat me.
> 
> I  won't say "no" ;-)
> Thanks for volunteering.
> 

Regards,
  Stefano

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>, bug-coreutils <at> gnu.org,
	automake-patches <at> gnu.org
Subject: Re: Automake patches for custom test drivers' support break coreutils
	testsuite
Date: Sat, 18 Jun 2011 13:28:05 +0200

Stefano Lattarini wrote:
>> ...
>> >
>> > In order to work with the upcoming new Automake testsuite harness, coreutils
>> > have two possibilities:
>> >  1. move the `shell_or_perl_' subroutine's functionality into a real acript,
>> >     and define the LOG_COMPILER to point to it; or
>> >  2. add a `.pl' extension to the perl test scripts, and define PL_LOG_COMPILER
>> >     appropriately (might be a little tricky, considering the hops that the
>> >     `shell_or_perl_' subroutine goes through in order to get the flags and
>> >     imports right).
>>
>> 1) sounds preferable.
>>
> But it has a serious drawback: the redirection `9>&2' placed at the end
> of TESTS_ENVIRONMENT will be rendered useless by the final exec done
> in the new `shell_or_perl' script (at least for with shells using the
> `cloexec' flag on fds > 2); this will bring back the problems fixed by
> commit `v8.12-82-g6b68745' :-(
>
> So I now think we should go with solution (2).

Ok.

Message #19 received at 8887-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 8887-done <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] tests: make test runner a script, not a shell function
Date: Fri, 22 Jul 2011 23:49:42 +0200

Jim Meyering wrote:
> Stefano Lattarini wrote:
>> Subject: [PATCH 2/2] tests: avoid extra forks in the testsuite
>>
>> * tests/shell-or-perl: Prefer the `read' builtin over `grep' to
>> look at the shebang line of test scripts.  Since `read' is a
>> special builtin, it might abort the whole program upon failures,
>> so add extra sanity checks, verifying that the test script exists
>> and is readable, before trying to read from it.
...
>
> Thank you.  I pushed that as-is.

And am marking this as "done".

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.