Re: [patch] allow --with-pic to accept package names

2010-11-18 Thread Ollie Wild
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

2010-11-10 Thread Ollie Wild
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

2010-11-07 Thread Ollie Wild
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

2010-11-01 Thread Ralf Wildenhues
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

2010-11-01 Thread Ollie Wild
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

2010-11-01 Thread Ralf Wildenhues
* 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

2010-10-23 Thread Ralf Wildenhues
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

2010-10-22 Thread Peter Rosin
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

2010-10-22 Thread Peter Rosin
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

2010-10-22 Thread Ollie Wild
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

2010-10-22 Thread Peter Rosin
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

2010-10-22 Thread Eric Blake

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