Re: cygwin build problem with m4 HEAD

[Ralf Wildenhues]

[ 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 <address@hidden>.

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 -0000       1.5
+++ libltdl/argz.c      14 Sep 2005 15:56:38 -0000
@@ -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 -0000       1.161
+++ Makefile.am 14 Sep 2005 15:56:38 -0000
@@ -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 -0000      1.231
+++ libltdl/ltdl.c      14 Sep 2005 15:56:38 -0000
@@ -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 <address@hidden>
 
    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, before, entry);
+  else
+    error = argz_append (pargz, pargz_len, entry, 1 + strlen (entry));
+
+  if (error)
     {
       switch (error)
        {

argz-workaround-1-5
Description: Text document