[PATCH]: Missing tests-base in gnulib-cache.m4

[Yoann Vandoorselaere]

Hi,

The --tests-base argument is not saved within gnulib-cache.m4. The
attached patch fixes this issue.

Additionally, I'm running into another problem when cross compiling for
Win32 systems:

- Importing GnuLib file using the following command:

../gnulib/gnulib-tool --import --dir=. --lib=libmissing
--tests-base=libmissing/tests --source-base=libmissing
--m4-base=libmissing/m4 --doc-base=libmissing/doc --aux-dir=.
--with-tests --lgpl --libtool --macro-prefix=gl --no-vc-files [modules
list]

- Then editing libmissing/Makefile.am by hand to list the "tests"
directory as SUBDIRS (shouldn't this be handled automatically?).

- After generating the autotools build system, doing 'make check' will
result in the following error (only when cross compiling for Win32
systems):


$ make check
make  check-recursive
make[1]: Entering directory
`/home/yoann/dev/prelude/git/libprelude/libmissing'
Making check in tests
make[2]: Entering directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make  check-recursive
make[3]: Entering directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make[4]: Entering directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make  libtests.a test-alloca-opt.exe test-arpa_inet.exe test-c-ctype.exe
test-cond.exe test-errno.exe test-fopen.exe test-fseeko.exe
test-getaddrinfo.exe test-getdelim.exe test-getline.exe
test-gettimeofday.exe test-lock.exe test-lseek.exe test-memchr.exe
test-memcmp.exe test-memmem.exe test-netinet_in.exe test-perror.exe
test-poll.exe test-sleep.exe test-snprintf.exe test-sockets.exe
test-stdbool.exe test-stdint.exe test-stdio.exe test-stdlib.exe
test-strcasestr.exe test-strerror.exe test-string.exe test-strings.exe
test-sys_select.exe test-sys_socket.exe test-sys_stat.exe
test-sys_time.exe test-time.exe test-tls.exe test-unistd.exe
test-vasnprintf.exe test-vsnprintf.exe test-wchar.exe test-wctype.exe
make[5]: Entering directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make[5]: `libtests.a' is up to date.
make[5]: `test-alloca-opt.exe' is up to date.
make[5]: `test-arpa_inet.exe' is up to date.
make[5]: `test-c-ctype.exe' is up to date.
make[5]: `test-cond.exe' is up to date.
make[5]: `test-errno.exe' is up to date.
make[5]: `test-fopen.exe' is up to date.
make[5]: `test-fseeko.exe' is up to date.
make[5]: `test-getaddrinfo.exe' is up to date.
make[5]: `test-getdelim.exe' is up to date.
make[5]: `test-getline.exe' is up to date.
make[5]: `test-gettimeofday.exe' is up to date.
make[5]: `test-lock.exe' is up to date.
make[5]: `test-lseek.exe' is up to date.
make[5]: `test-memchr.exe' is up to date.
make[5]: `test-memcmp.exe' is up to date.
make[5]: `test-memmem.exe' is up to date.
make[5]: `test-netinet_in.exe' is up to date.
make[5]: `test-perror.exe' is up to date.
make[5]: `test-poll.exe' is up to date.
make[5]: `test-sleep.exe' is up to date.
make[5]: `test-snprintf.exe' is up to date.
make[5]: `test-sockets.exe' is up to date.
make[5]: `test-stdbool.exe' is up to date.
make[5]: `test-stdint.exe' is up to date.
make[5]: `test-stdio.exe' is up to date.
make[5]: `test-stdlib.exe' is up to date.
make[5]: `test-strcasestr.exe' is up to date.
make[5]: `test-strerror.exe' is up to date.
make[5]: `test-string.exe' is up to date.
make[5]: `test-strings.exe' is up to date.
make[5]: `test-sys_select.exe' is up to date.
make[5]: `test-sys_socket.exe' is up to date.
make[5]: `test-sys_stat.exe' is up to date.
make[5]: `test-sys_time.exe' is up to date.
make[5]: `test-time.exe' is up to date.
make[5]: `test-tls.exe' is up to date.
make[5]: `test-unistd.exe' is up to date.
make[5]: `test-vasnprintf.exe' is up to date.
make[5]: `test-vsnprintf.exe' is up to date.
make[5]: `test-wchar.exe' is up to date.
make[5]: `test-wctype.exe' is up to date.
make[5]: Leaving directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make  check-TESTS
make[5]: Entering directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
i586-mingw32msvc-gcc -std=gnu99 -g -O2test-alloca-opt.c   -o
test-alloca-opt
test-alloca-opt.c:19:20: error: config.h: No such file or directory
test-alloca-opt.c:21:20: error: alloca.h: No such file or directory
make[5]: *** [test-alloca-opt] Error 1
make[5]: Leaving directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make[4]: *** [check-am] Error 2
make[4]: Leaving directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make[3]: *** [check-recursive] Error 1
make[3]: Leaving directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make[2]: *** [check] Error 2
make[2]: Leaving directory
`/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory
`/home/yoann/dev/prelude/git/libprelude/libmissing'
make: *** [check] Error 2

-- 
Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies
Tel: +33 (0)9 50 70 21 58  Fax: +33(0)9 57 25 21 58
http://www.prelude-ids.com
diff 

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Paolo Bonzini]

> - After generating the autotools build system, doing 'make check' will
> result in the following error (only when cross compiling for Win32
> systems):

"make check" does not work for Windows crosses because check-TESTS
depends on $(TESTS), which includes "test-allocaopt".  So make attempts
to make it instead of "test-allocaopt.exe".  I think that under native
configurations make attempts to add the ".exe" automatically, but I am
not sure of that.

It should be possible to fix this with a

sed -i '/^TESTS/ s/\(test-[-a-z0-9_]*\)\( \|$\)/\1$(EXEEXT)\2/g' *-tests

command in the modules directory (yes, tested).  It still wouldn't fix
the fact that tests need to be invoked with Wine, though.

Paolo

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Ralf Wildenhues]

* Paolo Bonzini wrote on Thu, Sep 25, 2008 at 12:43:05PM CEST:
> 
> > - After generating the autotools build system, doing 'make check' will
> > result in the following error (only when cross compiling for Win32
> > systems):
> 
> "make check" does not work for Windows crosses because check-TESTS
> depends on $(TESTS), which includes "test-allocaopt".  So make attempts
> to make it instead of "test-allocaopt.exe".  I think that under native
> configurations make attempts to add the ".exe" automatically, but I am
> not sure of that.

Which Automake version do you use?  This should be fixed in 1.10, I
believe.  If not, I'd appreciate a bug report, thanks.

Cheers,
Ralf

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Paolo Bonzini]

Ralf Wildenhues wrote:
> * Paolo Bonzini wrote on Thu, Sep 25, 2008 at 12:43:05PM CEST:
>>> - After generating the autotools build system, doing 'make check' will
>>> result in the following error (only when cross compiling for Win32
>>> systems):
>> "make check" does not work for Windows crosses because check-TESTS
>> depends on $(TESTS), which includes "test-allocaopt".  So make attempts
>> to make it instead of "test-allocaopt.exe".  I think that under native
>> configurations make attempts to add the ".exe" automatically, but I am
>> not sure of that.
> 
> Which Automake version do you use?  This should be fixed in 1.10, I
> believe.  If not, I'd appreciate a bug report, thanks.

1.9.6 indeed.

Paolo

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Simon Josefsson]

Ralf Wildenhues <[EMAIL PROTECTED]> writes:

> * Paolo Bonzini wrote on Thu, Sep 25, 2008 at 12:43:05PM CEST:
>> 
>> > - After generating the autotools build system, doing 'make check' will
>> > result in the following error (only when cross compiling for Win32
>> > systems):
>> 
>> "make check" does not work for Windows crosses because check-TESTS
>> depends on $(TESTS), which includes "test-allocaopt".  So make attempts
>> to make it instead of "test-allocaopt.exe".  I think that under native
>> configurations make attempts to add the ".exe" automatically, but I am
>> not sure of that.
>
> Which Automake version do you use?  This should be fixed in 1.10, I
> believe.  If not, I'd appreciate a bug report, thanks.

I added a mingw32 auto-build to simplify mingw32 portability tests, the
output will be visible daily from now on at:

http://autobuild.josefsson.org/gnulib/#dummy-i586-pc-mingw32msvc

Right now, the gnulib modules that fail to build under mingw32 are (see
log): savewd, sigpipe-die, getugroups, idcache, and userspec.  Once they
are fixed, the logs will contain output from the self-tests too, which
could be interesting.

FWIW, I use automake 1.10.1 with cross-compile builds without any
problem with 'make check'.

/Simon

[PATCH] modules/sys_select-tests (Depends-on): Remove sys_select.

[Simon Josefsson]

Pushed.

/Simon

---
 ChangeLog|5 +
 modules/sys_select-tests |1 -
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3a1f2c4..8098603 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-09-25  Simon Josefsson  <[EMAIL PROTECTED]>
+
+   * modules/sys_select-tests (Depends-on): Remove sys_select itself,
+   fixes gnulib-tool --test warning about duplicate dependency.
+
 2008-09-25  Bruno Haible  <[EMAIL PROTECTED]>
 
* gnulib-tool: Don't ask the user to perform edits in the generated
diff --git a/modules/sys_select-tests b/modules/sys_select-tests
index e004b33..ab9e59e 100644
--- a/modules/sys_select-tests
+++ b/modules/sys_select-tests
@@ -3,7 +3,6 @@ tests/test-sys_select.c
 
 Depends-on:
 stdbool
-sys_select
 netinet_in
 arpa_inet
 extensions
-- 
1.5.6.5

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Bruno Haible]

Hi Yoann,

> The --tests-base argument is not saved within gnulib-cache.m4. The
> attached patch fixes this issue.

It is saved, but only as argument of gl_TESTS_BASE, not in the comment.
I have applied your patch.

> Additionally, I'm running into another problem when cross compiling for
> Win32 systems:
> 
> - Importing GnuLib file using the following command:
> 
> ../gnulib/gnulib-tool --import --dir=. --lib=libmissing
> --tests-base=libmissing/tests --source-base=libmissing
> --m4-base=libmissing/m4 --doc-base=libmissing/doc --aux-dir=.
> --with-tests --lgpl --libtool --macro-prefix=gl --no-vc-files [modules
> list]
> 
> - Then editing libmissing/Makefile.am by hand to list the "tests"
> directory as SUBDIRS (shouldn't this be handled automatically?).

Right, it should be handled automatically. Currently gnulib-tool is asking
the user to make a change to a file that is generated by gnulib-tool, and
invoking "gnulib-tool --import" once again will erase the manual change.
I'm fixing this through the patch below.

> - After generating the autotools build system, doing 'make check' will
> result in the following error (only when cross compiling for Win32
> systems):
> 
> 
> $ make check
> make  check-recursive
> make[1]: Entering directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing'
> Making check in tests
> make[2]: Entering directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> make  check-recursive
> make[3]: Entering directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> make[4]: Entering directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> make  libtests.a test-alloca-opt.exe test-arpa_inet.exe test-c-ctype.exe
> test-cond.exe test-errno.exe test-fopen.exe test-fseeko.exe
> test-getaddrinfo.exe test-getdelim.exe test-getline.exe
> test-gettimeofday.exe test-lock.exe test-lseek.exe test-memchr.exe
> test-memcmp.exe test-memmem.exe test-netinet_in.exe test-perror.exe
> test-poll.exe test-sleep.exe test-snprintf.exe test-sockets.exe
> test-stdbool.exe test-stdint.exe test-stdio.exe test-stdlib.exe
> test-strcasestr.exe test-strerror.exe test-string.exe test-strings.exe
> test-sys_select.exe test-sys_socket.exe test-sys_stat.exe
> test-sys_time.exe test-time.exe test-tls.exe test-unistd.exe
> test-vasnprintf.exe test-vsnprintf.exe test-wchar.exe test-wctype.exe
> make[5]: Entering directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> make[5]: `libtests.a' is up to date.
> make[5]: `test-alloca-opt.exe' is up to date.
> make[5]: `test-arpa_inet.exe' is up to date.
> make[5]: `test-c-ctype.exe' is up to date.
> make[5]: `test-cond.exe' is up to date.
> make[5]: `test-errno.exe' is up to date.
> make[5]: `test-fopen.exe' is up to date.
> make[5]: `test-fseeko.exe' is up to date.
> make[5]: `test-getaddrinfo.exe' is up to date.
> make[5]: `test-getdelim.exe' is up to date.
> make[5]: `test-getline.exe' is up to date.
> make[5]: `test-gettimeofday.exe' is up to date.
> make[5]: `test-lock.exe' is up to date.
> make[5]: `test-lseek.exe' is up to date.
> make[5]: `test-memchr.exe' is up to date.
> make[5]: `test-memcmp.exe' is up to date.
> make[5]: `test-memmem.exe' is up to date.
> make[5]: `test-netinet_in.exe' is up to date.
> make[5]: `test-perror.exe' is up to date.
> make[5]: `test-poll.exe' is up to date.
> make[5]: `test-sleep.exe' is up to date.
> make[5]: `test-snprintf.exe' is up to date.
> make[5]: `test-sockets.exe' is up to date.
> make[5]: `test-stdbool.exe' is up to date.
> make[5]: `test-stdint.exe' is up to date.
> make[5]: `test-stdio.exe' is up to date.
> make[5]: `test-stdlib.exe' is up to date.
> make[5]: `test-strcasestr.exe' is up to date.
> make[5]: `test-strerror.exe' is up to date.
> make[5]: `test-string.exe' is up to date.
> make[5]: `test-strings.exe' is up to date.
> make[5]: `test-sys_select.exe' is up to date.
> make[5]: `test-sys_socket.exe' is up to date.
> make[5]: `test-sys_stat.exe' is up to date.
> make[5]: `test-sys_time.exe' is up to date.
> make[5]: `test-time.exe' is up to date.
> make[5]: `test-tls.exe' is up to date.
> make[5]: `test-unistd.exe' is up to date.
> make[5]: `test-vasnprintf.exe' is up to date.
> make[5]: `test-vsnprintf.exe' is up to date.
> make[5]: `test-wchar.exe' is up to date.
> make[5]: `test-wctype.exe' is up to date.
> make[5]: Leaving directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> make  check-TESTS
> make[5]: Entering directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> i586-mingw32msvc-gcc -std=gnu99 -g -O2test-alloca-opt.c   -o
> test-alloca-opt
> test-alloca-opt.c:19:20: error: config.h: No such file or directory
> test-alloca-opt.c:21:20: error: alloca.h: No such file or directory
> make[5]: *** [test-alloca-opt] Error 1
> make[5]: Leaving directory
> `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> make[4]: *** [check-am] Error 2
> make[4]: Leaving directory
> `/home/yoann/dev/prelude/git/libprelude/li

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Ralf Wildenhues]

* Bruno Haible wrote on Thu, Sep 25, 2008 at 02:42:39PM CEST:
> > `/home/yoann/dev/prelude/git/libprelude/libmissing/tests'
> > i586-mingw32msvc-gcc -std=gnu99 -g -O2test-alloca-opt.c   -o
> > test-alloca-opt
> > test-alloca-opt.c:19:20: error: config.h: No such file or directory
> > test-alloca-opt.c:21:20: error: alloca.h: No such file or directory
> > make[5]: *** [test-alloca-opt] Error 1
[...]
> Can you investigate? I cannot reproduce.

This is probably a followup error of using Automake < 1.10.
The target test-alloca-opt.exe will built correctly, the target
test-alloca-opt only through a GNU make builtin rule (which lacks
some of the flags).

Cheers,
Ralf

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Simon Josefsson]

