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

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