bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: first draft of "relocatable" module


From: Bruno Haible
Subject: Re: first draft of "relocatable" module
Date: Thu, 1 Mar 2007 03:50:43 +0100
User-agent: KMail/1.5.4

Hello Ben,

> I'm attaching updated sets of diffs, same ones as before.

Thanks a lot. I committed them, with minor tweaks.

> OK.  Now I've put essentially that text into
> doc/relocatable.texi, with part of the introduction from what was
> there before, and added doc/relocatable.texi to the module
> description.  Then I moved the previous text into new file
> doc/relocatable-maint.texi.

This is good, thanks.

> >>         * relocatable.c is from gettext, but I was able to take
> >>           advantage of new gnulib modules to drop quite a bit of
> >>           boilerplate from the beginning.  Perhaps my use of the
> >>           c-ctype module will be rejected, but I hope that the
> >>           rest is not controversial.
> >
> > However, relocatable.c is also used in libraries that are made
> > relocatable. ...
> 
> OK, I reverted these parts.

I also reverted the use of the c-ctype module in this file, to make its
inclusion in libraries as easy as possible. However, in relocwrapper.c
or progreloc.c the use of c-ctype is fine.

> > Also I didn't see where you moved the _GNU_SOURCE setting to.
> 
> I depended on the "extensions" module, which calls AC_GNU_SOURCE.
> Do you prefer to #define _GNU_SOURCE explicitly in the source file?

Yes, for a file that should be includable in any library, this is better
than to rely on the 'extensions' module.

> >>           Ditto for relocatable.m4.
> >
> > Where did the AC_EXEEXT invocation go?
> 
> Autoconf 2.50 had this item in its NEWS:
> 
>     - AC_EXEEXT, AC_OBJEXT
>       You are no longer expected to use them: their computation is
>       performed by default.
> 
> and AC_EXEEXT is listed in the "Obsolete Macros" section of the
> Autoconf manual.  So it seemed safe to drop it.

Ah, well spotted! I missed this announcement.

> >>         * relocatable.sh.in is an extract from gettext's
> >>           gettextize.sh.in explained in relocatable.texi.
> >
> > OK. But the largest part of this shell script code is not to be modified
> > by the maintainer. Can you make this code available through a single
> > autoconf variable (assuming autoconf-2.61) or through some Makefile.am
> > rule? For the part which contains the variables needed by the script,
> > this is obviously not fully automatable on the gnulib side; documenting
> > the customization/adaptation steps, like you did, is fine.
> 
> OK, done.

AC_SUBST_FILE - nice macro! I added the func_tmpfile to relocatable.sh.in,
otherwise it wasn't self contained. Also I moved it from lib/ to build-aux/ -
since it's not a file that is compiled or included.

> >>         * I wrote modules/relocatable myself.
> >
> > This is GPL, but libraries need part of it under LGPL, namely: 
> > relocatable.h,
> > relocatable.c, and the macros AC_RELOCATABLE_LIBRARY and AC_RELOCATABLE_NOP
> > from relocatable.m4? Can you group them into a module 'relocatable-lib'
> > that 'relocatable' depends upon?
> 
> OK, done.

Thanks. But it turns out that 'relocatable' cannot depend on 'relocatable-lib':
The relocatable-lib module, if it wants to be LGPL, must not use 'xalloc'.
So it needs "DEFS += -DNO_XMALLOC". But this define is bad for the GPLed
'relocatable'. So what I did is to copy all contents of 'relocatable-lib'
to 'relocatable' - except precisely this -DNO_XMALLOC setting.

> > I'm not sure whether then it's also worth making a separate
> > 'relocatable-script' module that takes care of the mechanics for the
> > relocatable shell scripts?
> 
> Yes, I think so: if we're going to be copying in an extra file
> (lib/relocatable.sh.in) for that case, then we might as well make
> it separate to avoid the need in packages that do not install any
> shell scripts.  Done.

Thanks.

> >>         * @SET_RELOCATABLE@ just expands to a variable
> >>           definition.  Do we need this extra level of
> >>           indirection?
> >
> > What could you simplify here? I don't see it.
> 
> I mean, why not do this:
> 
>     @@ -40,8 +52,9 @@
>            mingw*) is_noop=yes ;;
>            linux*) use_elf_origin_trick=yes ;;
>          esac
>          if test $is_noop = yes; then
>     -      SET_RELOCATABLE="RELOCATABLE_LDFLAGS = :"
>     +      RELOCATABLE_LDFLAGS=:
>     +      AC_SUBST([RELOCATABLE_LDFLAGS])
>          else
>            if test $use_elf_origin_trick = yes; then
>              dnl Use the dynamic linker's support for relocatable programs.
> 
> and so on.  It seems to work.  Am I missing something?