Bruno Haible <[EMAIL PROTECTED]> writes:

>> - Then editing libmissing/Makefile.am by hand to list the "tests"
>> directory as SUBDIRS (shouldn't this be handled automatically?).
>
> Right, it should be handled automatically. Currently gnulib-tool is asking
> the user to make a change to a file that is generated by gnulib-tool, and
> invoking "gnulib-tool --import" once again will erase the manual change.
> I'm fixing this through the patch below.

It doesn't work fully, in libidn doing a --import with the new
gnulib-tool results in a diff:

diff --git a/gl/Makefile.am b/gl/Makefile.am
index 934ab4f..31890af 100644
--- a/gl/Makefile.am
+++ b/gl/Makefile.am
@@ -9,10 +9,11 @@
 # the same distribution terms as the rest of that program.
 #
 # Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --dir=. --local-dir=gl/override 
--lib=libgnu --source-base=gl --m4-base=gl/m4 -
+# Reproduce by: gnulib-tool --import --dir=. --local-dir=gl/override 
--lib=libgnu --source-base=gl --m4-base=gl/m4 -
 
 AUTOMAKE_OPTIONS = 1.5 gnits
 
+SUBDIRS =
 noinst_HEADERS =
 noinst_LIBRARIES =
 noinst_LTLIBRARIES =
@@ -24,6 +25,7 @@ MOSTLYCLEANDIRS =
 CLEANFILES =
 DISTCLEANFILES =
 MAINTAINERCLEANFILES =
+EXTRA_DIST += m4/gnulib-cache.m4
 
 AM_CPPFLAGS =
 

As you can see, the EXTRA_DIST contains the wrong path: it should be
gl/m4/gnulib-cache.m4.

Otherwise it looks good to me too, FWIW.

/Simon

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Bruno Haible]

