Patchwork Add -lpwl to ppllibs.

login
register
mail settings
Submitter Sebastian Pop
Date Feb. 23, 2011, 6:32 a.m.
Message ID <1298442773-21455-1-git-send-email-sebpop@gmail.com>
Download mbox | patch
Permalink /patch/84106/
State New
Headers show

Comments

Sebastian Pop - Feb. 23, 2011, 6:32 a.m.
Hi Ralf,

here is the updated patch.  I tested this patch with the following
configure options:

without ppl
--with-ppl with pwl
--with-ppl without pwl
--with-ppl=/path/to/ppl_with_pwl
--with-ppl=/path/to/ppl_without_pwl
--with-ppl-libs=/path/to/ppl_with_pwl
--with-ppl-libs=/path/to/ppl_without_pwl
in-tree ppl
in-tree ppl --enable-watchdog
in-tree ppl --disable-watchdog

Usual bootstrap and regression test in progress using --with-ppl with pwl.
Ok for trunk?

Thanks,
Sebastian

---
 configure    |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 configure.ac |   26 ++++++++++++++++++-----
 2 files changed, 78 insertions(+), 11 deletions(-)
Rainer Orth - Feb. 23, 2011, 11:24 a.m.
Sebastian Pop <sebpop@gmail.com> writes:

