Opened 13 months ago

Closed 12 months ago

Last modified 12 months ago

#15452 closed defect (fixed)

wxrc links to libraries in build dir (on Mac OS X)

Reported by: mojca Owned by:
Priority: normal Milestone: 3.0.0
Component: build Version: 2.9.5
Keywords: Cc:
Blocked By: Blocking:
Patch: no

Description

The following script from configure.in makes sure to modify paths in all dylib files, but it forgets to change the paths in wxrc-2.9:

      *-*-darwin* )
        install_name_tool=`which ${HOST_PREFIX}install_name_tool`
        if test "$install_name_tool" -a -x "$install_name_tool"; then
            DYLIB_RPATH_POSTLINK="${HOST_PREFIX}install_name_tool -id \$@ \$@"
            cat <<EOF >change-install-names
#!/bin/sh
libnames=\`cd \${2} ; ls -1 | grep '\.[[0-9]][[0-9]]*\.dylib\$'\`
for i in \${libnames} ; do
    ${HOST_PREFIX}install_name_tool -id \${3}/\${i} \${1}/\${i}
    for dep in \${libnames} ; do
        ${HOST_PREFIX}install_name_tool -change \${2}/\${dep} \${3}/\${dep} \${1}/\${i}
    done
done
EOF
            chmod +x change-install-names
            DYLIB_RPATH_INSTALL="\$(wx_top_builddir)/change-install-names \${DESTDIR}\${libdir} \$(wx_top_builddir)/lib \${libdir}"
        fi

The following patch is currently being used in MacPorts to fix this:
http://trac.macports.org/browser/trunk/dports/graphics/wxWidgets-devel/files/patch-configure-change_install_names.diff?rev=108794
I'm not claiming that this is the most appropriate patch (if nothing else it should not hardcode wxrc-2.9), but something along these lines should probably be fixed in wxWidgets sources.

Without the patch I get a broken file:

otool -L /opt/local/bin/wxrc-2.9
/opt/local/bin/wxrc-2.9:
	/opt/local/var/macports/build/_some_path/wxWidgets-2.9.5/build/lib/libwx_baseu_xml-2.9.5.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/opt/local/var/macports/build/_some_path/wxWidgets-2.9.5/build/lib/libwx_baseu-2.9.5.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	...

On the other hand I find it a bit weird that this changes need to be done in a separate step. I would expect it from properly written autoconf/automake files to take care of this step automatically.

Apart from that wx-config creates a wrong symlink, but I might need a bit more testing rounds to be more specific. When testing I would suggest trying a non-standard prefix (./configure --prefix=/some/weird/path) and maybe

make install DESTDIR=/another/weird/path

Attachments (1)

patch-libnames.diff download (1.2 KB) - added by mojca 12 months ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 13 months ago by vadz

  • Status changed from new to confirmed
  • Summary changed from wxrc-2.9 links to libraries in build dir (on Mac OS X) & wx-config creates wrong symlink to wxrc links to libraries in build dir (on Mac OS X)

I'd prefer to not use install_name_tool too but I have no idea how to do it for the libraries. For the executables always installed in the known relative location to the libraries, it should be possible to just use @executable_path/../lib when linking it but this is probably not any simpler to do... So I'd just do roughly what the patch does (I'm not sure about ls vs find changes and passing 4 parameters to the script instead of 3, AFAICS we could avoid these changes and IMHO it would be better to), except it needs to be updated to use $(WX_RELEASE) instead of hardcoded 2.9. Could you please test if this works and attach an updated patch here?

P.S. Please open a different ticket for wrong symlinks.

comment:2 Changed 13 months ago by mojca

I took a closer look. It doesn't seem a problem to keep cd/ls instead of find, but I don't know how to avoid four parameters. That is: I don't know how to access the path to wxrc within the script.

The other change (running two loops one after another as opposed to one inside the other) seems more effective as it leads to running install_name_tool just 48 times instead of 48x48 times (assuming there are 48 libraries).

I also remembered that there is another problem with these libraries. If wxWidgets is already installed at default location, some libraries end up linking to the existing libraries instead of the ones specified by --prefix.

How can I play with the script without the need to recompile the whole wxWidgets for each test?

comment:3 Changed 13 months ago by mojca

Just for the reference (because it's difficult to see the differences in the patch), here's the original code:

#!/bin/sh

libnames=`cd ${2} ; ls -1 | grep '\.[0-9][0-9]*\.dylib$'`
for i in ${libnames} ; do
    ${HOST_PREFIX}install_name_tool -id ${3}/${i} ${1}/${i}
    for dep in ${libnames} ; do
        ${HOST_PREFIX}install_name_tool -change ${2}/${dep} ${3}/${dep} ${1}/${i}
    done
done

$(wx_top_builddir)/change-install-names ${DESTDIR}${libdir} $(wx_top_builddir)/lib ${libdir}

and here's the modified one (what's currently in MacPorts, not what I modified):

#!/bin/sh

libnames=`find -E $4  -type f -a -regex '.*\.[0-9]+\.dylib' -exec basename '{}' \;`
changes=''
for dep in $libnames; do
    changes="$changes -change $4/$dep $3/$dep"
    done
for i in $libnames; do
    ${HOST_PREFIX}install_name_tool $changes -id $3/$i $1/$i
done
${HOST_PREFIX}install_name_tool $changes $2/wxrc-2.9


$(wx_top_builddir)/change-install-names ${DESTDIR}${libdir} ${DESTDIR}${bindir} ${libdir} $(wx_top_builddir)/lib

comment:4 Changed 13 months ago by mojca

I think I found both the reason for this problem as well as a (much better) fix.

Currently the library gets created properly with a parameter

-install_name /opt/local/lib/libwx_osx_cocoau_xrc-2.9.5.dylib

Complete command is (note that /path/to is my work directory):

/usr/bin/clang++ -dynamiclib -single_module -headerpad_max_install_names -o /path/to/wxWidgets-2.9.5/build/lib/libwx_osx_cocoau_xrc-2.9.5.0.0.dylib  xrcdll_xh_animatctrl.o xrcdll_xh_bannerwindow.o xrcdll_xh_bmp.o xrcdll_xh_bmpcbox.o xrcdll_xh_bmpbt.o xrcdll_xh_bttn.o xrcdll_xh_cald.o xrcdll_xh_chckb.o xrcdll_xh_chckl.o xrcdll_xh_choic.o xrcdll_xh_choicbk.o xrcdll_xh_clrpicker.o xrcdll_xh_cmdlinkbn.o xrcdll_xh_collpane.o xrcdll_xh_combo.o xrcdll_xh_comboctrl.o xrcdll_xh_datectrl.o xrcdll_xh_dirpicker.o xrcdll_xh_dlg.o xrcdll_xh_editlbox.o xrcdll_xh_filectrl.o xrcdll_xh_filepicker.o xrcdll_xh_fontpicker.o xrcdll_xh_frame.o xrcdll_xh_gauge.o xrcdll_xh_gdctl.o xrcdll_xh_grid.o xrcdll_xh_html.o xrcdll_xh_hyperlink.o xrcdll_xh_listb.o xrcdll_xh_listbk.o xrcdll_xh_listc.o xrcdll_xh_mdi.o xrcdll_xh_menu.o xrcdll_xh_notbk.o xrcdll_xh_odcombo.o xrcdll_xh_panel.o xrcdll_xh_propdlg.o xrcdll_xh_radbt.o xrcdll_xh_radbx.o xrcdll_xh_scrol.o xrcdll_xh_scwin.o xrcdll_xh_htmllbox.o xrcdll_xh_sizer.o xrcdll_xh_slidr.o xrcdll_xh_spin.o xrcdll_xh_split.o xrcdll_xh_srchctrl.o xrcdll_xh_statbar.o xrcdll_xh_stbmp.o xrcdll_xh_stbox.o xrcdll_xh_stlin.o xrcdll_xh_sttxt.o xrcdll_xh_text.o xrcdll_xh_tglbtn.o xrcdll_xh_timectrl.o xrcdll_xh_toolb.o xrcdll_xh_toolbk.o xrcdll_xh_tree.o xrcdll_xh_treebk.o xrcdll_xh_unkwn.o xrcdll_xh_wizrd.o xrcdll_xmlres.o xrcdll_xmladv.o xrcdll_xmlrsall.o -L/path/to/wxWidgets-2.9.5/build/lib -L/path/to/wxWidgets-2.9.5/build/lib -L/path/to/wxWidgets-2.9.5/build/lib -L/path/to/wxWidgets-2.9.5/build/lib -L/path/to/wxWidgets-2.9.5/build/lib    -L/path/to/wxWidgets-2.9.5/build/lib -install_name /opt/local/lib/libwx_osx_cocoau_xrc-2.9.5.dylib   -compatibility_version 1.0 -current_version 1.0 -L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64 -L/opt/local/lib -framework IOKit -framework Carbon -framework Cocoa -framework AudioToolbox -framework System -framework OpenGL       -lpng -lz -ljpeg -ltiff -framework WebKit  -lwxregexu-2.9  -L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64 -L/opt/local/lib -framework IOKit -framework Carbon -framework Cocoa -framework AudioToolbox -framework System -framework OpenGL  -lz -lpthread -liconv  -lwx_osx_cocoau_html-2.9 -lwx_osx_cocoau_adv-2.9 -lwx_osx_cocoau_core-2.9 -lwx_baseu_xml-2.9 -lwx_baseu-2.9 -lz -lpthread -liconv

However the next command becomes:

install_name_tool -id /path/to/wxWidgets-2.9.5/build/lib/libwx_osx_cocoau_xrc-2.9.5.0.0.dylib /path/to/wxWidgets-2.9.5/build/lib/libwx_osx_cocoau_xrc-2.9.5.0.0.dylib

which spoils everything as it changes the library name from the proper one to the wrong one (which later gets kind-of-fixed in DYLIB_RPATH_INSTALL).

All I had to do to fix the problem was to delete both DYLIB_RPATH_POSTLINK="${HOST_PREFIX}install_name_tool -id \$@ \$@" and DYLIB_RPATH_INSTALL from the configure script (or rather set it to an empty string). I'm not sure how to fix the sources (bakefiles) properly though

Here's a blind guess of what I believe should be changed, but I don't know how to test these bakefiles.

--- a/build/bakefiles/common.bkl
+++ b/build/bakefiles/common.bkl
@@ -707,7 +707,6 @@ $(TAB)$(VC_COMPILER) /EP /nologo "$(DOLLAR)(InputPath)" > "$(SETUPHDIR)\wx\msw\r
         <if cond="FORMAT=='autoconf'">
             <res-include>$(RCDEFDIR)</res-include>
             <res-include>$(TOP_SRCDIR)include</res-include>
-            <postlink-command>$(DYLIB_RPATH_POSTLINK)</postlink-command>
         </if>
         <win32-res>$(WXTOPDIR)src/msw/version.rc</win32-res>
 
--- a/build/bakefiles/config.bkl
+++ b/build/bakefiles/config.bkl
@@ -426,8 +426,6 @@ to run the tests, include CppUnit library here.
         <option name="EXTRALIBS_SDL"/>
         <option name="CXXWARNINGS"/>
         <option name="HOST_SUFFIX"/>
-        <option name="DYLIB_RPATH_INSTALL"/>
-        <option name="DYLIB_RPATH_POSTLINK"/>
         <option name="SAMPLES_RPATH_FLAG"/>
 
         <!-- see configure.in; it's required by some samples on Mac OS X -->
--- a/build/bakefiles/wx.bkl
+++ b/build/bakefiles/wx.bkl
@@ -101,12 +101,6 @@
             </command>
         </action>
 
-        <modify-target target="install">
-            <command>
-                $(DYLIB_RPATH_INSTALL)
-            </command>
-        </modify-target>
-
         <set var="RCDEFS_H">
             <if cond="TOOLKIT=='MSW'">msw/rcdefs.h</if>
         </set>

--- a/configure.in
+++ b/configure.in
@@ -136,8 +136,6 @@ DEFAULT_DEFAULT_wxUSE_DFB=0
 
 PROGRAM_EXT=
 SAMPLES_RPATH_FLAG=
-DYLIB_RPATH_INSTALL=
-DYLIB_RPATH_POSTLINK=
 
 DEFAULT_STD_FLAG=yes
 
@@ -3830,23 +3828,6 @@ if test "$wxUSE_SHARED" = "yes"; then
       ;;
 
       *-*-darwin* )
-        install_name_tool=`which ${HOST_PREFIX}install_name_tool`
-        if test "$install_name_tool" -a -x "$install_name_tool"; then
-            DYLIB_RPATH_POSTLINK="${HOST_PREFIX}install_name_tool -id \$@ \$@"
-            cat <<EOF >change-install-names
-#!/bin/sh
-libnames=\`cd \${2} ; ls -1 | grep '\.[[0-9]][[0-9]]*\.dylib\$'\`
-for i in \${libnames} ; do
-    ${HOST_PREFIX}install_name_tool -id \${3}/\${i} \${1}/\${i}
-    for dep in \${libnames} ; do
-        ${HOST_PREFIX}install_name_tool -change \${2}/\${dep} \${3}/\${dep} \${1}/\${i}
-    done
-done
-EOF
-            chmod +x change-install-names
-            DYLIB_RPATH_INSTALL="\$(wx_top_builddir)/change-install-names \${DESTDIR}\${libdir} \$(wx_top_builddir)/lib \${libdir}"
-        fi
-
         dnl the HEADER_PAD_OPTION is required by some wx samples to avoid the error:
         dnl "install_name_tool: changing install names can't be redone for: the_exe_name
         dnl (for architecture ppc) because larger updated load commands do not fit
@@ -8053,8 +8034,6 @@ AC_SUBST(DEBUG_FLAG)
 TOOLKIT_LOWERCASE=`echo $TOOLKIT | tr '[[A-Z]]' '[[a-z]]'`
 AC_SUBST(TOOLKIT_LOWERCASE)
 AC_SUBST(TOOLKIT_VERSION)
-AC_SUBST(DYLIB_RPATH_INSTALL)
-AC_SUBST(DYLIB_RPATH_POSTLINK)
 AC_SUBST(SAMPLES_RPATH_FLAG)
 AC_SUBST(HEADER_PAD_OPTION)
 AC_SUBST(HOST_SUFFIX)

Maybe that part with HEADER_PAD_OPTION in configure script can also go away now that install_name_tool isn't called at all, but I don't know how to test that.

I believe that this should also fix other problems I had with wrong links to existing installation of wxWidgets.

comment:5 Changed 13 months ago by mojca

After browsing the sources for 2.8 I think I now understand why this code came to be. If one runs just make without make install, the code doesn't work (which is the expected behaviour). Current code probably wanted to do a dirty hack/workaround and first change the path to build dir during make and then fix this back to installation dir during make install, so that one could use the libraries both with or without installing.

The problem is that this simply doesn't work and it can become impossible to fix all links the way they are supposed to be, in particular if they already link to an already installed version of wxWidgets (the problem arises with wxPython which requires a different version of wxWidgets).

comment:6 Changed 13 months ago by vadz

We definitely want the libraries to work in uninstalled location, this is very useful for e.g. testing the samples and breaking this would be a huge step backwards. So I don't think we can avoid running install_name_tool during make install but then I don't see what exactly is the problem with doing this...

Just in case you want to experiment more with this, bakefile is used to generate Makefile.in and autoconf_inc.m4 which is used for creating configure from configure.in. So if you modified something that affects building the library itself only (i.e. any file under build/bakefiles) you just need to do

cd build/bakefiles && bakefile_gen -b wx.bkl -f autoconf

and if this updated autoconf_inc.m4, then rerun ./autogen.sh too.

comment:7 Changed 13 months ago by mojca

One obvious problem is that wxrc ends up with the wrong dependencies (but this could be fixed with a patch similar to the one in MacPorts). I'm not sure how to explain why this would be the wrong approach, but it definitely seems weird to me.

Tons of software I know doesn't work without installing it first. I don't consider this to be a problem since it's very easy to use something like, say,

./configure --prefix=$PWD/inst
make
make install

This is the first example of software I see that:

  1. creates a library with the -install_name flag to set its id to the final destination (which is the proper way to do it; but if you really insist in having working build dir, it would be better to change -install_name here than to do an extra DYLIB_RPATH_POSTLINK="${HOST_PREFIX}install_name_tool -id \$@ \$@" step
  2. right after creating that library, re-links the library (changes id) from final destination to working directory using DYLIB_RPATH_POSTLINK="${HOST_PREFIX}install_name_tool -id \$@ \$@"
  3. uses (slightly buggy) script to undo the second during make install

I'm actually experiencing another problem (bug?) that seems more nasty than the need to fix wxrc, but I'm not sure how to fix it properly and I'm not sure to what extent it is related. Let's say that I have wxWidgets 2.9.5 installed in /opt/local together with a lot of other dependencies that require -I/opt/local/include and -L/opt/local/lib flags. Then I try to compile wxWidgets 2.9.4 (to be used with wxPython) with ./configure --prefix=/some/weird/path and half of the libraries from wxWidgets 2.9.4 end up linking against /opt/local/lib/libwx_baseu-2.9.5.0.0.dylib rather than /some/weird/path/lib/libwx_baseu-2.9.4.0.0.dylib which breaks wxPython.

comment:8 follow-up: Changed 13 months ago by vadz

Certainly the idea of allowing to use the library without installing it is not new, e.g. libtool goes to great lengths in order to allow this. In any case, this is so useful in practice that even if we were the only ones to do it, I'd still keep it, but we're really hardly alone.

Of course, there might be better/simpler ways of achieving the same goal, in particular I have no idea why don't we link with the build directory paths and only update them when installing. And if anybody wants to improve (and especially simplify) this, it would be welcome.

But for now I'd gratefully settle for a fixed MacPorts patch...

comment:9 Changed 13 months ago by mojca

I can create a patch, but I don't know how to get away with 3 parameters as you requested because one also needs the path to wxrc.

comment:10 Changed 13 months ago by mojca

This is the script that uses cd/ls if you don't like using find, but apart from rearranging the arguments I'm not sure how to get rid of one:

#!/bin/sh

libnames=`cd ${2} ; ls -1 | grep '\.[0-9][0-9]*\.dylib$'`
changes=''
for dep in $libnames; do
    changes="$changes -change $4/$dep $3/$dep"
done
for i in $libnames; do
    ${HOST_PREFIX}install_name_tool $changes -id $3/$i $1/$i
done
${HOST_PREFIX}install_name_tool $changes $2/wxrc-$(WX_RELEASE)

$(wx_top_builddir)/change-install-names ${DESTDIR}${libdir} ${DESTDIR}${bindir} ${libdir} $(wx_top_builddir)/lib

(I don't know how to test the HOST_PREFIX though. It seems a bit weird, but I won't touch it.)

comment:11 follow-up: Changed 13 months ago by vadz

Thanks, I was wrong, we do need 4 arguments, sorry, the trick I was thinking of doesn't work.

OTOH I wonder if there is not a mistake in this script: shouldn't ${2} in the libnames assignment be ${4} actually? Or maybe ${1}? But I don't see why would we be looking for the libraries under ${bindir}...

If you could please test this once again and attach a patch that I could apply without editing it manually (as it's too simple to make a mistake with all these escapes), it would be perfect. Thanks again!

comment:12 in reply to: ↑ 11 Changed 13 months ago by mojca

Replying to vadz:

OTOH I wonder if there is not a mistake in this script: shouldn't ${2} in the libnames assignment be ${4} actually?

Yes, I'm sorry.

Or maybe ${1}?

That depends on when the script is executed. If it's executed before the libraries are copied, it has to be ${4}. If it's executed after, it has to be ${1}.

Now, this made me realize that the current approach apparently "breaks" the non-installed libraries once they are installed and nobody bothered ;)

(Explanation: the libraries can be modified while still in build dir and then copied, or they can be copied first and then modified. In the first case the libraries in build dir aren't "usable" any longer once they are modified. And this made me realize that there's another slightly weird thing going on during the second make install.)

But I don't see why would we be looking for the libraries under ${bindir}...

Not for libraries, but for wxrc. Again, here it makes a difference on when exactly this script is executed.

If you could please test this once again and attach a patch that I could apply without editing it manually (as it's too simple to make a mistake with all these escapes), it would be perfect. Thanks again!

I will. It only makes sense if you would comment on when this changes should best be done: before or after copying the libraries and the wxrc binary.

comment:13 in reply to: ↑ 8 Changed 13 months ago by neis

Replying to vadz:

Of course, there might be better/simpler ways of achieving the same goal, in particular I have no idea why don't we link with the build directory paths and only update them when installing. And if anybody wants to improve (and especially simplify) this, it would be welcome.

Can't we simply compile in the installed paths and ask people wanting to use uninstalled libs to just use DYLD_LIBRARY_PATH which overrides the paths that where built into binaries?

comment:14 Changed 12 months ago by vadz

Sorry for the delay with the reply.

I'd definitely prefer to avoid modifying the libraries in the build dir. So ideally we'd copy them (and wxrc) first and then modify them in their final locations.

If you could make a patch doing this, it would be perfect, TIA!

Changed 12 months ago by mojca

comment:15 Changed 12 months ago by mojca

Telepathy on work ;) I didn't see your reply before submitting the patch.

I'm not familiar enough with your build system to know where to plug in any changes (and I also sympathize with neis' suggestion to let the users adjust the DYLD_LIBRARY_PATH).

comment:16 Changed 12 months ago by vadz

I'm applying your patch as is, there is no need to plug it into anything (configure.in is not generated), thanks!

As for DYLD_LIBRARY_PATH it's the same thing as before: the people who know about it are precisely the people we don't particularly care about because they're qualified enough to do whatever it takes, including running install_name_tool themselves. We do care about people totally new to OS X and this is why we want the samples to run out of the box.

comment:17 Changed 12 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [74909]) Install wxrc with proper library dependencies under OS X.

In addition to changing the libraries themselves to point to the dependencies
in their installed location, we also need to do the same thing for wxrc when
installing it under OS X, otherwise it wouldn't run once the libraries are not
available in their original location any more.

Closes #15452.

comment:18 Changed 12 months ago by VZ

(In [74921]) Fix install_name_tool calls in OS X "make install".

Unfortunately the changes of r74909 (see #15452) don't seem to have been
tested and broke "make install" completely as libraries were not found in the
"bin" directory where the script was looking for them. Fix it to use "lib"
subdirectory as intended.

Closes #15551.

Note: See TracTickets for help on using tickets.