Simon Josefsson wrote:
> It doesn't work fully, in libidn doing a --import with the new
> gnulib-tool results in a diff:
> 
> @@ -9,10 +9,11 @@
>  # the same distribution terms as the rest of that program.
>  #
>  # Generated by gnulib-tool.
> -# Reproduce by: gnulib-tool --import --dir=. --local-dir=gl/override 
> --lib=libgnu --source-base=gl --m4-base=gl/m4 -
> +# Reproduce by: gnulib-tool --import --dir=. --local-dir=gl/override 
> --lib=libgnu --source-base=gl --m4-base=gl/m4 -

I cannot see any differences between these two lines.

> @@ -24,6 +25,7 @@ MOSTLYCLEANDIRS =
>  CLEANFILES =
>  DISTCLEANFILES =
>  MAINTAINERCLEANFILES =
> +EXTRA_DIST += m4/gnulib-cache.m4
>  
>  AM_CPPFLAGS =
>  
> 
> As you can see, the EXTRA_DIST contains the wrong path: it should be
> gl/m4/gnulib-cache.m4.

No, it is correct. File names in gl/Makefile.am are supposed to be relative
to the gl/ directory.

Bruno

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Bruno Haible]

Oops, the patch that I committed worked right in the second run, but not
in the first run. Fixed:

