Re: [PATCH] Avoid passing autotest job fds to test groups.

2010-07-21 Thread Ralf Wildenhues
Hi Eric,

* Eric Blake wrote on Wed, Jul 21, 2010 at 02:51:34PM CEST:
 On 07/20/2010 10:21 PM, Ralf Wildenhues wrote:
  * lib/autotest/general.m4 (AT_INIT) Fifo job dispatcher: Remove
  commented closing of job output fd.  In serial test group driver,
  ensure $at_jobs is set to 1, so other parts of Autotest can
  reliably use this variable as indicator for parallel testing.
 
 The $at_jobs change seems independently useful, even if we aren't
 relying on it right now.  I also see how I can compress the testsuite a
 bit by adding another shell function; patch coming up shortly...

Thanks.

  * Eric Blake wrote on Wed, Jul 21, 2010 at 05:49:59AM CEST:
  Technically, the AT_CHECK commands only need worry about one fd;
  AT_JOB_FIFO_OUT_FD (in fact, my patch already clobbers AT_JOB_FIFO_IN_FD
  before the test is run),
  
  For all but the first child.
 
 But the first child is run in a context where AT_JOB_FIFO_IN_FD was not
 yet opened by the parent, and thus is not a problem (technically, I
 suppose that means you could have inherited a random fd 6 from whoever
 invoked the overall testsuite, but at least with that, you can't exploit
 'echo 6' to change the state of the parallel tests).

ACK.  Lack of coffee and all that.  ;-)

  +  if test $at_jobs -ne 1; then
  +if $at_first; then
  +  exec AT_JOB_FIFO_IN_FD-
  +fi
 
 So this part is not necessary.  But maybe we want to do a global:
 
 for i in 3 4 5 6 7; do
   eval exec $i-
 done

(why not just exec 5- 6- 7-?)

 up front, to close any random fds inherited from outside the testsuite,
 so that all tests are run with the same set of fds, rather than the
 first test inheriting more random fds than any other test?

We don't do anything with fds 3 and 4; either we consider them special,
then shouldn't close them for that historical reason, or we could just
use them for internal purposes.  Or decide now not use them because
we've never done so (and our users may have come to rely on that fact).

But anyway this still means that users wanting to pass extra fds to test
groups, should know that 5, 6, and 7 are reserved by Autotest, at least
for passing from outside the testsuite.  And probably that 5 is better
avoided inside as well (I'm not sure if that's necessary, but it might
be at some point).

Cheers,
Ralf



Re: [PATCH] Avoid passing autotest job fds to test groups.

2010-07-21 Thread Eric Blake
On 07/21/2010 12:06 PM, Ralf Wildenhues wrote:
 Hi Eric,

Hi Ralf,

 But maybe we want to do a global:

 for i in 3 4 5 6 7; do
   eval exec $i-
 done
 
 (why not just exec 5- 6- 7-?)

Easier to extend if we wanted to clear out to fd 63 or some such (if seq
were portable, then 'for i in $(seq 63); do...' would be easier than
writing 60 - instances).  But yeah, it makes more sense to only clear
what we will explicitly use, rather than to clear arbitrarily high.

 We don't do anything with fds 3 and 4; either we consider them special,
 then shouldn't close them for that historical reason, or we could just
 use them for internal purposes.  Or decide now not use them because
 we've never done so (and our users may have come to rely on that fact).

We also document that users cannot rely on shell 'exec 3file' to be
preserved over an exec*(2) call (that is, fds 3 or larger may be marked
cloexec when opened by the shell), and that other shells explicitly
close all inherited fds 3 or larger on startup (that is, even if an fd
is not cloexec in the shell's parent, it might be closed rather than
inherited to the shell script body).  So, on thinking about it more,
there is no portable way to write a test that relies on external
inheritance of an fd = 3 opened by the parent of the testsuite; and
within a portable test, you can use only fds 3 or larger only so long as
you stick to shell constructs (you can't expect your C program or a
subsidiary script to see those fds unless you used the shell to
duplicate it onto one of the 3 standard streams).

 
 But anyway this still means that users wanting to pass extra fds to test
 groups, should know that 5, 6, and 7 are reserved by Autotest, at least
 for passing from outside the testsuite.  And probably that 5 is better
 avoided inside as well (I'm not sure if that's necessary, but it might
 be at some point).

Yeah, documenting that fd 5 is reserved for autotest logging may be
worthwhile.  But I don't see that holding up 2.67.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] Avoid passing autotest job fds to test groups.

2010-07-21 Thread Ralf Wildenhues
* Eric Blake wrote on Wed, Jul 21, 2010 at 08:22:15PM CEST:
 On 07/21/2010 12:06 PM, Ralf Wildenhues wrote:
  But maybe we want to do a global:
 
  for i in 3 4 5 6 7; do
