Bug 37870 - Force symbolic link creation during installation of systemd files
Summary: Force symbolic link creation during installation of systemd files
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.4.x
Hardware: All All
: low trivial
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://cgit.freedesktop.org/~smcv/dbu...
Whiteboard:
Keywords: patch
Depends on:
Blocks: dbus-1.4
  Show dependency treegraph
 
Reported: 2011-06-02 18:02 UTC by guido
Modified: 2011-06-08 06:08 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
bus: use ln -fs to enable dbus in systemd, not $(LN_S) (2.20 KB, patch)
2011-06-07 06:04 UTC, Simon McVittie
Details | Splinter Review
Initial patch - do not apply as is without further checking validity (12.87 KB, patch)
2011-06-07 11:33 UTC, guido
Details | Splinter Review

Description guido 2011-06-02 18:02:02 UTC
The installation fails on a Linux system where systemd files are already installed.

This is due to the variable LN_S='ln -s' in Makefiles under directory bus and perhaps directory dbus.

Please use LN_SF='ln -sf' so that symbolic link creation does not break the installation build target.
Comment 1 Simon McVittie 2011-06-07 06:03:48 UTC
Most people install dbus via a ${DESTDIR} (as used in the preparation of RPM/dpkg packages) so this failure mode hadn't been seen before. Here's a patch.
Comment 2 Simon McVittie 2011-06-07 06:04:49 UTC
Created attachment 47659 [details] [review]
bus: use ln -fs to enable dbus in systemd, not $(LN_S)

Using $(LN_S) is inappropriate because it could in theory mean either
ln -s, ln or cp -p depending on autoconf checks.

Not using -f breaks reinstallation directly from source (DESTDIR unset),
because the symlinks will already exist.

Because systemd isn't currently portable to non-Linux, let alone
non-SUS-compliant systems, it seems safe to assume that ln -fs behaves
as specified by SUS if systemd was found.
Comment 3 Colin Walters 2011-06-07 10:47:37 UTC
Review of attachment 47659 [details] [review]:

Looks fine.
Comment 4 guido 2011-06-07 11:04:34 UTC
Thanks very much guys.

We shall not forget that apparently "Solaris /bin/ln does not understand -f" (see for example ltmain.sh:3009).

I will now check and try to review the patch and report back in a few minutes.
Comment 5 guido 2011-06-07 11:23:20 UTC
Patch not effective apparently...

It seems to me that:
- all Makefile.in have not been tackled;
- top-level configure has not been tackled;
- libtool.m4 ?
- please do not forget Solaris' special case.
Comment 6 guido 2011-06-07 11:33:24 UTC
Created attachment 47675 [details] [review]
Initial patch - do not apply as is without further checking validity

Initial patch - do not apply as is without further checking validity

At least it needs to be modified in order to account for Solaris' special case (ln -sf not understood).
Comment 7 Simon McVittie 2011-06-08 02:24:23 UTC
(In reply to comment #4)
> We shall not forget that apparently "Solaris /bin/ln does not understand -f"

systemd doesn't support Solaris and probably never will, so if we have systemd, we can't be on Solaris.

(In reply to comment #5)
> It seems to me that:
> - all Makefile.in have not been tackled;
> - top-level configure has not been tackled;
> - libtool.m4 ?

The various Makefile.in are generated by automake from Makefile.am, configure is generated by autoconf from configure.ac, and libtool.m4 is copied in by libtoolize. To make my patch effective, you need to apply it then run "autoreconf" in the top level directory. Please see autoconf, automake, libtool documentation for details of what generates what.

In general it is not a bug for LN_S to be defined to "ln -s" - the uses of LN_S in bus/Makefile.am were wrong, because they wanted "ln -sf" semantics, but the uses of LN_S elsewhere should be fine.
Comment 8 Simon McVittie 2011-06-08 02:28:07 UTC
Review of attachment 47675 [details] [review]:

Every file patched here, except Makefile.am, is generated from something else. If developing patches in future, please use what's in the git repository as a basis for patching - the git repository does not contain any generated files, only the "real source", whereas the tarball releases also contain files generated by autoconf and automake.
Comment 9 Simon McVittie 2011-06-08 03:44:08 UTC
(In reply to comment #3)
> Review of attachment 47659 [details] [review]:
> 
> Looks fine.

Applied in git for 1.4.12, 1.5.4.

Guido, if you're still having trouble applying the patch, you could try one of these snapshot tarballs (depending whether you're following releases of dbus-1.4 or master), which have it pre-applied and the autotools stuff re-generated:

http://people.collabora.co.uk/~smcv/expires-201107/dbus-1.4.11-37870.tar.gz

http://people.collabora.co.uk/~smcv/expires-201107/dbus-1.5.3-37870.tar.gz
Comment 10 guido 2011-06-08 06:08:00 UTC
Yes, you're right Simon. I had done autoconf instead of autoreconf....

And clearly I didn't know the case for Solaris as I am not using it and I've never used it, but thanks for pointing that out.

Everything sorted now.

Cheers


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.