2008-09-25  Bruno Haible  <[EMAIL PROTECTED]>

* gnulib-tool (func_import): When computing the edits, consider not
only the Makefile.ams that exist but also those that will be generated.

*** gnulib-tool.orig2008-09-25 15:59:10.0 +0200
--- gnulib-tool 2008-09-25 15:57:51.0 +0200
***
*** 2929,2940 
fi
func_note_Makefile_am_edit "" ACLOCAL_AMFLAGS "-I ${m4base}"
{
! # Find the first parent directory of $m4base that contains a Makefile.am.
  sed_last='s,^.*/\([^/][^/]*\)//*$,\1/,
  s,//*$,/,'
  sed_butlast='s,[^/][^/]*//*$,,'
  dir1="${m4base}/"; dir2=""
! while test -n "$dir1" && test ! -f "${destdir}/${dir1}Makefile.am"; do
dir2=`echo "$dir1" | sed -e "$sed_last"`"$dir2"
dir1=`echo "$dir1" | sed -e "$sed_butlast"`
  done
--- 2929,2947 
fi
func_note_Makefile_am_edit "" ACLOCAL_AMFLAGS "-I ${m4base}"
{
! # Find the first parent directory of $m4base that contains or will contain
! # a Makefile.am.
  sed_last='s,^.*/\([^/][^/]*\)//*$,\1/,
  s,//*$,/,'
  sed_butlast='s,[^/][^/]*//*$,,'
  dir1="${m4base}/"; dir2=""
! while test -n "$dir1" \
!   && ! { test -f "${destdir}/${dir1}Makefile.am" \
!  || test "${dir1}Makefile.am" = "$sourcebase/$makefile_am" \
!  || test "./${dir1}Makefile.am" = "$sourcebase/$makefile_am" \
!  || { test -n "$inctests" \
!   && { test "${dir1}Makefile.am" = 
"$testsbase/$makefile_am" \
!|| test "./${dir1}Makefile.am" = 
"$testsbase/$makefile_am"; }; }; }; do
dir2=`echo "$dir1" | sed -e "$sed_last"`"$dir2"
dir1=`echo "$dir1" | sed -e "$sed_butlast"`
  done

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Yoann Vandoorselaere]

Le jeudi 25 septembre 2008 à 12:55 +0200, Ralf Wildenhues a écrit :
> * Paolo Bonzini wrote on Thu, Sep 25, 2008 at 12:43:05PM CEST:
> > 
> > > - After generating the autotools build system, doing 'make check' will
> > > result in the following error (only when cross compiling for Win32
> > > systems):
> > 
> > "make check" does not work for Windows crosses because check-TESTS
> > depends on $(TESTS), which includes "test-allocaopt".  So make attempts
> > to make it instead of "test-allocaopt.exe".  I think that under native
> > configurations make attempts to add the ".exe" automatically, but I am
> > not sure of that.
> 
> Which Automake version do you use?  This should be fixed in 1.10, I
> believe.  If not, I'd appreciate a bug report, thanks.

I was using automake 1.9.6. I just tested using automake 1.10, and can
confirm this solve the problem.

Thanks!

-- 
Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies
Tel: +33 (0)9 50 70 21 58  Fax: +33(0)9 57 25 21 58
http://www.prelude-ids.com

Re: Friendlier gnulib-tool error messages with multiple incompatible modules

[Derek Price]

Ralf Wildenhues wrote:
> a couple of nits, if I may:

The attached patch addresses all of Ralf's nits, fixes a few minor bugs,
and contains a few additional simplifications of the new functions.  Any
objections?

Regards,

Derek
-- 
Derek R. Price
Solutions Architect
Ximbiot, LLC 
Get CVS and Subversion Support from Ximbiot!

v: +1 248.835.1260
f: +1 248.246.1176
Index: gnulib-tool
===
RCS file: /srv/git/gnulib.git/HEAD/gnulib-tool,v
retrieving revision 1.307
diff -p -u -N gnulib-tool
--- gnulib-tool revision 1.307
+++ gnulib-tool working copy
@@ -396,11 +396,70 @@
   }
 fi
 