Nice simplification. Probably I was too much thinking about having a small
'Makefile.in' - but this is a non-issue: 10 more or 10 less AC_SUBSTed
variables don't matter.

There is still one problem: The comment at the head of gl_RELOCATABLE
says that it accepts an argument. But the macro uses always $gl_source_base,
not $1.

For reference, here are the changes that I did, compared to your patch.

Bruno


Only in /packages/GNULIB/gnulib-cvs/build-aux: relocatable.sh.in
diff -r -u --exclude=CVS gnulib-cvs/lib/progreloc.c 
/packages/GNULIB/gnulib-cvs/lib/progreloc.c
--- gnulib-cvs/lib/progreloc.c  2007-03-01 03:45:43.000000000 +0100
+++ /packages/GNULIB/gnulib-cvs/lib/progreloc.c 2007-03-01 02:23:04.000000000 
+0100
@@ -172,7 +172,7 @@
   {
     char *link;
 
-    link = xreadlink ("/proc/self/exe", 4096);
+    link = xreadlink ("/proc/self/exe");
     if (link != NULL && link[0] != '[')
       return link;
     if (executable_fd < 0)
@@ -181,7 +181,7 @@
     {
       char buf[6+10+5];
       sprintf (buf, "/proc/%d/exe", getpid ());
-      link = xreadlink (buf, 4096);
+      link = xreadlink (buf);
       if (link != NULL && link[0] != '[')
        return link;
       if (executable_fd < 0)
diff -r -u --exclude=CVS gnulib-cvs/lib/relocatable.c 
/packages/GNULIB/gnulib-cvs/lib/relocatable.c
--- gnulib-cvs/lib/relocatable.c        2007-03-01 03:45:43.000000000 +0100
+++ /packages/GNULIB/gnulib-cvs/lib/relocatable.c       2006-09-15 
14:01:51.000000000 +0200
@@ -1,5 +1,5 @@
 /* Provide relocatable packages.
-   Copyright (C) 2003-2006, 2007 Free Software Foundation, Inc.
+   Copyright (C) 2003-2006 Free Software Foundation, Inc.
    Written by Bruno Haible <address@hidden>, 2003.
 
    This program is free software; you can redistribute it and/or modify it
@@ -17,6 +17,14 @@
    Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
    USA.  */
 
+
+/* Tell glibc's <stdio.h> to provide a prototype for getline().
+   This must come before <config.h> because <config.h> may include
+   <features.h>, and once <features.h> has been included, it's too late.  */
+#ifndef _GNU_SOURCE
+# define _GNU_SOURCE   1
+#endif
+
 #include <config.h>
 
 /* Specification.  */
@@ -216,8 +224,6 @@
 
        while (rpi > rel_installdir && cpi > cp_base)
          {
-            unsigned char c1, c2;
-            
            rpi--;
            cpi--;
            if (ISSLASH (*rpi) || ISSLASH (*cpi))
@@ -226,18 +232,16 @@
                  same = true;
                break;
              }
-
            /* Do case-insensitive comparison if the filesystem is always or
               often case-insensitive.  It's better to accept the comparison
               if the difference is only in case, rather than to fail.  */
-            c1 = *rpi;
-            c2 = *cpi;
 #if defined _WIN32 || defined __WIN32__ || defined __CYGWIN__ || defined 
__EMX__ || defined __DJGPP__
            /* Win32, Cygwin, OS/2, DOS - case insignificant filesystem */
-           if (c_toupper (c1) != c_toupper (c2))
+           if ((*rpi >= 'a' && *rpi <= 'z' ? *rpi - 'a' + 'A' : *rpi)
+               != (*cpi >= 'a' && *cpi <= 'z' ? *cpi - 'a' + 'A' : *cpi))
              break;
 #else
-           if (c1 != c2)
+           if (*rpi != *cpi)
              break;
 #endif
          }
Only in gnulib-cvs/lib: relocatable.sh.in
diff -r -u --exclude=CVS gnulib-cvs/m4/relocatable.m4 
/packages/GNULIB/gnulib-cvs/m4/relocatable.m4
--- gnulib-cvs/m4/relocatable.m4        2007-03-01 03:45:43.000000000 +0100
+++ /packages/GNULIB/gnulib-cvs/m4/relocatable.m4       2007-03-01 
02:56:19.000000000 +0100
@@ -75,10 +75,11 @@
   AM_CONDITIONAL([RELOCATABLE_VIA_LD],
     [test $is_noop = yes || test $use_elf_origin_trick = yes])
 
+  dnl RELOCATABLE_LIBRARY_PATH can be set in configure.ac. Default is empty.
   AC_SUBST([RELOCATABLE_LIBRARY_PATH])
+  AC_SUBST([RELOCATABLE_CONFIG_H_DIR])
   AC_SUBST([RELOCATABLE_SRC_DIR])
   AC_SUBST([RELOCATABLE_BUILD_DIR])
-  AC_SUBST([RELOCATABLE_CONFIG_H_DIR])
 ])
 
 dnl Determine the platform dependent parameters needed to use relocatability:
