[Buildroot] [PATCH] package/gpsd: Fix systemd service installation and paths

[Buildroot] [PATCH] package/gpsd: Fix systemd service installation and paths

[Robert Hancock]

Fix several issues with systemd service file installation for gpsd:

-systemd support in the gpsd build was defaulting to enabled or not
based on whether the host system had systemd directories present. Set
this explicitly based on whether BR2_INIT_SYSTEMD is set.

-The installed systemd service files referenced paths in /usr/local when
the actual binaries are installed in /usr. Replace /usr/local with /usr
in the installed service files.

-When BR2_PACKAGE_HAS_UDEV was enabled, all of the binaries were
re-installed again, along with the udev rules, as part of the
post-install hooks. Just choose between using install and udev-install
based on whether udev is enabled to avoid redundant re-installations.

Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
---
 package/gpsd/gpsd.mk | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/package/gpsd/gpsd.mk b/package/gpsd/gpsd.mk
index d2c7612..a5c7477 100644
--- a/package/gpsd/gpsd.mk
+++ b/package/gpsd/gpsd.mk
@@ -24,7 +24,8 @@ GPSD_SCONS_OPTS = \
 	sysroot=$(STAGING_DIR)\
 	strip=no\
 	python=no \
-	qt=no
+	qt=no \
+	systemd=$(if $(BR2_INIT_SYSTEMD),yes,no)
 
 ifeq ($(BR2_PACKAGE_NCURSES),y)
 GPSD_DEPENDENCIES += ncurses
@@ -216,7 +217,7 @@ define GPSD_INSTALL_TARGET_CMDS
 		DESTDIR=$(TARGET_DIR) \
 		$(HOST_DIR)/bin/python2 $(SCONS) \
 		$(GPSD_SCONS_OPTS) \
-		install)
+		$(if $(BR2_PACKAGE_HAS_UDEV),udev-install,install))
 endef
 
 define GPSD_INSTALL_INIT_SYSV
@@ -224,6 +225,14 @@ define GPSD_INSTALL_INIT_SYSV
 	$(SED) 's,^DEVICES=.*,DEVICES=$(BR2_PACKAGE_GPSD_DEVICES),' $(TARGET_DIR)/etc/init.d/S50gpsd
 endef
 
+define GPSD_INSTALL_INIT_SYSTEMD
+# systemd unit files are installed automatically, but need to update the
+# /usr/local path references in the provided files to /usr.
+	$(SED) 's%/usr/local%/usr%' \
+		$(TARGET_DIR)/usr/lib/systemd/system/gpsd.service \
+		$(TARGET_DIR)/usr/lib/systemd/system/gpsdctl at .service
+endef
+
 define GPSD_INSTALL_STAGING_CMDS
 	(cd $(@D); \
 		$(GPSD_SCONS_ENV) \
@@ -233,16 +242,10 @@ define GPSD_INSTALL_STAGING_CMDS
 		install)
 endef
 
-# After installing the udev rule, make it writable so that this
+# After the udev rule is installed, make it writable so that this
 # package can be re-built/re-installed.
 ifeq ($(BR2_PACKAGE_HAS_UDEV),y)
 define GPSD_INSTALL_UDEV_RULES
-	(cd $(@D); \
-		$(GPSD_SCONS_ENV) \
-		DESTDIR=$(TARGET_DIR) \
-		$(HOST_DIR)/bin/python2 $(SCONS) \
-		$(GPSD_SCONS_OPTS) \
-		udev-install)
 	chmod u+w $(TARGET_DIR)/lib/udev/rules.d/25-gpsd.rules
 endef
 
-- 
1.8.3.1

[Buildroot] [PATCH] package/gpsd: Fix systemd service installation and paths

[Thomas Petazzoni]

Hello Robert,

Thanks for this patch!

On Wed, 29 May 2019 17:02:41 -0600
Robert Hancock <hancock@sedsystems.ca> wrote:

> Fix several issues with systemd service file installation for gpsd:
> 
> -systemd support in the gpsd build was defaulting to enabled or not
> based on whether the host system had systemd directories present. Set
> this explicitly based on whether BR2_INIT_SYSTEMD is set.
> 
> -The installed systemd service files referenced paths in /usr/local when
> the actual binaries are installed in /usr. Replace /usr/local with /usr
> in the installed service files.

I have not looked into the gpsd build system, but I was wondering if
there was a way to convince gpsd to produce those files with the right
prefix (i.e /usr instead or /usr/local).

> -When BR2_PACKAGE_HAS_UDEV was enabled, all of the binaries were
> re-installed again, along with the udev rules, as part of the
> post-install hooks. Just choose between using install and udev-install
> based on whether udev is enabled to avoid redundant re-installations.