> diff --git a/configure.ac b/configure.ac
> index 9121d65..05c295e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1621,8 +1621,9 @@ AC_ARG_WITH(boot-ldflags,
>  AC_SUBST(poststage1_ldflags)
>  
>  # Check for PPL
> -ppllibs=" -lppl_c -lppl -lgmpxx"
> +ppllibs=
>  pplinc=
> +pwllib=
>  
>  AC_ARG_WITH(ppl,
>  [AS_HELP_STRING([--with-ppl=PATH],
> @@ -1636,14 +1637,14 @@ AC_ARG_WITH(ppl-lib,
>  [AS_HELP_STRING([--with-ppl-lib=PATH],
>  		[specify directory for the installed PPL library])])
>  
> -case $with_ppl in 
> +case $with_ppl in
>    no)
> -    ppllibs=
>      ;;
>    "" | yes)
> +    ppllibs="-lppl_c -lppl"
>      ;;
>    *)
> -    ppllibs="-L$with_ppl/lib -lppl_c -lppl -lgmpxx"
> +    ppllibs="-L$with_ppl/lib -lppl_c -lppl"
>      pplinc="-I$with_ppl/include $pplinc"
>      ;;
>  esac
> @@ -1651,10 +1652,13 @@ if test "x$with_ppl_include" != x; then
>    pplinc="-I$with_ppl_include $pplinc"
>  fi
>  if test "x$with_ppl_lib" != x; then
> -  ppllibs="-L$with_ppl_lib -lppl_c -lppl -lgmpxx"
> +  ppllibs="-L$with_ppl_lib -lppl_c -lppl"
>  fi

Could you please avoid duplicating -lppl_c -lppl over and over again?
This is a complete maintenance nightmare.

Thanks.
	Rainer
Ralf Wildenhues - Feb. 23, 2011, 7:39 p.m.
* Sebastian Pop wrote on Wed, Feb 23, 2011 at 07:32:53AM CET:
> here is the updated patch.  I tested this patch with the following
> configure options:
> 
> without ppl
> --with-ppl with pwl
> --with-ppl without pwl
> --with-ppl=/path/to/ppl_with_pwl
> --with-ppl=/path/to/ppl_without_pwl
> --with-ppl-libs=/path/to/ppl_with_pwl
> --with-ppl-libs=/path/to/ppl_without_pwl
> in-tree ppl
> in-tree ppl --enable-watchdog
> in-tree ppl --disable-watchdog
> 
> Usual bootstrap and regression test in progress using --with-ppl with pwl.
> Ok for trunk?

The patch is lacking a ChangeLog entry.

Does libpwl depend on libppl_c or libppl?  If it does, then your
AC_CHECK_LIB test will fail on a system with only static libraries, as
the -lppl_c -lppl will be listed before -lpwl thus the link will fail
due to misordering.  '-l' flags generally belong in LIBS not LDFLAGS,
but in the AC_CHECK_LIB test you would need to ensure that eventual -L
flags needed to find pwl would be listed in LDFLAGS.

If libpwl does not depend on libppl_c or libppl, then there is one more
questionable case: the AC_CHECK_LIB is also tried out if ppl is in-tree.
In that case it may be the case that an out-of-tree pwl is found.  Would
that be problematic for libpwl?  If not, then the patch is OK.  If yes,
then the patch is still ok if you do the change I noted below inline.

Rainer's comment applies, but as the library list was duplicated before
your patch, I won't require fixing that issue within this patch.  It can
be done separately, and as it is not release-critical, it can be done in
Stage 1.

Thanks,
Ralf

> --- a/configure.ac
> +++ b/configure.ac
> @@ -1621,8 +1621,9 @@ AC_ARG_WITH(boot-ldflags,
>  AC_SUBST(poststage1_ldflags)
>  
>  # Check for PPL
> -ppllibs=" -lppl_c -lppl -lgmpxx"
> +ppllibs=
>  pplinc=
> +pwllib=
>  
>  AC_ARG_WITH(ppl,
>  [AS_HELP_STRING([--with-ppl=PATH],
> @@ -1636,14 +1637,14 @@ AC_ARG_WITH(ppl-lib,
>  [AS_HELP_STRING([--with-ppl-lib=PATH],
>  		[specify directory for the installed PPL library])])
>  
> -case $with_ppl in 
> +case $with_ppl in
>    no)
> -    ppllibs=
>      ;;
>    "" | yes)
> +    ppllibs="-lppl_c -lppl"
>      ;;
>    *)
> -    ppllibs="-L$with_ppl/lib -lppl_c -lppl -lgmpxx"
> +    ppllibs="-L$with_ppl/lib -lppl_c -lppl"
>      pplinc="-I$with_ppl/include $pplinc"
>      ;;
>  esac
> @@ -1651,10 +1652,13 @@ if test "x$with_ppl_include" != x; then
>    pplinc="-I$with_ppl_include $pplinc"
>  fi
>  if test "x$with_ppl_lib" != x; then
> -  ppllibs="-L$with_ppl_lib -lppl_c -lppl -lgmpxx"
> +  ppllibs="-L$with_ppl_lib -lppl_c -lppl"
>  fi
>  if test "x$with_ppl$with_ppl_include$with_ppl_lib" = x && test -d ${srcdir}/ppl; then
> -  ppllibs='-L$$r/$(HOST_SUBDIR)/ppl/interfaces/C/'"$lt_cv_objdir"' -L$$r/$(HOST_SUBDIR)/ppl/src/'"$lt_cv_objdir"' -lppl_c -lppl -lgmpxx '
> +  if test "x$enable_watchdog" = xyes; then
> +    pwllib="-lpwl"
> +  fi
> +  ppllibs='-L$$r/$(HOST_SUBDIR)/ppl/interfaces/C/'"$lt_cv_objdir"' -L$$r/$(HOST_SUBDIR)/ppl/src/'"$lt_cv_objdir"' -lppl_c -lppl '
>    pplinc='-I$$r/$(HOST_SUBDIR)/ppl/src -I$$r/$(HOST_SUBDIR)/ppl/interfaces/C '
>    enable_ppl_version_check=no
>  fi
> @@ -1677,6 +1681,16 @@ if test "x$with_ppl" != "xno" -a "${ENABLE_PPL_CHECK}" = "yes"; then
>    CFLAGS="$saved_CFLAGS"
>  fi
>  
> +if test "x$ppllibs" != x; then
> +  if test "x$pwllib" = x; then

Instead of this line, use
    if test "x$pwllib" = x && test ! -d ${srcdir}/ppl; then

> +    saved_LDFLAGS="$LDFLAGS"
> +    LDFLAGS="$LDFLAGS $ppllibs"
> +    AC_CHECK_LIB(pwl,PWL_handle_timeout,[pwllib="-lpwl"])
> +    LDFLAGS="$saved_LDFLAGS"
> +  fi
> +  ppllibs="$ppllibs $pwllib -lgmpxx"
> +fi
> +
>  # Flags needed for PPL
>  AC_SUBST(ppllibs)
>  AC_SUBST(pplinc)
Sebastian Pop - Feb. 23, 2011, 8 p.m.
On Wed, Feb 23, 2011 at 13:39, Ralf Wildenhues <ralf.wildenhues@gmx.de> wrote:
> Does libpwl depend on libppl_c or libppl?  If it does, then your

Roberto, could you please confirm that PWL is not dependent on PPL?

> AC_CHECK_LIB test will fail on a system with only static libraries, as
> the -lppl_c -lppl will be listed before -lpwl thus the link will fail
> due to misordering.  '-l' flags generally belong in LIBS not LDFLAGS,
> but in the AC_CHECK_LIB test you would need to ensure that eventual -L
> flags needed to find pwl would be listed in LDFLAGS.
>
> If libpwl does not depend on libppl_c or libppl, then there is one more
> questionable case: the AC_CHECK_LIB is also tried out if ppl is in-tree.
> In that case it may be the case that an out-of-tree pwl is found.  Would
> that be problematic for libpwl?  If not, then the patch is OK.  If yes,
> then the patch is still ok if you do the change I noted below inline.

Roberto, would it be possible to use a version of PWL different than
the version of PPL?  Again, I guess that PWL and PPL are independent.

> Rainer's comment applies, but as the library list was duplicated before
> your patch, I won't require fixing that issue within this patch.  It can
> be done separately, and as it is not release-critical, it can be done in
> Stage 1.

I already reworked the patch to address Rainer's comments.
I will send out an updated patch.  Thanks for your careful review.

Sebastian
Prof. Roberto Bagnara - Feb. 27, 2011, 7:59 a.m.
On 02/23/2011 09:00 PM, Sebastian Pop wrote:
> On Wed, Feb 23, 2011 at 13:39, Ralf Wildenhues<ralf.wildenhues@gmx.de>  wrote:
>> Does libpwl depend on libppl_c or libppl?  If it does, then your
>
> Roberto, could you please confirm that PWL is not dependent on PPL?

Confirmed: PWL does not depend on PPL.

> Roberto, would it be possible to use a version of PWL different than
> the version of PPL?  Again, I guess that PWL and PPL are independent.

The two packages are independent: they do not even share the version
number.
Cheers,

    Roberto

Patch

diff --git a/configure b/configure
index ac7db39..5b0c615 100755
--- a/configure
+++ b/configure
@@ -5660,8 +5660,9 @@  fi
 
 
 # Check for PPL
-ppllibs=" -lppl_c -lppl -lgmpxx"
+ppllibs=
 pplinc=
+pwllib=
 
 
 # Check whether --with-ppl was given.
@@ -5684,12 +5685,12 @@  fi
 
 case $with_ppl in
   no)
-    ppllibs=
     ;;
   "" | yes)