diff -r -u --exclude=CVS gnulib-cvs/modules/relocatable 
/packages/GNULIB/gnulib-cvs/modules/relocatable
--- gnulib-cvs/modules/relocatable      2007-03-01 03:45:43.000000000 +0100
+++ /packages/GNULIB/gnulib-cvs/modules/relocatable     2007-03-01 
03:08:35.000000000 +0100
@@ -6,32 +6,38 @@
 build-aux/config.libpath
 build-aux/install-reloc
 build-aux/reloc-ldflags
+doc/relocatable.texi
+lib/relocatable.h
+lib/relocatable.c
 lib/progreloc.c
 lib/relocwrapper.c
 m4/relocatable.m4
+m4/relocatable-lib.m4
 
 Depends-on:
 relocatable-lib
 canonicalize
 c-ctype
-extensions
 progname
 setenv
 strerror
 unistd
+xalloc
 
 configure.ac:
 gl_RELOCATABLE([$gl_source_base])
 
 Makefile.am:
 DEFS += -DEXEEXT=\"$(EXEEXT)\"
-lib_SOURCES += progreloc.c
+lib_SOURCES += relocatable.c progreloc.c
 
 Include:
-"progname.h"
+#include "relocatable.h"
+#include "progname.h"
 
 License:
 GPL
 
 Maintainer:
+Bruno Haible, Ben Pfaff
 
diff -r -u --exclude=CVS gnulib-cvs/modules/relocatable-lib 
/packages/GNULIB/gnulib-cvs/modules/relocatable-lib
--- gnulib-cvs/modules/relocatable-lib  2007-03-01 03:45:43.000000000 +0100
+++ /packages/GNULIB/gnulib-cvs/modules/relocatable-lib 2007-03-01 
03:08:49.000000000 +0100
@@ -4,8 +4,8 @@
 
 Files:
 doc/relocatable.texi
-lib/relocatable.c
 lib/relocatable.h
+lib/relocatable.c
 m4/relocatable-lib.m4
 
 Depends-on:
@@ -13,7 +13,8 @@
 configure.ac:
 
 Makefile.am:
-lib_SOURCES += relocatable.c relocatable.h
+DEFS += -DNO_XMALLOC
+lib_SOURCES += relocatable.c
 
 Include:
 "relocatable.h"
@@ -22,3 +23,5 @@
 LGPL
 
 Maintainer:
+Bruno Haible, Ben Pfaff
+
diff -r -u --exclude=CVS gnulib-cvs/modules/relocatable-script 
/packages/GNULIB/gnulib-cvs/modules/relocatable-script
--- gnulib-cvs/modules/relocatable-script       2007-03-01 03:45:43.000000000 
+0100
+++ /packages/GNULIB/gnulib-cvs/modules/relocatable-script      2007-03-01 
03:09:07.000000000 +0100
@@ -3,14 +3,14 @@
 function properly when copied to an arbitrary directory.
 
 Files:
-lib/relocatable.sh.in
+build-aux/relocatable.sh.in
 
 Depends-on:
 relocatable
 
 configure.ac:
-AC_SUBST_FILE(relocatable_sh)
-relocatable_sh=$gl_source_base/relocatable.sh.in
+relocatable_sh=$ac_aux_dir/relocatable.sh.in
+AC_SUBST_FILE([relocatable_sh])
 
 Makefile.am:
 
@@ -20,4 +20,5 @@
 GPL
 
 Maintainer:
+Bruno Haible, Ben Pfaff
 





reply via email to

[Prev in Thread] Current Thread [Next in Thread]