This should be part of a separate patch, because it's not a fix, just
an unrelated optimization.

But in fact, the systemd fixes could also be two separate patches.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/gpsd: Fix systemd service installation and paths

[Robert Hancock]

On 2019-05-31 3:11 p.m., Thomas Petazzoni wrote:
> Hello Robert,
> 
> Thanks for this patch!
> 
> On Wed, 29 May 2019 17:02:41 -0600
> Robert Hancock <hancock@sedsystems.ca> wrote:
> 
>> Fix several issues with systemd service file installation for gpsd:
>>
>> -systemd support in the gpsd build was defaulting to enabled or not
>> based on whether the host system had systemd directories present. Set
>> this explicitly based on whether BR2_INIT_SYSTEMD is set.
>>
>> -The installed systemd service files referenced paths in /usr/local when
>> the actual binaries are installed in /usr. Replace /usr/local with /usr
>> in the installed service files.
> 
> I have not looked into the gpsd build system, but I was wondering if
> there was a way to convince gpsd to produce those files with the right
> prefix (i.e /usr instead or /usr/local).

As far as I can see, no - the service files are just part of the gpsd
source tree and the build system installs them as-is, there's no
mechanism provided to set the prefix etc.

> 
>> -When BR2_PACKAGE_HAS_UDEV was enabled, all of the binaries were
>> re-installed again, along with the udev rules, as part of the
>> post-install hooks. Just choose between using install and udev-install
>> based on whether udev is enabled to avoid redundant re-installations.
> 
> This should be part of a separate patch, because it's not a fix, just
> an unrelated optimization.

I suppose it wasn't obvious from the description, but this issue
actually caused problems with fixing the systemd service file paths,
since we would fix the paths after the files were installed, but then
udev-install was being run afterwards and reinstalled everything, which
wiped out our fix.

> 
> But in fact, the systemd fixes could also be two separate patches.
> 
> Thanks,
> 
> Thomas
> 

-- 
Robert Hancock
Senior Software Developer
SED Systems, a division of Calian Ltd.
Email: hancock at sedsystems.ca

[Buildroot] [PATCH] package/gpsd: Fix systemd service installation and paths

[Arnout Vandecappelle]

On 30/05/2019 01:02, Robert Hancock wrote:
> Fix several issues with systemd service file installation for gpsd:
> 
> -systemd support in the gpsd build was defaulting to enabled or not
> based on whether the host system had systemd directories present. Set
> this explicitly based on whether BR2_INIT_SYSTEMD is set.
> 
> -The installed systemd service files referenced paths in /usr/local when
> the actual binaries are installed in /usr. Replace /usr/local with /usr
> in the installed service files.
> 
> -When BR2_PACKAGE_HAS_UDEV was enabled, all of the binaries were
> re-installed again, along with the udev rules, as part of the
> post-install hooks. Just choose between using install and udev-install
> based on whether udev is enabled to avoid redundant re-installations.
> 
> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>

 Applied to master, thanks, with a small changes.


>  
> +define GPSD_INSTALL_INIT_SYSTEMD
> +# systemd unit files are installed automatically, but need to update the
> +# /usr/local path references in the provided files to /usr.

 Comments inside a define get passed to the shell, which is not so nice. So I
moved these out before the define.

 Regards,
 Arnout

> +	$(SED) 's%/usr/local%/usr%' \
> +		$(TARGET_DIR)/usr/lib/systemd/system/gpsd.service \
> +		$(TARGET_DIR)/usr/lib/systemd/system/gpsdctl at .service
> +endef
> +
>  define GPSD_INSTALL_STAGING_CMDS
>  	(cd $(@D); \
>  		$(GPSD_SCONS_ENV) \
> @@ -233,16 +242,10 @@ define GPSD_INSTALL_STAGING_CMDS
>  		install)
>  endef
>  
> -# After installing the udev rule, make it writable so that this
> +# After the udev rule is installed, make it writable so that this
>  # package can be re-built/re-installed.
>  ifeq ($(BR2_PACKAGE_HAS_UDEV),y)
>  define GPSD_INSTALL_UDEV_RULES
> -	(cd $(@D); \
> -		$(GPSD_SCONS_ENV) \
> -		DESTDIR=$(TARGET_DIR) \
> -		$(HOST_DIR)/bin/python2 $(SCONS) \
> -		$(GPSD_SCONS_OPTS) \
> -		udev-install)
>  	chmod u+w $(TARGET_DIR)/lib/udev/rules.d/25-gpsd.rules
>  endef
>  
>