Re: Mac OS X .dylib not working

2011-03-04 Thread Peter O'Gorman

On 03/04/2011 03:44 AM, Hans Aberg wrote:

On 4 Mar 2011, at 03:59, Peter O'Gorman wrote:



The important thing is to try .dylib - all libraries I have sen use it. It can 
of course try .so as well.


Patch. Fails new testcase before (well, new testcase as in copy 
pasted lt_dlopenext testcase and slightly modified it), passes after.

.dylib is tried after .so so there is no potentially surprising change 
in behaviour for existing applications.


Oh, I forgot a NEWS entry, will add:

On Mac OS X .dylib is now tried as well as .so with lt_dlopenext there 
before pushing.


Ok?

Peter
From 9321b9aa351a1532ca9511d2aa1e1e174b9ffbd6 Mon Sep 17 00:00:00 2001
From: Peter O'Gorman pe...@pogma.com
Date: Fri, 4 Mar 2011 12:00:23 -0600
Subject: [PATCH] On Mac OS X try .dylib as well as .so with lt_dlopenext

* libltdl/m4/ltdl.m4: Define extra extension if module extension
differs from shared lib extension.
* libltdl/ltdl.c: Use it.
* tests/darwin.at: Test it.
Reported by Hans Aberg, Michael Ellis, and others.
---
 ChangeLog  |9 ++
 libltdl/ltdl.c |   19 +
 libltdl/m4/ltdl.m4 |7 ++
 tests/darwin.at|  221 
 4 files changed, 256 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7f74eab..d8fa215 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-03-04  Peter O'Gorman  pe...@pogma.com
+
+	On Mac OS X try .dylib as well as .so with lt_dlopenext
+	* libltdl/m4/ltdl.m4: Define extra extension if module extension
+	differs from shared lib extension.
+	* libltdl/ltdl.c: Use it.
+	* tests/darwin.at: Test it.
+	Reported by Hans Aberg, Michael Ellis, and others.
+
 2011-02-12  Peter O'Gorman  pe...@pogma.com
 
 	Install ltmain.sh without execute bit set.
diff --git a/libltdl/ltdl.c b/libltdl/ltdl.c
index be1e4c0..942ed91 100644
--- a/libltdl/ltdl.c
+++ b/libltdl/ltdl.c
@@ -80,6 +80,11 @@ static  const char	libprefix[]		= LT_LIBPREFIX;
 #if defined(LT_MODULE_EXT)
 static	const char	shlib_ext[]		= LT_MODULE_EXT;
 #endif
+/* If the loadable module suffix is not the same as the linkable
+ * shared library suffix, this will be defined. */
+#if defined(LT_SHARED_EXT)
+static	const char	shared_ext[]		= LT_SHARED_EXT;
+#endif
 #if defined(LT_DLSEARCH_PATH)
 static	const char	sys_dlsearch_path[]	= LT_DLSEARCH_PATH;
 #endif
@@ -1537,6 +1542,9 @@ has_library_ext (const char *filename)
 #if defined(LT_MODULE_EXT)
 	 || (streq (ext, shlib_ext))
 #endif
