| Message ID | 1559170961-14091-1-git-send-email-hancock@sedsystems.ca |
|---|---|
| State | Accepted |
| Headers | show |
| Series | package/gpsd: Fix systemd service installation and paths | expand |
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
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 >
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@.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 > >
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@.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
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(-)