+# Treat the shell variable with name $1 like a space delimited stack and
+# append the rest of [EMAIL PROTECTED]  Inserts a leading space when appending 
the first
+# element.
+func_push ()
+{
+  var=$1
+  shift
+  for e; do
+func_append $var " $e"
+  done
+}
+
+# Using $1 as a separator, join the rest of $@, then echo the result.
+#
+# e.g. `join ", " a b c d e f' yields:
+#
+#   a, b, c, d, e, f
+func_join ()
+{
+  sep=$1
+  out=$2
+  shift
+  shift
+  for e; do
+func_append out "$sep$e"
+  done
+  echo "$out"
+}
+
+# Word wrap $1 at $3 characters, prefixing each line with $2, then echo it.
+#
+# $3 (wrap length) defaults to 80.
+#
+# e.g. `wrap "a b c d e f g h i j k xx" "gnulib: " 14' yields:
+#
+#  gnulib: a b c 
+#  gnulib: d e f 
+#  gnulib: g h i 
+#  gnulib: j k 
+#  gnulib: 
+#  xx
+#
+# NOTES
+#   This function won't currently deal correctly with EOL in $1.  Replacing `.'
+#   in the first regular expression with a portable representation of `[^\n]'
+#   should fix this.
+func_wrap ()
+{
+  plen=`echo "$2" | wc -c`
+  len=`expr ${3-80} - $plen`
+
+  # First sed command wraps, second removes the extra EOL we added, third
+  # inserts the first prefix, fourth inserts the remaining prefixes.
+  re='s#\(.\{1,'"$len"'\}\)  *#\1\
+#g'
+  echo "$1 " | sed -e "$re" -e 's#.$##' -e "s#^#$2#" -e "s#\\n#&$2#g"
+}
+
+
 # func_fatal_error message
 # outputs to stderr a fatal error message, and terminates the program.
 func_fatal_error ()
 {
-  echo "gnulib-tool: *** $1" 1>&2
+  func_wrap "$1" "gnulib-tool: *** " 1>&2
   echo "gnulib-tool: *** Stop." 1>&2
   func_exit 1
 }
@@ -2516,6 +2575,7 @@
 
   # If --lgpl, verify that the licenses of modules are compatible.
   if test -n "$lgpl"; then
+incompatible_modules=
 for module in $main_modules; do
   license=`func_get_license $module`
   case $license in
@@ -2526,13 +2586,13 @@
 yes | 3)
   case $license in
 LGPL | LGPLv2+) ;;
-*) func_fatal_error "incompatible license on module $module: 
$license" ;;
+*) func_push incompatible_modules $module/$license ;;
   esac
   ;;
 2)
   case $license in
 LGPLv2+) ;;
-*) func_fatal_error "incompatible license on module $module: 
$license" ;;
+*) func_push incompatible_modules $module/$license ;;
   esac
   ;;
 *) func_fatal_error "invalid value lgpl=$lgpl" ;;
@@ -2540,6 +2600,10 @@
   ;;
   esac
 done
+if test -n "$incompatible_modules"; then
+   list=`func_join ", " $incompatible_modules`
+   func_fatal_error "incompatible license on modules: $list"
+fi
   fi
 
   # Show banner notice of every module.

fts: avoid O(n^2) ext3/ext4 performance hit on million+-entry dirs

[Jim Meyering]

Hello,

Here is a patch that makes it so tools using fts,
like chmod, chown, chgrp, chcon, du, and find are no
longer susceptible to an O(n^2) performance penalty when
processing very large directory-entry counts (as in millions).
I first noticed the problem on ext3 and ext4 file systems,
but the patch also improves performance on reiserfs and xfs,
but not on tmpfs.

I've been using this on multiple systems via coreutils,
and of course, it passes all of the tests there.
I expect to push this tomorrow.

>From 81cd6073dfc09d295f9cf0910cc29138f017f58d Mon Sep 17 00:00:00 2001
From: Jim Meyering <[EMAIL PROTECTED]>
Date: Tue, 16 Sep 2008 10:05:47 +0200
Subject: [PATCH] fts: sort dirent entries on inode number before traversing