+#if defined(LT_SHARED_EXT)
+	 || (streq (ext, shared_ext))
+#endif
 ))
 {
   return 1;
@@ -1682,6 +1690,17 @@ lt_dlopenadvise (const char *filename, lt_dladvise advise)
   if (handle || ((errors  0)  !file_not_found ()))
 	return handle;
 #endif
+
+#if defined(LT_SHARED_EXT)
+  /* Try appending SHARED_EXT.   */
+  LT__SETERRORSTR (saved_error);
+  errors = try_dlopen (handle, filename, shared_ext, advise);
+
+  /* As before, if the file was found but loading failed, return now
+	 with the current error message.  */
+  if (handle || ((errors  0)  !file_not_found ()))
+	return handle;
+#endif
 }
 
   /* Still here?  Then we really did fail to locate any of the file
diff --git a/libltdl/m4/ltdl.m4 b/libltdl/m4/ltdl.m4
index 42e07e9..c256e08 100644
--- a/libltdl/m4/ltdl.m4
+++ b/libltdl/m4/ltdl.m4
@@ -553,12 +553,19 @@ AC_CACHE_CHECK([which extension is used for runtime loadable modules],
 [
 module=yes
 eval libltdl_cv_shlibext=$shrext_cmds
+module=no
+eval libltdl_cv_shrext=$shrext_cmds
   ])
 if test -n $libltdl_cv_shlibext; then
   m4_pattern_allow([LT_MODULE_EXT])dnl
   AC_DEFINE_UNQUOTED([LT_MODULE_EXT], [$libltdl_cv_shlibext],
 [Define to the extension used for runtime loadable modules, say, .so.])
 fi
+if test $libltdl_cv_shrext != $libltdl_cv_shlibext; then
+  m4_pattern_allow([LT_SHARED_EXT])dnl
+  AC_DEFINE_UNQUOTED([LT_SHARED_EXT], [$libltdl_cv_shrext],
+[Define to the shared library suffix, say, .dylib.])
+fi
 ])# LT_SYS_MODULE_EXT
 
 # Old name:
diff --git a/tests/darwin.at b/tests/darwin.at
index 4e5034e..f022f50 100644
--- a/tests/darwin.at
+++ b/tests/darwin.at
@@ -228,3 +228,224 @@ mv stdout expout
 LT_AT_CONFIGURE([LDFLAGS=-L/there/is/no/dir/here])
 AT_CHECK([./libtool --config],[ignore],[expout],[ignore])
 AT_CLEANUP
+
+AT_SETUP(darwin can lt_dlopen .dylib and .so files)
+
+AT_KEYWORDS([libltdl dylib])
+
+# This test requires shared library support.
+AT_CHECK([$LIBTOOL --features | grep 'enable shared libraries' || exit 77],
+	 [], [ignore])
+
+
+eval `$LIBTOOL --config | $EGREP '^(shlibpath_var|shrext_cmds)='`
+
+module=no
+eval shared_ext=\$shrext_cmds\
+module=yes
+eval module_ext=\$shrext_cmds\
+
+# Only bother with this test if module extension is different from
+# shared extension
+AT_CHECK([test $shared_ext != $module_ext || exit 77],
+ [], [ignore])
+
+# Skip this test when called from:
+#make distcheck DISTCHECK_CONFIGURE_FLAGS=--disable-ltdl-install
+AT_CHECK([case $LIBLTDL in #(
+ 

Re: Mac OS X .dylib not working

2011-03-04 Thread Ralf Wildenhues
[ dropping bug-libtool ]

Hi Peter,

* Peter O'Gorman wrote on Fri, Mar 04, 2011 at 07:07:30PM CET:
 Ok?

A few copyright year bumps are missing.
Some minor nits inline below.

Thank you,
Ralf

 Subject: [PATCH] On Mac OS X try .dylib as well as .so with lt_dlopenext
 
 * libltdl/m4/ltdl.m4: Define extra extension if module extension
 differs from shared lib extension.
 * libltdl/ltdl.c: Use it.
 * tests/darwin.at: Test it.
 Reported by Hans Aberg, Michael Ellis, and others.

 --- a/tests/darwin.at
 +++ b/tests/darwin.at
 @@ -228,3 +228,224 @@ mv stdout expout
  LT_AT_CONFIGURE([LDFLAGS=-L/there/is/no/dir/here])
  AT_CHECK([./libtool --config],[ignore],[expout],[ignore])
  AT_CLEANUP
 +
 +AT_SETUP(darwin can lt_dlopen .dylib and .so files)

Missing m4 quotes (for style only)

 +AT_KEYWORDS([libltdl dylib])
 +
 +# This test requires shared library support.
 +AT_CHECK([$LIBTOOL --features | grep 'enable shared libraries' || exit 77],
 +  [], [ignore])
 +
 +
 +eval `$LIBTOOL --config | $EGREP '^(shlibpath_var|shrext_cmds)='`
 +
 +module=no
 +eval shared_ext=\$shrext_cmds\
 +module=yes
 +eval module_ext=\$shrext_cmds\
 +
 +# Only bother with this test if module extension is different from
 +# shared extension
 +AT_CHECK([test $shared_ext != $module_ext || exit 77],
 + [], [ignore])

You can drop arguments two and three here.

 +# This code is copied from the Autobook:
 +# http://sources.redhat.com/autobook/autobook/autobook_169.html
 +# so if it needs changes, be sure to notify the Autobook authors
 +# about them.

 +int
 +main (int argc, const char *argv[])
 +{
 +  char *errormsg = NULL;
 +  lt_dlhandle module = NULL;
 +  entrypoint *run = NULL;
 +  int errors = 0;

Isn't this lacking
  LTDL_SET_PRELOADED_SYMBOLS();

or was that needed only for static libs (which you've excluded earlier)?

 +  if (argc != 3)
 +{
 +  fprintf (stderr, USAGE: main MODULENAME ARGUMENT\n);
 +  exit (EXIT_FAILURE);
 +}
 +
 +  /* Initialise libltdl. */
 +  errors = lt_dlinit ();
 +
 +  /* Set the module search path. */
 +  if (!errors)
 +{
 +  const char *path = getenv (MODULE_PATH_ENV);
 +
 +  if (path != NULL)
 +errors = lt_dlsetsearchpath (path);
 +}
 +
 +  /* Load the module. */
 +  if (!errors)
 +module = lt_dlopenext (argv[1]);
 +
 +  /* Find the entry point. */
 +  if (module)
 +{
 +  run = (entrypoint *) lt_dlsym (module, run);
 +
 +  /* In principle, run might legitimately be NULL, so
 + I don't use run == NULL as an error indicator
 + in general. */
 +  errormsg = dlerrordup (errormsg);
 +  if (errormsg != NULL)
 +{
 +  errors = lt_dlclose (module);
 +  module = NULL;
 +}
 +}
 +  else
 +errors = 1;
 +
 +  /* Call the entry point function. */
 +  if (!errors)
 +{
 +  int result = (*run) (argv[2]);
 +  if (result  0)
 +errormsg = strdup (module entry point execution failed);
 +  else
 +printf (\t= %d\n, result);
 +}
 +
 +  /* Unload the module, now that we are done with it. */
 +  if (!errors)
 +errors = lt_dlclose (module);
 +
 +  if (errors)
 +{
 +  /* Diagnose the encountered error. */
 +  errormsg = dlerrordup (errormsg);
 +
 +  if (!errormsg)
 +{
 +  fprintf (stderr, %s: dlerror() failed.\n, argv[0]);
 +  return EXIT_FAILURE;
 +}
 +}
 +
 +  /* Finished with ltdl now. */
 +  if (!errors)
 +if (lt_dlexit () != 0)
 +  errormsg = dlerrordup (errormsg);

