Re: [patch] allow --with-pic to accept package names
On Wed, Nov 10, 2010 at 1:41 PM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote: I'm pushing the patch with the diff below squashed in, hope you don't mind. Ralf, being as you're a dual libtool / GCC maintainer, what is the process for getting libtool changes pushed to GCC? Do I just wait for a new libtool release, or is it reasonable / preferred to send individual patches to GCC as well? Thanks, Ollie
Re: [patch] allow --with-pic to accept package names
On Wed, Nov 10, 2010 at 3:41 PM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote: Thanks. The patch looks good to me, with only trivial nits: the testsuite addition should be sorted under the 'libtool script generation' banner rather than with the sysroot tests, and I don't think that it is necessary to skip the test if static libraries are disabled: your --disable-shared will (should!) lead to static libs being enabled everywhere. I'm pushing the patch with the diff below squashed in, hope you don't mind. Sounds good to me. Thanks, Ralf. Ollie
Re: [patch] allow --with-pic to accept package names
On Sat, Oct 23, 2010 at 2:32 AM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote: This patch looks ok but it uses $pkg and $p which are not in Libtool's name space, and it lacks updates to NEWS, libtool.texi, and the test suite. Oh yes, the --enable-shared code has similar problems, but a patch shouldn't be held hostage for drive-by bugs. ;-) Here's an updated patch which includes a new test, documentation, and fixed variable names. Please take another look. Thanks, Ollie 2010-11-07 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. * tests/with-pic.at: New test. * Makefile.am (TESTSUITE_AT): Add tests/with-pic.at. * doc/libtool.texi (LT_INIT): Enhance documentation of --with-pic configure flag. * NEWS (New features): Mention that --with-pic now accepts a comma-separated list of package names. 2010-11-07 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. * tests/with-pic.at: New test. * Makefile.am (TESTSUITE_AT): Add tests/with-pic.at. * doc/libtool.texi (LT_INIT): Enhance documentation of --with-pic configure flag. * NEWS (New features): Mention that --with-pic now accepts a comma-separated list of package names. diff --git a/Makefile.am b/Makefile.am index db2c0a9..53c8e8b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -511,7 +511,8 @@ TESTSUITE_AT= tests/testsuite.at \ tests/darwin.at \ tests/dumpbin-symbols.at \ tests/deplibs-mingw.at \ - tests/sysroot.at + tests/sysroot.at \ + tests/with-pic.at EXTRA_DIST += $(srcdir)/$(TESTSUITE) $(TESTSUITE_AT) $(srcdir)/tests/package.m4 diff --git a/NEWS b/NEWS index d8d692e..6930274 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,12 @@ NEWS - list of user-visible changes between releases of GNU Libtool New in 2.4.2 2010-12-??: git version 2.4.1a, Libtool team: +* New features: + + - The --with-pic configure option now supports a list of comma-separated +package names. This can be used to build some static libraries with PIC +objects while building others with non-PIC objects. + * Bug fixes: - The generic approximation of the command line length limit (when getconf is diff --git a/doc/libtool.texi b/doc/libtool.texi index 2f48a09..5c2570d 100644 --- a/doc/libtool.texi +++ b/doc/libtool.texi @@ -2042,9 +2042,10 @@ LT_PREREQ([...@value{version}]) @defmac LT_INIT (@var{options}) @defmacx AC_PROG_LIBTOOL @defmacx AM_PROG_LIBTOOL -Add support for the @option{--enable-shared} and @option{--disable-shared} -...@code{configure} fla...@footnote{@code{LT_INIT} requires that -you define the @file{Makefile} variable @code{top_builddir} in your +Add support for the @option{--enable-shared}, @option{--disable-shared}, +...@option{--enable-static}, @option{--disable-static}, @option{--with-pic}, and +...@option{--without-pic} @code{configure} fla...@footnote{@code{LT_INIT} requires +that you define the @file{Makefile} variable @code{top_builddir} in your @file{Makefile.in}. Automake does this automatically, but Autoconf users should set it to the relative path to the top of your build directory (@file{../..}, for example).} @code{AC_PROG_LIBTOOL} and @@ -2086,6 +2087,16 @@ behaves similarly, but it uses @option{--enable-static} and The package name @samp{default} matches any packages that have not set their name in the @code{PACKAGE} environment variable. +The @option{--with-pic} and @option{--without-pic} configure flags can be used +to specify whether or not @command{libtool} uses PIC objects. By default, +...@command{libtool} uses PIC objects for shared libraries and non-PIC objects for +static libraries. The @option{--with-pic} option also accepts a comma-separated +list of package names. Specifying @option{--with-p...@var{pkgs}} is the same +as configuring every package in @var{pkgs} with @option{--with-pic} and every +other package with the default configuration. The package name @samp{default} +is treated the same as for @option{--enable-shared} and +...@option{--enable-static}. + This macro also sets the shell variable @code{LIBTOOL_DEPS}, that you can use to automatically update the libtool script if it becomes out-of-date. In order to do that, add to your @file{configure.ac}: diff --git a/libltdl/m4/ltoptions.m4 b/libltdl/m4/ltoptions.m4 index 17cfd51..5d9acd8 100644 --- a/libltdl/m4/ltoptions.m4 +++ b/libltdl/m4/ltoptions.m4 @@ -326,9 +326,24 @@ dnl AC_DEFUN([AM_DISABLE_FAST_INSTALL], []) # MODE is either `yes' or `no'. If omitted, it defaults to `both'.
Re: [patch] allow --with-pic to accept package names
Hi Ollie, * Ollie Wild wrote on Fri, Oct 22, 2010 at 06:32:08PM CEST: Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. Peter, thanks for noticing the quoting bug. Updated patch attached. Thanks. The patch still has the issues I described in http://article.gmane.org/gmane.comp.gnu.libtool.patches/10924 Please indicate whether you are still working on any of those issues, and which. Thanks, Ralf 2010-10-21 Ollie Wild a...@... Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. diff --git a/libltdl/m4/ltoptions.m4 b/libltdl/m4/ltoptions.m4 index 17cfd51..160f7f2 100644 --- a/libltdl/m4/ltoptions.m4 +++ b/libltdl/m4/ltoptions.m4 @@ -326,9 +326,24 @@ dnl AC_DEFUN([AM_DISABLE_FAST_INSTALL], []) # MODE is either `yes' or `no'. If omitted, it defaults to `both'. m4_define([_LT_WITH_PIC], [AC_ARG_WITH([pic], -[AS_HELP_STRING([--with-pic], +[AS_HELP_STRING([--with-pic@:@=PKGS@:@], [try to use only PIC/non-PIC objects @:@default=use both@:@])], -[pic_mode=$withval], +[p=${PACKAGE-default} +case $withval in +yes|no) pic_mode=$withval ;; +*) + pic_mode=default + # Look at the argument we got. We use all the common list separators. + lt_save_ifs=$IFS; IFS=${IFS}$PATH_SEPARATOR, + for pkg in $withval; do + IFS=$lt_save_ifs + if test X$pkg = X$p; then + pic_mode=yes + fi + done + IFS=$lt_save_ifs + ;; +esac], [pic_mode=default]) test -z $pic_mode pic_mode=m4_default([$1], [default])
Re: [patch] allow --with-pic to accept package names
On Mon, Nov 1, 2010 at 3:44 PM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote: Hi Ollie, * Ollie Wild wrote on Fri, Oct 22, 2010 at 06:32:08PM CEST: Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. Peter, thanks for noticing the quoting bug. Updated patch attached. Thanks. The patch still has the issues I described in http://article.gmane.org/gmane.comp.gnu.libtool.patches/10924 Please indicate whether you are still working on any of those issues, and which. Sorry, Ralf. I *am* working on this, but it's relatively low priority. At this point, I just need to update the documentation. I'll send a new patch later in the week. Ollie
Re: [patch] allow --with-pic to accept package names
* Ollie Wild wrote on Mon, Nov 01, 2010 at 09:55:36PM CET: On Mon, Nov 1, 2010 at 3:44 PM, Ralf Wildenhues wrote: * Ollie Wild wrote on Fri, Oct 22, 2010 at 06:32:08PM CEST: Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. Peter, thanks for noticing the quoting bug. Updated patch attached. Thanks. The patch still has the issues I described in http://article.gmane.org/gmane.comp.gnu.libtool.patches/10924 Please indicate whether you are still working on any of those issues, and which. Sorry, Ralf. I *am* working on this, but it's relatively low priority. At this point, I just need to update the documentation. I'll send a new patch later in the week. Oh, I certainly didn't want to sound pushing. Rather, as: just in case you are not working on this any more, please give us a heads-up so some volunteer can pick it up if she so likes. Sorry, Ralf
Re: [patch] allow --with-pic to accept package names
Hello Ollie, * Ollie Wild wrote on Fri, Oct 22, 2010 at 06:32:08PM CEST: 2010-10-21 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. This patch looks ok but it uses $pkg and $p which are not in Libtool's name space, and it lacks updates to NEWS, libtool.texi, and the test suite. Oh yes, the --enable-shared code has similar problems, but a patch shouldn't be held hostage for drive-by bugs. ;-) Seriously though, if you need help with any of the remaining issues please ping us. Thanks, Ralf --- a/libltdl/m4/ltoptions.m4 +++ b/libltdl/m4/ltoptions.m4 @@ -326,9 +326,24 @@ dnl AC_DEFUN([AM_DISABLE_FAST_INSTALL], []) # MODE is either `yes' or `no'. If omitted, it defaults to `both'. m4_define([_LT_WITH_PIC], [AC_ARG_WITH([pic], -[AS_HELP_STRING([--with-pic], +[AS_HELP_STRING([--with-pic@:@=PKGS@:@], [try to use only PIC/non-PIC objects @:@default=use both@:@])], -[pic_mode=$withval], +[p=${PACKAGE-default} +case $withval in +yes|no) pic_mode=$withval ;; +*) + pic_mode=default + # Look at the argument we got. We use all the common list separators. + lt_save_ifs=$IFS; IFS=${IFS}$PATH_SEPARATOR, + for pkg in $withval; do + IFS=$lt_save_ifs + if test X$pkg = X$p; then + pic_mode=yes + fi + done + IFS=$lt_save_ifs + ;; +esac], [pic_mode=default]) test -z $pic_mode pic_mode=m4_default([$1], [default])
Re: [patch] allow --with-pic to accept package names
Hi Ollie, Den 2010-10-22 00:29 skrev Ollie Wild: This is motivated by GCC. We compile Fortran shared libraries which must execute on systems with no libgfortrans.so. The usual approach, passing --with-pic to configure is undesirable because it reduces the performance of other static libraries. Instead, I have modified --with-pic to accept a list of packages (as --enable-shared does). This allows us to configure GCC with --with-pic=libgfortran to compile only libgfortran.a with position-independent code. All tests pass with and without this change. I have not added a new test. AFAICT, the demo-[no]pic-* tests don't actually check that -fPIC is used during compilation, and there doesn't appear to be a comparable test for --enable-shared. If you would prefer a test for this feature, please provide suggestions for how to approach this (I am not familiar with the libtool testing framework). I haven't tested the patch, and I have no comment on the larger picture, but there are quoting nits: 2010-10-21 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. diff --git a/libltdl/m4/ltoptions.m4 b/libltdl/m4/ltoptions.m4 index 17cfd51..93931ec 100644 --- a/libltdl/m4/ltoptions.m4 +++ b/libltdl/m4/ltoptions.m4 @@ -326,9 +326,24 @@ dnl AC_DEFUN([AM_DISABLE_FAST_INSTALL], []) # MODE is either `yes' or `no'. If omitted, it defaults to `both'. m4_define([_LT_WITH_PIC], [AC_ARG_WITH([pic], -[AS_HELP_STRING([--with-pic], +[AS_HELP_STRING([--with-pic@:@=PKGS@:@], [try to use only PIC/non-PIC objects @:@default=use both@:@])], -[pic_mode=$withval], +[p=${PACKAGE-default} +case $withval in No quotes needed: case $withval in +yes|no) pic_mode=$withval ;; No quotes needed: yes|no) pic_mode=$withval ;; +*) + pic_mode=default + # Look at the argument we got. We use all the common list separators. + lt_save_ifs=$IFS; IFS=${IFS}$PATH_SEPARATOR, No quotes needed. + for pkg in $withval; do No quotes *allowed*. for pkg in $withval; do If you have quotes here, isn't the changed IFS meaningless? This is the issue that made me send the mail, the others are not really important. + IFS=$lt_save_ifs No quotes needed. + if test X$pkg = X$p; then + pic_mode=yes + fi + done + IFS=$lt_save_ifs No quotes needed. + ;; +esac], [pic_mode=default]) test -z $pic_mode pic_mode=m4_default([$1], [default]) Cheers, Peter
Re: [patch] allow --with-pic to accept package names
Den 2010-10-22 08:21 skrev Peter Rosin: + for pkg in $withval; do No quotes *allowed*. for pkg in $withval; do If you have quotes here, isn't the changed IFS meaningless? This is the issue that made me send the mail, the others are not really important. Oh, forgot to mention, the handling of --enable-shared does not have this quoting bug. Cheers, Peter
Re: [patch] allow --with-pic to accept package names
On Thu, Oct 21, 2010 at 5:29 PM, Ollie Wild a...@google.com wrote: This is motivated by GCC. We compile Fortran shared libraries which must execute on systems with no libgfortrans.so. The usual approach, passing --with-pic to configure is undesirable because it reduces the performance of other static libraries. Instead, I have modified --with-pic to accept a list of packages (as --enable-shared does). This allows us to configure GCC with --with-pic=libgfortran to compile only libgfortran.a with position-independent code. All tests pass with and without this change. I have not added a new test. AFAICT, the demo-[no]pic-* tests don't actually check that -fPIC is used during compilation, and there doesn't appear to be a comparable test for --enable-shared. If you would prefer a test for this feature, please provide suggestions for how to approach this (I am not familiar with the libtool testing framework). Thanks, Ollie 2010-10-21 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. Peter, thanks for noticing the quoting bug. Updated patch attached. I've removed quoting of $withval, but left $lt_save_ifs quoting in place. That's consistent with --enable-shared, and even though the new value of IFS may make that unnecessary, I'd prefer to be conservative. (BTW, please cc me directly on responses. I am not on the libtool-patches mailing list.) Ollie 2010-10-21 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. 2010-10-21 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. diff --git a/libltdl/m4/ltoptions.m4 b/libltdl/m4/ltoptions.m4 index 17cfd51..160f7f2 100644 --- a/libltdl/m4/ltoptions.m4 +++ b/libltdl/m4/ltoptions.m4 @@ -326,9 +326,24 @@ dnl AC_DEFUN([AM_DISABLE_FAST_INSTALL], []) # MODE is either `yes' or `no'. If omitted, it defaults to `both'. m4_define([_LT_WITH_PIC], [AC_ARG_WITH([pic], -[AS_HELP_STRING([--with-pic], +[AS_HELP_STRING([--with-pic@:@=PKGS@:@], [try to use only PIC/non-PIC objects @:@default=use both@:@])], -[pic_mode=$withval], +[p=${PACKAGE-default} +case $withval in +yes|no) pic_mode=$withval ;; +*) + pic_mode=default + # Look at the argument we got. We use all the common list separators. + lt_save_ifs=$IFS; IFS=${IFS}$PATH_SEPARATOR, + for pkg in $withval; do + IFS=$lt_save_ifs + if test X$pkg = X$p; then + pic_mode=yes + fi + done + IFS=$lt_save_ifs + ;; +esac], [pic_mode=default]) test -z $pic_mode pic_mode=m4_default([$1], [default])
Re: [patch] allow --with-pic to accept package names
Hi Ollie, Den 2010-10-22 18:32 skrev Ollie Wild: On Thu, Oct 21, 2010 at 5:29 PM, Ollie Wild a...@google.com wrote: 2010-10-21 Ollie Wild a...@google.com Modify --with-pic to support per-package configurations. * libltdl/m4/libtool.m4: Modify --with-pic to accept a list of package names. Modelled off --enable-shared. Peter, thanks for noticing the quoting bug. Updated patch attached. I've removed quoting of $withval, but left $lt_save_ifs quoting in place. That's consistent with --enable-shared, and even though the new value of IFS may make that unnecessary, I'd prefer to be conservative. There is never any need to quote the right-hand side of assignments, unless you have literal whitespace in them. If you do a=foo; b= bar; c=$a$b; d=$c both $c and $d will be foo bar. See 'Shell Substitutions' in the 'Portable Shell' chapter in the autoconf manual. But, there are so many examples of spurious quotes that a few more will not make much difference. I'm sure I'm not innocent either for that matter. I just stated tangential info since the problematic quoting needed fixing, and since that is fixed I don't see any need to do a re-spin just because of this. (BTW, please cc me directly on responses. I am not on the libtool-patches mailing list.) Oops, sorry about that... Cheers, Peter
Re: [patch] allow --with-pic to accept package names
On 10/22/2010 03:02 PM, Peter Rosin wrote: There is never any need to quote the right-hand side of assignments, unless you have literal whitespace in them. If you do a=foo; b= bar; c=$a$b; d=$c both $c and $d will be foo bar. See 'Shell Substitutions' in the 'Portable Shell' chapter in the autoconf manual. In fact, there's sometimes a requirement that you must NOT quote the right-hand side of assignments if you care about portability to buggy shells: a=`echo b c` is non-portable, but a=`echo b c` reliably assigns b c to $a in all shells. I just stated tangential info since the problematic quoting needed fixing, and since that is fixed I don't see any need to do a re-spin just because of this. Agreed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org