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 770759 - Can no longer be built on non-systemd systems
Can no longer be built on non-systemd systems
Status: RESOLVED FIXED
Product: vino
Classification: Applications
Component: Server
git master
Other Linux
: Normal normal
: ---
Assigned To: Vino Maintainer(s)
Vino Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-09-02 14:46 UTC by Michael Biebl
Modified: 2016-09-06 08:59 UTC
See Also:
GNOME target: 3.22
GNOME version: ---


Attachments
build: Don't install systemd user unit files if target directory is not set (2.16 KB, patch)
2016-09-02 15:15 UTC, Michael Biebl
needs-work Details | Review

Description Michael Biebl 2016-09-02 14:46:37 UTC
The addition of systemd user service files is a welcome change.
But adding

PKG_CHECK_EXISTS([systemd],, [AC_MSG_ERROR([systemd development headers not found])])

makes the package non-buildable on non-systemd systems.

There are two ways to solve this:

We add a fallback in case the systemd.pc file is not found (like e.g. dbus [1] does), or we add a
AM_CONDITIONAL(HAVE_SYSTEMD,...) and install the user unit files only if systemduserunitdir is not empty.

Let me know which approach you prefer and I'll prep a patch accordingly.


[1]  https://cgit.freedesktop.org/dbus/dbus/tree/configure.ac#n1554
Comment 1 Michael Biebl 2016-09-02 15:15:37 UTC
Created attachment 334656 [details] [review]
build: Don't install systemd user unit files if target  directory is not set

I've decided to conditionally install the systemd user units as this seems the proper fix.
Comment 2 Bastien Nocera 2016-09-05 11:13:36 UTC
Review of attachment 334656 [details] [review]:

::: Makefile.am
@@ +238,3 @@
 	$(AM_V_GEN)$(SED) -e "s|[@]libexecdir[@]|$(libexecdir)|" $< > $@
 
+if HAVE_SYSTEMD

This won't add the files into the dist if systemd isn't available on the systemd that create the tarball.

Either you need to turn the systemd check in configure.ac into an error following a flag (which would be added to AM_DISTCHECK_CONFIGURE_FLAGS), or add those files manually to the dist in the else of this conditional.

The latter requires less code.
Comment 3 Michael Biebl 2016-09-05 11:23:51 UTC
(In reply to Bastien Nocera from comment #2)
> Review of attachment 334656 [details] [review] [review]:
> 
> ::: Makefile.am
> @@ +238,3 @@
>  	$(AM_V_GEN)$(SED) -e "s|[@]libexecdir[@]|$(libexecdir)|" $< > $@
>  
> +if HAVE_SYSTEMD
> 
> This won't add the files into the dist if systemd isn't available on the
> systemd that create the tarball.
> 
> Either you need to turn the systemd check in configure.ac into an error
> following a flag (which would be added to AM_DISTCHECK_CONFIGURE_FLAGS), or
> add those files manually to the dist in the else of this conditional.
> 
> The latter requires less code.

I explicitly tested that and server/vino-server.service.in was correctly added to the dist tarball in the absence of systemd.pc.
TTBOMK, AM_CONDITIONAL does unconditional build but not dist nowadays. Old automake versions did not handle this properly.

Is this not working for you? Which automake version do you use?
Comment 4 Michael Biebl 2016-09-05 11:26:06 UTC
(In reply to Michael Biebl from comment #3)
> TTBOMK, AM_CONDITIONAL does unconditional build but not dist nowadays. Old

That's the wrong way around: AM_CONDITIONAL should unconditionally dist files, but conditionally build/process files.
Comment 5 Michael Biebl 2016-09-05 11:48:47 UTC
Looking at the generated Makefile.in, I have

am__dist_noinst_DATA_DIST = server/vino-server.service.in \
...

This is with automake 1.15
Comment 6 David King 2016-09-06 08:58:59 UTC
Fixed on master with commit 1538798a89653b8921ca574aebb3f153543b4921.