I'm not particularly fond of this coding style, where ownership
information essentially gets lots once an error occurs in any
of the commands.  Might be ok for a test like this, but not such
a good example for users.  lt_dlexit could be warranted even if
some error occurred before.  Anyway, I won't reject the patch for
this.

 +  if (errormsg)
 +{
 +  fprintf (stderr, %s: %s.\n, argv[0], errormsg);
 +  free (errormsg);
 +  exit (EXIT_FAILURE);
 +}
 +
 +  return EXIT_SUCCESS;
 +}
 +
 +/* Be careful to save a copy of the error message,
 +   since the  next API call may overwrite the original. */
 +static char *
 +dlerrordup (char *errormsg)
 +{
 +  char *error = (char *) lt_dlerror ();
 +  if (error  !errormsg)
 +errormsg = strdup (error);
 +  return errormsg;
 +}
 +]])

 +if test $shlibpath_var = PATH; then

This looks wrong; shouldn't it be != here?  Otherwise, ...

 +  $unset shlibpath_var || shlibpath_var=
 +fi
 +rm $libdir/simple-module.la

... this has only a small chance of succeeding.

 +rm $libdir/libsimple-dylib.la
 +
 +for dir in inst/lib $libdir; do
 +  LT_AT_EXEC_CHECK([./ltdl-loader], [], [stdout], [ignore],
 + [$dir/simple-module World])
 +  AT_CHECK([grep Hello, World stdout], [], [ignore])
 +  LT_AT_EXEC_CHECK([./ltdl-loader], [], [stdout], [ignore],
 + [$dir/libsimple-dylib World])
 +  AT_CHECK([grep Hello, World stdout], [], [ignore])
 +done
 +
 +AT_CLEANUP



Re: Mac OS X .dylib not working

2011-03-04 Thread Peter O'Gorman

On 03/04/2011 12:47 PM, Ralf Wildenhues wrote:

[ dropping bug-libtool ]

Hi Peter,

* Peter O'Gorman wrote on Fri, Mar 04, 2011 at 07:07:30PM CET:

Ok?


A few copyright year bumps are missing.


Oh, yeah, will fix.


Some minor nits inline below.



+AT_SETUP(darwin can lt_dlopen .dylib and .so files)


Missing m4 quotes (for style only)


Ok.


+AT_CHECK([test $shared_ext != $module_ext || exit 77],
+ [], [ignore])


You can drop arguments two and three here.


Ok.


+  int errors = 0;


Isn't this lacking
   LTDL_SET_PRELOADED_SYMBOLS();

or was that needed only for static libs (which you've excluded earlier)?


It's only needed for static. This bit was copypasted from lt_dlopenext.at.


+if (lt_dlexit () != 0)
+  errormsg = dlerrordup (errormsg);


I'm not particularly fond of this coding style, where ownership
information essentially gets lots once an error occurs in any
of the commands.  Might be ok for a test like this, but not such
a good example for users.  lt_dlexit could be warranted even if
some error occurred before.  Anyway, I won't reject the patch for
this.


Ok, to be honest I didn't even read much of this code, it's from 
lt_dlopenext.at.





+if test $shlibpath_var = PATH; then


This looks wrong; shouldn't it be != here?  Otherwise, ...


Again, didn't read it!! Will look into it and fix the old test if it's 
wrong.


This bit is not needed or used in the new test anyway, will remove it.

Thanks for the quick review. I will adjust and push, probably tomorrow.

Peter