After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744772 - systemtap and gdb scripts install in wrong place
systemtap and gdb scripts install in wrong place
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: build
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 606872 751974 (view as bug list)
Depends on:
Blocks: 606872
 
 
Reported: 2015-02-19 08:56 UTC by Alexander Larsson
Modified: 2018-02-15 14:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Correctly set ABS_GLIB_RUNTIME_LIBDIR (1.87 KB, patch)
2015-02-19 08:56 UTC, Alexander Larsson
none Details | Review
build: Calculate ABS_GLIB_RUNTIME_LIBDIR at build time (4.89 KB, patch)
2016-01-22 19:27 UTC, Philip Withnall
none Details | Review
build: Calculate ABS_GLIB_RUNTIME_LIBDIR at build time (5.17 KB, patch)
2016-01-22 20:23 UTC, Philip Withnall
committed Details | Review

Description Alexander Larsson 2015-02-19 08:56:20 UTC
ABS_GLIB_RUNTIME_LIBDIR gets set to "" so things break
Comment 1 Alexander Larsson 2015-02-19 08:56:48 UTC
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.
Comment 2 Allison Karlitskaya (desrt) 2015-03-02 14:33:42 UTC
Why do we need to do this expansion at all?
Comment 3 Alexander Larsson 2015-03-12 14:24:07 UTC
Because these file needs absolute paths, and the --with-runtime-libdir makes these different from the regular prefix.
Comment 4 Daniel Macks 2015-04-23 05:17:11 UTC
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.
Comment 5 Philip Withnall 2015-12-09 19:20:37 UTC
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}.
Comment 6 Dan Winship 2015-12-09 20:04:56 UTC
(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.
Comment 7 Philip Withnall 2016-01-22 19:27:57 UTC
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.
Comment 8 Philip Withnall 2016-01-22 19:30:11 UTC
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).
Comment 9 Colin Walters 2016-01-22 19:41:15 UTC
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?
Comment 10 Philip Withnall 2016-01-22 20:12:52 UTC
(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.
Comment 11 Philip Withnall 2016-01-22 20:23:30 UTC
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.
Comment 12 Colin Walters 2016-01-23 01:58:54 UTC
Review of attachment 319569 [details] [review]:

OK.
Comment 13 Philip Withnall 2016-01-24 14:46:53 UTC
Attachment 319569 [details] pushed as 8c26300 - build: Calculate ABS_GLIB_RUNTIME_LIBDIR at build time
Comment 14 Philip Withnall 2017-11-17 14:28:56 UTC
*** Bug 751974 has been marked as a duplicate of this bug. ***
Comment 15 Philip Withnall 2018-02-15 14:57:46 UTC
*** Bug 606872 has been marked as a duplicate of this bug. ***