eval exec $i-
  done
  
  (why not just exec 5- 6- 7-?)
 
 Easier to extend if we wanted to clear out to fd 63 or some such (if seq
 were portable, then 'for i in $(seq 63); do...' would be easier than
 writing 60 - instances).  But yeah, it makes more sense to only clear
 what we will explicitly use, rather than to clear arbitrarily high.

$ ksh -c '( exec 17-)'
ksh[1]: exec: 17: not found
ksh -c '( fd=17; eval exec $fd\\-)'
ksh[1]: eval[1]: exec: 17: not found

It is not portable to use fds higher than 9, and the above eval had
quoting problems too ;-)

  We don't do anything with fds 3 and 4; either we consider them special,
  then shouldn't close them for that historical reason, or we could just
  use them for internal purposes.  Or decide now not use them because
  we've never done so (and our users may have come to rely on that fact).
 
 We also document that users cannot rely on shell 'exec 3file' to be
 preserved over an exec*(2) call (that is, fds 3 or larger may be marked
 cloexec when opened by the shell), and that other shells explicitly
 close all inherited fds 3 or larger on startup (that is, even if an fd
 is not cloexec in the shell's parent, it might be closed rather than
 inherited to the shell script body).  So, on thinking about it more,
 there is no portable way to write a test that relies on external
 inheritance of an fd = 3 opened by the parent of the testsuite; and
 within a portable test, you can use only fds 3 or larger only so long as
 you stick to shell constructs (you can't expect your C program or a
 subsidiary script to see those fds unless you used the shell to
 duplicate it onto one of the 3 standard streams).

This paragraph assumes that the portability requirements of some user
package are roughly the same as those Autoconf assumes for itself,
right?

  But anyway this still means that users wanting to pass extra fds to test
  groups, should know that 5, 6, and 7 are reserved by Autotest, at least
  for passing from outside the testsuite.  And probably that 5 is better
  avoided inside as well (I'm not sure if that's necessary, but it might
  be at some point).
 
 Yeah, documenting that fd 5 is reserved for autotest logging may be
 worthwhile.  But I don't see that holding up 2.67.

Of course not.  Finishing 2.67 is more important than these details.

Cheers,
Ralf



Relative paths for $INSTALL

2010-07-21 Thread Russ Allbery
We recently ran into some problems with OpenAFS where the relative path to
the install-sh script that Autoconf (config.status) substitutes into files
on systems without a good system install program (such as Solaris) caused
issues.  One of those involved calling a script created by config.status
in a loop that used cd, and the other was a place where our build system
symlinks a makefile to another makefile in a different directory.

There is usually always some sort of build system reconfiguration or
refactoring that one can do to make this work with relative paths, but
it's kind of annoying.  The first instinct of other project developers is
to play various games with AC_CONFIG_AUX_DIR to try to force it to be an
absolute path, but I'm worried we're going to do something that will break
later.

In doing some web searches, I see that GCC ran into the same problem and
changes the value of $INSTALL in configure to use an absolute path, which
is generated with `cd $srcdir ; pwd`.  But there too this seems too
fragile, since detecting the case of an Autoconf-set path to install-sh
may fail later.

So, in short, it would be very nice if there were some way to force
Autoconf to use absolute paths when substituting paths to scripts in the
aux directory into generated files.  Is there any chance that Autoconf
could add an AC_PROG_INSTALL_ABS or some other way to say that the
substituted path to install-sh needs to be an absolute path?  I think
that's the main program affected; config.sub and config.guess are run
internally by Autoconf in ways that don't have this problem, and the other
helper programs are generally Automake's business and Automake handles
generating the right make rules internally.  (OpenAFS uses Autoconf but
not Automake, as it has a large and complex build system that does some
things that Automake can't easily handle, such as build kernel modules for
nearly a dozen versions of UNIX.)

-- 
Russ Allbery (r...@stanford.edu) http://www.eyrie.org/~eagle/



[PATCH] Avoid spurious testsuite failures.

2010-07-21 Thread Eric Blake
* doc/autoconf.texi (Generating Sources): Don't mix gcc '-E' and
'-o -', since the former already implies stdout, while the latter
creates -.exe on cygwin.
* tests/compile.at (AC_LANG_SOURCE example)
(AC_LANG_PROGRAM example): Likewise.  Also prevent any config.site
interference.

Signed-off-by: Eric Blake ebl...@redhat.com
---

This fixes both issues, tested on Linux and cygwin, with and
without a config.site installed.

See also http://cygwin.com/ml/cygwin/2010-07/msg00450.html

 ChangeLog |8 
 doc/autoconf.texi |4 ++--
 tests/compile.at  |   14 --
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e44f78a..d435c19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2010-07-21  Eric Blake  ebl...@redhat.com

