GNOME Bugzilla – Bug 744772
systemtap and gdb scripts install in wrong place
Last modified: 2018-02-15 14:57:46 UTC
ABS_GLIB_RUNTIME_LIBDIR gets set to "" so things break
Created attachment 297246 [details] [review] Correctly set ABS_GLIB_RUNTIME_LIBDIR This is currently expanding readlink -f $libdir/$with_runtime_libdir Which is meant to canonicalize the absolute path. But $libdir is ${exec_prefix}/lib which doesn't exist, so the readlink call fails and the result is "". To compound things further, exec_prefix is NONE by default, so we have to do some magic to expand this right.
Why do we need to do this expansion at all?
Because these file needs absolute paths, and the --with-runtime-libdir makes these different from the regular prefix.
See also Bug #640834, on the non-portability of 'readlink -f'. And possibly also Bug #606872 that is some seemingly related confusion/misdefinition of that variable.
I’m not sure this should be done in configure.ac, because then you can’t do things like `make install prefix=/some/custom/path` — this is the reason that autoconf carefully does not expand ${libdir} anywhere. I think this needs to be done using sed in the relevant Makefile.ams, which have access to the make-install-time value of ${prefix}.
(In reply to Philip Withnall from comment #5) > I’m not sure this should be done in configure.ac, because then you can’t do > things like `make install prefix=/some/custom/path` — this is the reason > that autoconf carefully does not expand ${libdir} anywhere. It is? At any rate, that's not going to work in most packages because paths get compiled into the binaries. Eg, GLIB_LOCALE_DIR.
Created attachment 319567 [details] [review] build: Calculate ABS_GLIB_RUNTIME_LIBDIR at build time Rather than calculating it at configure time. This means it can expand $libdir properly, and use the Make $(realpath) function rather than invoking the non-portable `readlink -f`. This fixes problems where `readlink` would be called on an invalid path (due to a variable not being expanded) and would evaluate to "", which would then cause things to be installed in the wrong place.
This patch is my alternative to Alex’s patch. Note that it depends on the patches in bug #759813 but I can easily rebase it after review so it can be committed first if needed (the difference is the changes to gio/Makefile.am).
Review of attachment 319567 [details] [review]: I think this RUNTIME_LIBDIR thing should be considered legacy now - install into /usr consistently and use an initramfs. (Yes, I am aware of the Debian holdout here, but they're just slow and conservative) ::: gio/Makefile.am @@ +788,3 @@ + -e 's|[@]LT_CURRENT[@]|$(LT_CURRENT)|g' \ + -e 's|[@]LT_REVISION[@]|$(LT_REVISION)|g' \ + $< > $@ Wish we were using nonrecursive make so this didn't have to the copy&pasted. Probably not worth creating an include for it yet though. @@ +864,3 @@ cp $$d/$$f $(distdir) || exit 1; done +ABS_GLIB_RUNTIME_LIBDIR = $(realpath $(libdir)/$(GLIB_RUNTIME_LIBDIR)) This does bother me because we're introspecting the target root. I suspect that in e.g. a cross compilation scenario, it might not exist. Maybe something like: if HAVE_GLIB_RUNTIME_LIBDIR ABS_GLIB_RUNTIME_LIBDIR = $(realpath $(libdir)/$(GLIB_RUNTIME_LIBDIR)) else ABS_GLIB_RUNTIME_LIBDIR = $(libdir) fi to contain the breakage to only if one uses runtime libdir?
(In reply to Colin Walters from comment #9) > Review of attachment 319567 [details] [review] [review]: > > I think this RUNTIME_LIBDIR thing should be considered legacy now - install > into /usr consistently and use an initramfs. (Yes, I am aware of the Debian > holdout here, but they're just slow and conservative) If you want to get rid of RUNTIME_LIBDIR, I have no problems with that. I took a look at the original bug which added it (bug #586675) and I couldn’t obviously see it was no longer necessary (or unnecessary). > ::: gio/Makefile.am > @@ +788,3 @@ > + -e 's|[@]LT_CURRENT[@]|$(LT_CURRENT)|g' \ > + -e 's|[@]LT_REVISION[@]|$(LT_REVISION)|g' \ > + $< > $@ > > Wish we were using nonrecursive make so this didn't have to the copy&pasted. > > Probably not worth creating an include for it yet though. Yeah, it’s not great. But you’re not tricking me into converting GLib to non-recursive automake. > @@ +864,3 @@ > cp $$d/$$f $(distdir) || exit 1; done > > +ABS_GLIB_RUNTIME_LIBDIR = $(realpath $(libdir)/$(GLIB_RUNTIME_LIBDIR)) > > This does bother me because we're introspecting the target root. I suspect > that in e.g. a cross compilation scenario, it might not exist. > > Maybe something like: > > if HAVE_GLIB_RUNTIME_LIBDIR > ABS_GLIB_RUNTIME_LIBDIR = $(realpath $(libdir)/$(GLIB_RUNTIME_LIBDIR)) > else > ABS_GLIB_RUNTIME_LIBDIR = $(libdir) > fi > > to contain the breakage to only if one uses runtime libdir? Sounds reasonable. New patch coming.
Created attachment 319569 [details] [review] build: Calculate ABS_GLIB_RUNTIME_LIBDIR at build time Rather than calculating it at configure time. This means it can expand $libdir properly, and use the Make $(realpath) function rather than invoking the non-portable `readlink -f`. This fixes problems where `readlink` would be called on an invalid path (due to a variable not being expanded) and would evaluate to "", which would then cause things to be installed in the wrong place.
Review of attachment 319569 [details] [review]: OK.
Attachment 319569 [details] pushed as 8c26300 - build: Calculate ABS_GLIB_RUNTIME_LIBDIR at build time
*** Bug 751974 has been marked as a duplicate of this bug. ***
*** Bug 606872 has been marked as a duplicate of this bug. ***