+    ppllibs="-lppl_c -lppl"
     ;;
   *)
-    ppllibs="-L$with_ppl/lib -lppl_c -lppl -lgmpxx"
+    ppllibs="-L$with_ppl/lib -lppl_c -lppl"
     pplinc="-I$with_ppl/include $pplinc"
     ;;
 esac
@@ -5697,10 +5698,13 @@  if test "x$with_ppl_include" != x; then
   pplinc="-I$with_ppl_include $pplinc"
 fi
 if test "x$with_ppl_lib" != x; then
-  ppllibs="-L$with_ppl_lib -lppl_c -lppl -lgmpxx"
+  ppllibs="-L$with_ppl_lib -lppl_c -lppl"
 fi
 if test "x$with_ppl$with_ppl_include$with_ppl_lib" = x && test -d ${srcdir}/ppl; then
-  ppllibs='-L$$r/$(HOST_SUBDIR)/ppl/interfaces/C/'"$lt_cv_objdir"' -L$$r/$(HOST_SUBDIR)/ppl/src/'"$lt_cv_objdir"' -lppl_c -lppl -lgmpxx '
+  if test "x$enable_watchdog" = xyes; then
+    pwllib="-lpwl"
+  fi
+  ppllibs='-L$$r/$(HOST_SUBDIR)/ppl/interfaces/C/'"$lt_cv_objdir"' -L$$r/$(HOST_SUBDIR)/ppl/src/'"$lt_cv_objdir"' -lppl_c -lppl '
   pplinc='-I$$r/$(HOST_SUBDIR)/ppl/src -I$$r/$(HOST_SUBDIR)/ppl/interfaces/C '
   enable_ppl_version_check=no
 fi
@@ -5744,6 +5748,55 @@  rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
   CFLAGS="$saved_CFLAGS"
 fi
 