+   Avoid spurious testsuite failures.
+   * doc/autoconf.texi (Generating Sources): Don't mix gcc '-E' and
+   '-o -', since the former already implies stdout, while the latter
+   creates -.exe on cygwin.
+   * tests/compile.at (AC_LANG_SOURCE example)
+   (AC_LANG_PROGRAM example): Likewise.  Also prevent any config.site
+   interference.
+
Partially revert previous patch.
* lib/autotest/general.m4 (AT_INIT) serial testing: Changing
at_jobs here breaks output if -j2 was requested but shell is
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index c3a8714..e510354 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -8744,7 +8744,7 @@ Generating Sources
 AC_LANG([C])
 AC_LANG_CONFTEST(
[AC_LANG_SOURCE([[const char hw[] = Hello, World\n;]])])
-gcc -E -dD -o - conftest.c
+gcc -E -dD conftest.c
 @end example

 @noindent
@@ -8789,7 +8789,7 @@ Generating Sources
 AC_LANG_CONFTEST(
 [AC_LANG_PROGRAM([[const char hw[] = Hello, World\n;]],
  [[fputs (hw, stdout);]])])
-gcc -E -dD -o - conftest.c
+gcc -E -dD conftest.c
 @end example

 @noindent
diff --git a/tests/compile.at b/tests/compile.at
index e3e4e07..028f456 100644
--- a/tests/compile.at
+++ b/tests/compile.at
@@ -169,6 +169,11 @@ AT_CLEANUP

 AT_SETUP([AC_LANG_SOURCE example])

+# Set CONFIG_SITE to a nonexistent file, so that there are
+# no worries about configure output caused by sourcing a config.site.
+CONFIG_SITE=no-such-file
+export CONFIG_SITE
+
 AT_DATA([configure.ac],
 [[# Taken from autoconf.texi:Generating Sources.
 # The only change is to not fail if gcc doesn't work.
@@ -179,7 +184,7 @@ AC_DEFINE([HELLO_WORLD], [Hello, World\n],
 AC_LANG([C])
 AC_LANG_CONFTEST(
[AC_LANG_SOURCE([[const char hw[] = Hello, World\n;]])])
-gcc -E -dD -o - conftest.c || AS_EXIT([77])
+gcc -E -dD conftest.c || AS_EXIT([77])
 ]])

 AT_CHECK_AUTOCONF
@@ -210,6 +215,11 @@ AT_CLEANUP

 AT_SETUP([AC_LANG_PROGRAM example])

+# Set CONFIG_SITE to a nonexistent file, so that there are
+# no worries about configure output caused by sourcing a config.site.
+CONFIG_SITE=no-such-file
+export CONFIG_SITE
+
 AT_DATA([configure.ac],
 [[# Taken from autoconf.texi:Generating Sources.
 # The only change is to not fail if gcc doesn't work.
@@ -220,7 +230,7 @@ AC_DEFINE([HELLO_WORLD], [Hello, World\n],
 AC_LANG_CONFTEST(
 [AC_LANG_PROGRAM([[const char hw[] = Hello, World\n;]],
  [[fputs (hw, stdout);]])])
-gcc -E -dD -o - conftest.c || AS_EXIT([77])
+gcc -E -dD conftest.c || AS_EXIT([77])
 ]])

 AT_CHECK_AUTOCONF
-- 
1.7.1.1




Re: testsuite failure on cygwin

2010-07-21 Thread Ralf Wildenhues
Hi Eric,

* Eric Blake wrote on Wed, Jul 21, 2010 at 11:14:42PM CEST:
 It turns out that this line (in both tests):
 
 gcc -E -dD -o - conftest.c || AS_EXIT([77])
 
 is not portable to cygwin, where gcc currently compiles ./-.exe rather
 than outputting to stdout.  Do we really need the '-o -' in that command
 line?

No; thanks for fixing it.  The marker comments are paying off in making
our manual suggest more portable examples!  :-)

 And should I be reporting the surprising behavior of creating
 -.exe as a gcc/cygwin bug to the appropriate folks?

I think you should.

Thanks,
Ralf



Re: Relative paths for $INSTALL

2010-07-21 Thread Ralf Wildenhues
Hi Russ,

* Russ Allbery wrote on Wed, Jul 21, 2010 at 11:41:41PM CEST:
 So, in short, it would be very nice if there were some way to force
 Autoconf to use absolute paths when substituting paths to scripts in the
 aux directory into generated files.  Is there any chance that Autoconf
 could add an AC_PROG_INSTALL_ABS or some other way to say that the
 substituted path to install-sh needs to be an absolute path?

I think this is a valid request, but you should also take into account
that it breaks building in directories with, e.g., spaces in the
absolute name.  This works with current Autoconf and Automake (not
Libtool, unfortunately) as long as you specify a relative name without
spaces when calling configure.

There are more programs affected besides install-sh, and the way to
enable it should not carry INSTALL in the name, because that is
misleading IMVHO.  The macro could ensure `pwd` contains no dangerous
characters.

I can look into it eventually.

Thanks,
Ralf