Re: cygwin build problem with m4 HEAD
[ replacing bug-libtool with libtool-patches ]
Hi Eric, others,
* Eric Blake wrote on Fri, Sep 09, 2005 at 03:36:54PM CEST:
According to Ralf Wildenhues on 9/9/2005 6:46 AM:
I hate runtime checks. This one will mean hopeless complication of
things, and worse results for any kind of cross compilation. And all of
this for a system where people are unlikely to run year-old
installations anyway. Besides, the workaround costs like what? Two
lines of code. Multiply by a hundred for the test + replacement.
True enough - to date, only newlib has been identified with the bug, the
bug is now fixed, and will show up in the next mingw and cygwin release.
Maybe you could be optimistic when cross-compiling and assume there is no
bug (since cross-compiling is hard enough that they are likely to have an
up-to-date version of newlib). But if you don't want to do the runtime
test at all, it is somewhat your call as maintainer of the gnulib argz module.
Well, look at it from this perspective: both newlib and libtool are free
software. Updating either is bound to be a similar amount of work
(unless newer versions contain incompatibilities). Updating newlib
fixes all instances of this bug, whereas recreating a configure script
fixes only this particular package.
I seriously believe expensive configure checks should only be done for
software that is not as easily updated/fixed. In other cases, just put
the fix in the right place: newlib. And convince users to use recent
versions of that.
Well, currently, neither gnulib/lib/argz_.h nor argz.c have documentation,
so that may be the best approach - just documenting all of the argz_*
functions, and stating that in the spirit of portability to older versions
of newlib, use argz_append instead of argz_insert(,,NULL,).
I'll happily accept a patch for this (or look into it eventually
myself). Given need of other argz_* functions for other users, it may
even be useful to add them.
By the way, I
was also able to configure m4 with ac_cv_func_argz_insert=no
ac_cv_type_error_t=no to force the use of gnulib's argz module in
preference to newlib's system argz_* without even patching libtool.
Good.
So maybe this libtool patch can be applied as a bandaid to the current
cygwin 1.5.18, then in another year or so be reverted when all known sane
systems that provide argz_insert have correctly accepted NULL for a while.
Yes, a good idea.
I have applied both this patch and a small change to argz.c to
Libtool (the newlib workaround to both HEAD and branch-1-5). The
resulting patches are shown below/attached.
Should I send a complete updated copy of argz.c to bug-gnulib?
Cheers,
Ralf
* libltdl/argz.c HAVE_CONFIG_H: Avoid redefinition warning.
* libltdl/ltdl.c (lt_argz_insert): Work around newlib
argz_insert bug.
* Makefile.am (VERSION_INFO): Bumped revision.
Reported by Eric Blake [EMAIL PROTECTED].
Index: libltdl/argz.c
===
RCS file: /cvsroot/libtool/libtool/libltdl/argz.c,v
retrieving revision 1.5
diff -u -r1.5 argz.c
--- libltdl/argz.c 1 Jun 2005 19:09:00 - 1.5
+++ libltdl/argz.c 14 Sep 2005 15:56:38 -
@@ -29,6 +29,7 @@
/* Provide our wierdo HAVE_CONFIG_H rvalue for other clients. */
#if !defined(LTDL) defined(HAVE_CONFIG_H)
+# undef HAVE_CONFIG_H
# define HAVE_CONFIG_H config.h
#endif
Index: Makefile.am
===
RCS file: /cvsroot/libtool/libtool/Makefile.am,v
retrieving revision 1.161
diff -u -r1.161 Makefile.am
--- Makefile.am 5 Sep 2005 06:21:48 - 1.161
+++ Makefile.am 14 Sep 2005 15:56:38 -
@@ -230,7 +230,7 @@
AM_CPPFLAGS= -I. -I$(srcdir) -Ilibltdl -I$(srcdir)/libltdl \
-I$(srcdir)/libltdl/libltdl
AM_LDFLAGS = -no-undefined
-VERSION_INFO = -version-info 6:0:0
+VERSION_INFO = -version-info 6:1:0
noinst_LTLIBRARIES = $(LT_DLLOADERS)
Index: libltdl/ltdl.c
===
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.231
diff -u -r1.231 ltdl.c
--- libltdl/ltdl.c 28 Jul 2005 10:01:03 - 1.231
+++ libltdl/ltdl.c 14 Sep 2005 15:56:38 -
@@ -1,5 +1,5 @@
/* ltdl.c -- system independent dlopen wrapper
- Copyright (C) 1998, 1999, 2000, 2004 Free Software Foundation, Inc.
+ Copyright (C) 1998, 1999, 2000, 2004, 2005 Free Software Foundation, Inc.
Originally by Thomas Tanner [EMAIL PROTECTED]
NOTE: The canonical source of this file is maintained with the
@@ -1445,7 +1445,14 @@
{
error_t error;
- if ((error = argz_insert (pargz, pargz_len, before, entry)))
+ /* Prior to Sep 8, 2005, newlib had a bug where argz_insert(pargz,
+ pargz_len, NULL, entry) failed with EINVAL. */
+ if (before)
+error = argz_insert (pargz, pargz_len,
Re: cygwin build problem with m4 HEAD
On 2005-09-14, Ralf Wildenhues [EMAIL PROTECTED] wrote: --- libltdl/argz.c1 Jun 2005 19:09:00 - 1.5 +++ libltdl/argz.c14 Sep 2005 15:56:38 - @@ -29,6 +29,7 @@ /* Provide our wierdo HAVE_CONFIG_H rvalue for other clients. */ #if !defined(LTDL) defined(HAVE_CONFIG_H) +# undef HAVE_CONFIG_H # define HAVE_CONFIG_H config.h #endif While you're patching, wierdo should be spelled weirdo. Cheers, Olly ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: cygwin build problem with m4 HEAD
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Ralf Wildenhues on 9/9/2005 1:31 AM:
ltdl.c, in lt_argz_insert, blindly calls argz_insert without checking whether
the before argument is NULL. But newlib (up until my patch posted there
today
is incorporated, http://sources.redhat.com/ml/newlib/2005/msg00516.html)
treated this as EINVAL, thus breaking load_deplibs() and failing every single
testcase of the m4 testsuite because of a failure to initialize.
Thank you so much for analyzing this and providing a fix, I owe you a
beer for that one!
Your fix is not quite appropriate for inclusion, however, because our
replacement argz.c does not provide argz_add. Does newlib have a
similar bug in argz_append?
No - the newlib bug in argz_insert was that it called argz_add and
correctly appended the string, but then a missing return statement made it
fall through to the next line which checked for before being out of range,
such that it returned EINVAL on success, and overwrote the proper ENOMEM
on failure. Adding the return statement fixed newlib, although that means
there are systems in the wild (such as cygwin 1.5.18) with the bug based
on when they compiled newlib. I checked newlib's argz_append, and it
works just fine.
If not, then the patch below should work as
well, I believe (untested with newlib), and is less work than providing
argz_add as well. Could you confirm this? If ok, I'll apply this CVS
HEAD and backport to branch-1-5.
Don't apply the patch as is - I've thought about it a bit more. I think a
better patch, in the spirit of gnulib, is to update m4/argz.m4 to do a
configure-time run test to see if the system argz_insert is broken, in
which case libtool's argz_insert is used instead. Leave libtool's ltdl.c
alone - no need to dirty it with workarounds that can be solved by fixing
argz_insert instead. Besides, if any other project uses gnulib's argz
module, they should get correct behavior as well. Unfortunately, I
imagine that such a fix to the argz.m4 file will have to be pessimistic
during cross-compiles, and is further complicated by the fact that argz.c
should probably provide only a replacement argz_insert that falls back on
the system argz_append if it was only the system argz_insert that is
broken. However, my employer has not signed a copyright disclaimer,
despite my repeated requests (although I'm making progress, and hope to
get it one of these days), and the amount of m4 magic needed to add a
runtime check and improve argz.c is most likely beyond the trivial patch
limits.
By the way, how does the m4 testsuite fare on cygwin with this fixed?
It dropped from 76 failures (ie. every single test) down to 9 failures
(I'll post further info to the m4 lists).
Cheers,
Ralf
* libltdl/ltdl.c (lt_argz_insert): Work around newlib argz_insert bug.
* Makefile.am (VERSION_INFO): Bumped revision.
Reported by Eric Blake [EMAIL PROTECTED].
Index: Makefile.am
===
RCS file: /cvsroot/libtool/libtool/Makefile.am,v
retrieving revision 1.161
diff -u -r1.161 Makefile.am
--- Makefile.am 5 Sep 2005 06:21:48 - 1.161
+++ Makefile.am 9 Sep 2005 07:21:02 -
@@ -230,7 +230,7 @@
AM_CPPFLAGS = -I. -I$(srcdir) -Ilibltdl -I$(srcdir)/libltdl \
-I$(srcdir)/libltdl/libltdl
AM_LDFLAGS = -no-undefined
-VERSION_INFO = -version-info 6:0:0
+VERSION_INFO = -version-info 6:1:0
noinst_LTLIBRARIES = $(LT_DLLOADERS)
Index: libltdl/ltdl.c
===
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.231
diff -u -r1.231 ltdl.c
--- libltdl/ltdl.c28 Jul 2005 10:01:03 - 1.231
+++ libltdl/ltdl.c9 Sep 2005 07:21:02 -
@@ -1,5 +1,5 @@
/* ltdl.c -- system independent dlopen wrapper
- Copyright (C) 1998, 1999, 2000, 2004 Free Software Foundation, Inc.
+ Copyright (C) 1998, 1999, 2000, 2004, 2005 Free Software Foundation, Inc.
Originally by Thomas Tanner [EMAIL PROTECTED]
NOTE: The canonical source of this file is maintained with the
@@ -1445,7 +1445,12 @@
{
error_t error;
- if ((error = argz_insert (pargz, pargz_len, before, entry)))
+ if (before)
+error = argz_insert (pargz, pargz_len, before, entry);
+ else
+error = argz_append (pargz, pargz_len, entry, 1 + strlen (entry));
+
+ if (error)
{
switch (error)
{
- --
Life is short - so eat dessert first!
Eric Blake [EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFDIXKS84KuGfSFAYARAr/WAJ9+otyfidVTsHWCLtPOUOqEqq/qSACdF4w4
KIRsezZCrcvJ/3dD3kIv90k=
=Ejrm
-END PGP SIGNATURE-
Re: cygwin build problem with m4 HEAD
[ taking libtool-patches back out ] Hi Eric, * Eric Blake wrote on Fri, Sep 09, 2005 at 01:31:30PM CEST: According to Ralf Wildenhues on 9/9/2005 1:31 AM: If not, then the patch below should work as well, I believe (untested with newlib), and is less work than providing argz_add as well. Could you confirm this? If ok, I'll apply this CVS HEAD and backport to branch-1-5. Don't apply the patch as is - I've thought about it a bit more. I think a better patch, in the spirit of gnulib, is to update m4/argz.m4 to do a configure-time run test to see if the system argz_insert is broken, in which case libtool's argz_insert is used instead. Leave libtool's ltdl.c alone - no need to dirty it with workarounds that can be solved by fixing argz_insert instead. Besides, if any other project uses gnulib's argz module, they should get correct behavior as well. Unfortunately, I imagine that such a fix to the argz.m4 file will have to be pessimistic during cross-compiles, and is further complicated by the fact that argz.c should probably provide only a replacement argz_insert that falls back on the system argz_append if it was only the system argz_insert that is broken. I hate runtime checks. This one will mean hopeless complication of things, and worse results for any kind of cross compilation. And all of this for a system where people are unlikely to run year-old installations anyway. Besides, the workaround costs like what? Two lines of code. Multiply by a hundred for the test + replacement. However, my employer has not signed a copyright disclaimer, despite my repeated requests (although I'm making progress, and hope to get it one of these days), and the amount of m4 magic needed to add a runtime check and improve argz.c is most likely beyond the trivial patch limits. I could attempt such a patch, but do not see its value outweighing its costs, see above. In matters like these, I would _by far_ prefer a documentation of such limitations, in the spirit of the Autoconf manual. By the way, how does the m4 testsuite fare on cygwin with this fixed? It dropped from 76 failures (ie. every single test) down to 9 failures (I'll post further info to the m4 lists). Quite a relief. Thanks for checking. Cheers, Ralf * libltdl/ltdl.c (lt_argz_insert): Work around newlib argz_insert bug. * Makefile.am (VERSION_INFO): Bumped revision. Reported by Eric Blake [EMAIL PROTECTED]. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: cygwin build problem with m4 HEAD
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Ralf Wildenhues on 9/9/2005 6:46 AM: I hate runtime checks. This one will mean hopeless complication of things, and worse results for any kind of cross compilation. And all of this for a system where people are unlikely to run year-old installations anyway. Besides, the workaround costs like what? Two lines of code. Multiply by a hundred for the test + replacement. True enough - to date, only newlib has been identified with the bug, the bug is now fixed, and will show up in the next mingw and cygwin release. Maybe you could be optimistic when cross-compiling and assume there is no bug (since cross-compiling is hard enough that they are likely to have an up-to-date version of newlib). But if you don't want to do the runtime test at all, it is somewhat your call as maintainer of the gnulib argz module. However, my employer has not signed a copyright disclaimer, despite my repeated requests (although I'm making progress, and hope to get it one of these days), and the amount of m4 magic needed to add a runtime check and improve argz.c is most likely beyond the trivial patch limits. I could attempt such a patch, but do not see its value outweighing its costs, see above. In matters like these, I would _by far_ prefer a documentation of such limitations, in the spirit of the Autoconf manual. Well, currently, neither gnulib/lib/argz_.h nor argz.c have documentation, so that may be the best approach - just documenting all of the argz_* functions, and stating that in the spirit of portability to older versions of newlib, use argz_append instead of argz_insert(,,NULL,). By the way, I was also able to configure m4 with ac_cv_func_argz_insert=no ac_cv_type_error_t=no to force the use of gnulib's argz module in preference to newlib's system argz_* without even patching libtool. * libltdl/ltdl.c (lt_argz_insert): Work around newlib argz_insert bug. * Makefile.am (VERSION_INFO): Bumped revision. Reported by Eric Blake [EMAIL PROTECTED]. So maybe this libtool patch can be applied as a bandaid to the current cygwin 1.5.18, then in another year or so be reverted when all known sane systems that provide argz_insert have correctly accepted NULL for a while. - -- Life is short - so eat dessert first! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.1 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDIY/284KuGfSFAYARAibbAKCM7m5HHHyEBEy1lUb5EBl4mPrd3ACg0PnP +YczwIyajB+6Dv/+xPRMmbg= =RBi/ -END PGP SIGNATURE- ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