+if test "x$ppllibs" != x; then
+  if test "x$pwllib" = x; then
+    saved_LDFLAGS="$LDFLAGS"
+    LDFLAGS="$LDFLAGS $ppllibs"
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PWL_handle_timeout in -lpwl" >&5
+$as_echo_n "checking for PWL_handle_timeout in -lpwl... " >&6; }
+if test "${ac_cv_lib_pwl_PWL_handle_timeout+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_check_lib_save_LIBS=$LIBS
+LIBS="-lpwl  $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char PWL_handle_timeout ();
+int
+main ()
+{
+return PWL_handle_timeout ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_lib_pwl_PWL_handle_timeout=yes
+else
+  ac_cv_lib_pwl_PWL_handle_timeout=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_pwl_PWL_handle_timeout" >&5
+$as_echo "$ac_cv_lib_pwl_PWL_handle_timeout" >&6; }
+if test "x$ac_cv_lib_pwl_PWL_handle_timeout" = x""yes; then :
+  pwllib="-lpwl"
+fi
+
+    LDFLAGS="$saved_LDFLAGS"
+  fi
+  ppllibs="$ppllibs $pwllib -lgmpxx"
+fi
+
 # Flags needed for PPL
 
 
diff --git a/configure.ac b/configure.ac
index 9121d65..05c295e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1621,8 +1621,9 @@  AC_ARG_WITH(boot-ldflags,
 AC_SUBST(poststage1_ldflags)
 
 # Check for PPL
-ppllibs=" -lppl_c -lppl -lgmpxx"
+ppllibs=
 pplinc=
+pwllib=
 
 AC_ARG_WITH(ppl,
 [AS_HELP_STRING([--with-ppl=PATH],
@@ -1636,14 +1637,14 @@  AC_ARG_WITH(ppl-lib,
 [AS_HELP_STRING([--with-ppl-lib=PATH],
 		[specify directory for the installed PPL library])])
 
-case $with_ppl in 
+case $with_ppl in
   no)
-    ppllibs=
     ;;
   "" | yes)
+    ppllibs="-lppl_c -lppl"
     ;;
   *)
-    ppllibs="-L$with_ppl/lib -lppl_c -lppl -lgmpxx"
+    ppllibs="-L$with_ppl/lib -lppl_c -lppl"
     pplinc="-I$with_ppl/include $pplinc"
     ;;
 esac
@@ -1651,10 +1652,13 @@  if test "x$with_ppl_include" != x; then
   pplinc="-I$with_ppl_include $pplinc"
 fi
 if test "x$with_ppl_lib" != x; then
-  ppllibs="-L$with_ppl_lib -lppl_c -lppl -lgmpxx"
+  ppllibs="-L$with_ppl_lib -lppl_c -lppl"
 fi
 if test "x$with_ppl$with_ppl_include$with_ppl_lib" = x && test -d ${srcdir}/ppl; then
-  ppllibs='-L$$r/$(HOST_SUBDIR)/ppl/interfaces/C/'"$lt_cv_objdir"' -L$$r/$(HOST_SUBDIR)/ppl/src/'"$lt_cv_objdir"' -lppl_c -lppl -lgmpxx '
+  if test "x$enable_watchdog" = xyes; then
+    pwllib="-lpwl"
+  fi
+  ppllibs='-L$$r/$(HOST_SUBDIR)/ppl/interfaces/C/'"$lt_cv_objdir"' -L$$r/$(HOST_SUBDIR)/ppl/src/'"$lt_cv_objdir"' -lppl_c -lppl '
   pplinc='-I$$r/$(HOST_SUBDIR)/ppl/src -I$$r/$(HOST_SUBDIR)/ppl/interfaces/C '
   enable_ppl_version_check=no
 fi
@@ -1677,6 +1681,16 @@  if test "x$with_ppl" != "xno" -a "${ENABLE_PPL_CHECK}" = "yes"; then
   CFLAGS="$saved_CFLAGS"
 fi
 
+if test "x$ppllibs" != x; then
+  if test "x$pwllib" = x; then
+    saved_LDFLAGS="$LDFLAGS"
+    LDFLAGS="$LDFLAGS $ppllibs"
+    AC_CHECK_LIB(pwl,PWL_handle_timeout,[pwllib="-lpwl"])
+    LDFLAGS="$saved_LDFLAGS"
+  fi
+  ppllibs="$ppllibs $pwllib -lgmpxx"
+fi
+
 # Flags needed for PPL
 AC_SUBST(ppllibs)
 AC_SUBST(pplinc)