This avoids a quadratic, seek-related performance penalty when
operating on a directory containing many entries (measurable at 10k,
3.5 hours at 2 million entries with a cold cache) on certain types
of file systems, including ext3 and ext4, but not tmpfs.
* lib/fts.c (DT_MUST_BE, NOT_AN_INODE_NUMBER, D_INO): Define.
(FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD): Define if not defined.
(S_MAGIC_TMPFS, S_MAGIC_NFS): Define.
(fs_handles_readdir_ordered_dirents_efficiently): New function.
(dirent_inode_sort_may_be_useful, fts_compare_ino): Likewise.
(fts_build): Set the stat.st_ino member from D_INO.
If it is likely to be useful, sort dirent entries on inode number.
* m4/fts.m4 (gl_FUNC_FTS_CORE): Check for fstatfs, sys/vfs.h,
and the struct statfs.f_type member.
* modules/fts (Depends-on): Add d-ino.
---
 lib/fts.c   |   90 +++
 m4/fts.m4   |9 +++--
 modules/fts |1 +
 3 files changed, 96 insertions(+), 4 deletions(-)

diff --git a/lib/fts.c b/lib/fts.c
index 7956f72..cd14ec4 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -91,6 +91,29 @@ static char sccsid[] = "@(#)fts.c8.6 (Berkeley) 8/14/94";
 # define DT_MUST_BE(d, t) false
 #endif

+enum
+{
+  NOT_AN_INODE_NUMBER = 0
+};
+
+#ifdef D_INO_IN_DIRENT
+# define D_INO(dp) (dp)->d_ino
+#else
+/* Some systems don't have inodes, so fake them to avoid lots of ifdefs.  */
+# define D_INO(dp) NOT_AN_INODE_NUMBER
+#endif
+
+/* If there are more than this many entries in a directory,
+   and the conditions mentioned below are satisfied, then sort
+   the entries on inode number before any further processing.  */
+#ifndef FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD
+# define FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD 1
+#endif
+enum
+{
+  _FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD = FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD
+};
+
 enum Fts_stat
 {
   FTS_NO_STAT_REQUIRED = 1,
@@ -911,6 +934,58 @@ fts_children (register FTS *sp, int instr)
return (sp->fts_child);
 }

+#if defined HAVE_SYS_VFS_H && HAVE_FSTATFS && HAVE_STRUCT_STATFS_F_TYPE
+# include 
+/* FIXME: what about when f_type is not an integral type?
+   deal with that if/when it's encountered.  */
+static bool
+fs_handles_readdir_ordered_dirents_efficiently (uintmax_t fs_type)
+{
+/* From coreutils' src/fs.h */
+#define S_MAGIC_TMPFS 0x1021994
+#define S_MAGIC_NFS 0x6969
+  switch (fs_type)
+{
+case S_MAGIC_TMPFS:
+case S_MAGIC_NFS:
+  return true;
+default:
+  return false;
+}
+}
+
+/* Return true if it is easy to determine the file system type of the
+   current directory, and sorting dirents on inode numbers is known to
+   improve traversal performance with that type of file system.  */
+static bool
+dirent_inode_sort_may_be_useful (FTS const *sp)
+{
+  struct statfs fs_buf;
+  /* Skip the sort only if we can determine efficiently
+ that it's the right thing to do.  */
+  bool skip = (ISSET (FTS_CWDFD)
+  && fstatfs (sp->fts_cwd_fd, &fs_buf) == 0
+  && fs_handles_readdir_ordered_dirents_efficiently
+   (fs_buf.f_type));
+  return !skip;
+}
+#else
+static bool dirent_inode_sort_may_be_useful (FTS const *sp) { return true; }
+#endif
+
+/* A comparison function to sort on increasing inode number.
+   For some file system types, sorting either way makes a huge
+   performance difference for a directory with very many entries,
+   but sorting on increasing values is slightly better than sorting
+   on decreasing values.  The difference is in the 5% range.  */
+static int
+fts_compare_ino (struct _ftsent const **a, struct _ftsent const **b)
+{
+  int diff = (b[0]->fts_statp->st_ino
+ - a[0]->fts_statp->st_ino);
+  return diff;
+}
+
 /*
  * This is the tricky part -- do not casually change *anything* in here.  The
  * idea is to build the linked list of entries that are used by fts_children
@@ -,6 +1186,9 @@ mem1: saved_errno = errno;
if (dp->d_type == DT_WHT)
p->fts_flags |= FTS_ISW;
 #endif
+   /* Store dirent.d_ino, in case we need to sort
+  entries before processing them.  */
+   p->fts_statp->st_ino = D_INO (dp);

/* Build a file name for

Re: Friendlier gnulib-tool error messages with multiple incompatible modules

[Ralf Wildenhues]

* Derek Price wrote on Thu, Sep 25, 2008 at 05:10:31PM CEST:
> --- gnulib-tool revision 1.307
> +++ gnulib-tool working copy

> +func_push ()
> +{
> +  var=$1
> +  shift
> +  for e; do

The more portable way really is
  for e
  do

not
  for e; do

sorry for not stating that clearly the first time.



> +func_append $var " $e"
> +  done

Cheers,
Ralf

Re: fts: avoid O(n^2) ext3/ext4 performance hit on million+-entry dirs

[Ralf Wildenhues]

Hi Jim,

* Jim Meyering wrote on Thu, Sep 25, 2008 at 06:16:58PM CEST:
> --- a/lib/fts.c
> +++ b/lib/fts.c

> +/* A comparison function to sort on increasing inode number.
> +   For some file system types, sorting either way makes a huge
> +   performance difference for a directory with very many entries,
> +   but sorting on increasing values is slightly better than sorting
> +   on decreasing values.  The difference is in the 5% range.  */
> +static int
> +fts_compare_ino (struct _ftsent const **a, struct _ftsent const **b)
> +{
> +  int diff = (b[0]->fts_statp->st_ino
> +   - a[0]->fts_statp->st_ino);

This can over- resp. underflow and then give the wrong sign, no?

> +  return diff;
> +}

Cheers,
Ralf

Re: fts: avoid O(n^2) ext3/ext4 performance hit on million+-entry dirs

[Jim Meyering]

Ralf Wildenhues <[EMAIL PROTECTED]> wrote:
> * Jim Meyering wrote on Thu, Sep 25, 2008 at 06:16:58PM CEST:
>> --- a/lib/fts.c
>> +++ b/lib/fts.c
>
>> +/* A comparison function to sort on increasing inode number.
>> +   For some file system types, sorting either way makes a huge
>> +   performance difference for a directory with very many entries,
>> +   but sorting on increasing values is slightly better than sorting
>> +   on decreasing values.  The difference is in the 5% range.  */
>> +static int
>> +fts_compare_ino (struct _ftsent const **a, struct _ftsent const **b)
>> +{
>> +  int diff = (b[0]->fts_statp->st_ino
>> +  - a[0]->fts_statp->st_ino);
>
> This can over- resp. underflow and then give the wrong sign, no?

Hi Ralf,

Thanks for the quick feedback.
You're right, of course, and I'll fold in this change:

diff --git a/lib/fts.c b/lib/fts.c
index cd14ec4..3cf49fa 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -981,9 +981,8 @@ static bool dirent_inode_sort_may_be_useful (FTS const *sp) 
{ return true; }
 static int
 fts_compare_ino (struct _ftsent const **a, struct _ftsent const **b)
 {
-  int diff = (b[0]->fts_statp->st_ino
- - a[0]->fts_statp->st_ino);
-  return diff;
+  return (a[0]->fts_statp->st_ino < b[0]->fts_statp->st_ino ? 1
+ : b[0]->fts_statp->st_ino < a[0]->fts_statp->st_ino ? -1 : 0);
 }

 /*

Re: Friendlier gnulib-tool error messages with multiple incompatible modules

[Derek Price]

Ralf Wildenhues wrote:
> * Derek Price wrote on Thu, Sep 25, 2008 at 05:10:31PM CEST:
>> --- gnulib-tool revision 1.307
>> +++ gnulib-tool working copy
> 
>> +func_push ()
>> +{
>> +  var=$1
>> +  shift
>> +  for e; do
> 
> The more portable way really is
>   for e
>   do
> 
> not
>   for e; do

Okay, I've made this change.  Feels like a waste to regenerate the whole
patch again.  Any objections to my committing, assuming my previous
patch includes this change?

Regards,

Derek
-- 
Derek R. Price
Solutions Architect
Ximbiot, LLC 
Get CVS and Subversion Support from Ximbiot!

v: +1 248.835.1260
f: +1 248.246.1176

Re: fts: avoid O(n^2) ext3/ext4 performance hit on million+-entry dirs

[Bruno Haible]

Jim Meyering wrote:
> + if (nitems > _FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD
> + && !sp->fts_compar
> + && dirent_inode_sort_may_be_useful (sp)) {
> + sp->fts_compar = fts_compare_ino;
> + head = fts_sort (sp, head, nitems);
> + sp->fts_compar = NULL;
> + }

This uses fts_sort, which uses qsort, which is O(n^2) in the worst case.
(Even the implementation in glibc can be O(n^2), if it guesses that mergesort
takes too much memory.)

Couldn't this be avoided by using Paul's mpsort module, and changing fts_sort
to allocate 50% more room, as scratch space for mpsort?

Bruno

Re: Friendlier gnulib-tool error messages with multiple incompatible modules

[Bruno Haible]

Hi Derek,

> Anyone mind if I commit the attached patch?  It makes `gnulib-tool
> --update --lgpl' list all incompatible modules instead of breaking after
> the first one

This use-case is not very convincing, because
  1) gnulib-tool is not documented to accept the option --lgpl together with
 --update at all. (See "gnulib-tool --help".) Only
 "gnulib-tool --import --lgpl" makes sense.
  2) It is possible to get all the conflicts by using the --create-testdir
 option.

But I understand that some people may not think of the --create-testdir. So
ok, we can enhance --import to show all license incompatibilities.

> like so:
> 
> $ ../gnulib/gnulib-tool --update --lgpl
> gnulib-tool: *** incompatible license on modules:
> gnulib-tool: *** areadlink-with-size/GPL, canonicalize/GPL, chdir-long/GPL,

I have two objections against this output:
  1) The use of a slash is confusing. It makes people think that modules are
 directories with a file 'GPL' in it.
  2) The formatting is confusing and hard to process. In Unix, best output one
 item per line, so it can be filtered with 'grep', sorted with 'sort', etc.

> +func_wrap ()
> +{
> +  prefix=$2
> +  plen=`echo "$prefix" |wc -c`
> +  if test -z "$3"; then
> +len=80
> +  fi
> +  len=`expr $len - $plen`
> +  echo "$1" |sed -e "s#.\\{1,$len\\} #&\\n$prefix#g" -e "s#^#$prefix&#"
> +}

This is an interesting use of 'sed'. Btw, I think it would be useful to have
a library of shell script functions, not limited to those needed by autoconf.
Is it possible to implement the entire 'fmt' in 'sed'?

I've applied this. It yields tabular formatting:

gnulib-tool: *** incompatible license on modules:
 areadlink-with-size   GPL
 canonicalize  GPL
 chdir-longGPL
 chown GPL
 ...
 xstrndup  GPL
gnulib-tool: *** Stop.


2008-09-25  Derek Price  <[EMAIL PROTECTED]>
Bruno Haible  <[EMAIL PROTECTED]>

* gnulib-tool (func_import): Report all license incompatibilities, not
just the first one.

*** gnulib-tool.orig2008-09-25 20:30:06.0 +0200
--- gnulib-tool 2008-09-25 20:28:26.0 +0200
***
*** 2516,2521 
--- 2516,2522 
  
# If --lgpl, verify that the licenses of modules are compatible.
if test -n "$lgpl"; then
+ license_incompatibilities=
  for module in $main_modules; do
license=`func_get_license $module`
case $license in
***
*** 2526,2538 
  yes | 3)
case $license in
  LGPL | LGPLv2+) ;;
! *) func_fatal_error "incompatible license on module $module: 
$license" ;;
esac
;;
  2)
case $license in
  LGPLv2+) ;;
! *) func_fatal_error "incompatible license on module $module: 
$license" ;;
esac
;;
  *) func_fatal_error "invalid value lgpl=$lgpl" ;;
--- 2527,2539 
  yes | 3)
case $license in
  LGPL | LGPLv2+) ;;
! *) func_append license_incompatibilities "$module 
$license$nl" ;;
esac
;;
  2)
case $license in
  LGPLv2+) ;;
! *) func_append license_incompatibilities "$module 
$license$nl" ;;
esac
;;
  *) func_fatal_error "invalid value lgpl=$lgpl" ;;
***
*** 2540,2545 
--- 2541,2553 
;;
esac
  done
+ if test -n "$license_incompatibilities"; then
+   # Format the license incompatibilities as a table.
+   sed_expand_column1_width50_indent17='s,^\([^ ]*\) ,\1   
,
+ s,^\(.[^ ]*\) *,  
   \1 ,'
+   license_incompatibilities=`echo "$license_incompatibilities" | sed -e 
"$sed_expand_column1_width50_indent17"`
+   func_fatal_error "incompatible license on 
modules:$nl$license_incompatibilities"
+ fi
fi
  
# Show banner notice of every module.

Re: [PATCH]: Missing tests-base in gnulib-cache.m4

[Simon Josefsson]

Bruno Haible <[EMAIL PROTECTED]> writes:

>> +EXTRA_DIST += m4/gnulib-cache.m4
>>  
>>  AM_CPPFLAGS =
>>  
>> 
>> As you can see, the EXTRA_DIST contains the wrong path: it should be
>> gl/m4/gnulib-cache.m4.
>
> No, it is correct. File names in gl/Makefile.am are supposed to be relative
> to the gl/ directory.

Duh!  Of course, I'm not sure what I was thinking.

Thanks,
/Simon

Re: Friendlier gnulib-tool error messages with multiple incompatible modules

[Derek Price]

Bruno Haible wrote:

>> +func_wrap ()
>> +{
>> +  prefix=$2
>> +  plen=`echo "$prefix" |wc -c`
>> +  if test -z "$3"; then
>> +len=80
>> +  fi
>> +  len=`expr $len - $plen`
>> +  echo "$1" |sed -e "s#.\\{1,$len\\} #&\\n$prefix#g" -e "s#^#$prefix&#"
>> +}
> 
> This is an interesting use of 'sed'. Btw, I think it would be useful to have
> a library of shell script functions, not limited to those needed by autoconf.
> Is it possible to implement the entire 'fmt' in 'sed'?

Glancing over `info coreutils "fmt invocation"', I would say yes,
mostly.  --crown-margin would be challenging, but I think doable.  I'm
not sure what --tagged-paragraph is getting at, but if I understand the
documentation correctly, then its implementation should be similar to
--crown-margin.  Finally, I would probably leave out "[trying] to avoid
line breaks after the first word of a sentence or before the last word
of a sentence".

Otherwise, it looks like a workable idea.  Where would this library of
shell script functions reside?

> I've applied this. It yields tabular formatting:
> 
> gnulib-tool: *** incompatible license on modules:
>  areadlink-with-size   GPL
>  canonicalize  GPL
>  chdir-longGPL
>  chown GPL
>  ...
>  xstrndup  GPL
> gnulib-tool: *** Stop.
> 
> 
> 2008-09-25  Derek Price  <[EMAIL PROTECTED]>
> Bruno Haible  <[EMAIL PROTECTED]>
> 
>   * gnulib-tool (func_import): Report all license incompatibilities, not
>   just the first one.

Thanks!

Derek
-- 
Derek R. Price
Solutions Architect
Ximbiot, LLC 
Get CVS and Subversion Support from Ximbiot!

v: +1 248.835.1260
f: +1 248.246.1176

Re: fts: avoid O(n^2) ext3/ext4 performance hit on million+-entry dirs

[Jim Meyering]

Bruno Haible <[EMAIL PROTECTED]> wrote:
> Jim Meyering wrote:
>> +if (nitems > _FTS_INODE_SORT_DIR_ENTRIES_THRESHOLD
>> +&& !sp->fts_compar
>> +&& dirent_inode_sort_may_be_useful (sp)) {
>> +sp->fts_compar = fts_compare_ino;
>> +head = fts_sort (sp, head, nitems);
>> +sp->fts_compar = NULL;
>> +}
>
> This uses fts_sort, which uses qsort, which is O(n^2) in the worst case.
> (Even the implementation in glibc can be O(n^2), if it guesses that mergesort
> takes too much memory.)
>
> Couldn't this be avoided by using Paul's mpsort module, and changing fts_sort
> to allocate 50% more room, as scratch space for mpsort?

Good point!
That would make a fine improvement, as a separate patch.

The new use of qsort in the code I've just proposed for
rm would benefit from the same treatment.