diff mbox series

[2/2] package/gpsd: actually wait for after chrony

Message ID 8785_1663677905_6329B5D1_8785_15_1_b7ced13c9a2651fbf36d4009a03c0590457c82a3.1663677504.git.yann.morin@orange.com
State Accepted
Headers show
Series [1/2] package/gpsd: drop legacy cleanup | expand

Commit Message

Yann E. MORIN Sept. 20, 2022, 12:45 p.m. UTC
From: "Yann E. MORIN" <yann.morin@orange.com>

We use gpsd's upstream systemd service unit files, which define a
dependency on chronyd.service. And indeed, upstream chrony does
provide an example service unit file chronyd.service.

However, in Buildroot, we are not using chrony's upstream unit, we are
providing our own, much simplified as compared to upstream. We install
that unit file as chrony.service. Notice that subtle difference in the
name: upstream's is chronyd, with a trailing 'd', while ours just
chrony, without the trailing 'd'.

As a consequence, in a Buildroot-built system, gpsd does not wait for
after chrony is started, which causes all kind of mayhem when gpsd
actually needs to talk to chrony.

We have multiple options:
 1. use chrony's upstream unit file;
 2  rename the chrony service file as installed by Buildroot, to match
    what chrony would actually do;
 3. tweak gpsd's unit file to refer to chrony.service, not
    chronyd.service;
 4. leverage systemd's flexibility in how units are defined, and provide
    a drop-in to complement gpsd's unit to also wait for chrony.service.

For 1. it is totally unknown why we do have our unit file to begin with,
rather than use upstream's. Since upstream's is much more complex than
ours, using it might have unforetold consequences.

Going with 2. seems the easiest at first sight, but then it would break
systems where users provide their own drop-ins for chrony, as they would
no longer match.

3. is relatively easy, but running sed is not entirely nice. Besides, it
semantically should be a post-install hook, rather than a systemd-init
command, but again that makes things a bit more ugly. Also, some people
may have their own gpsd.service in an overlay or whatever, which would
break our fixup.

Solution 4. is pretty straightforward, although it is not ideal either.

To be noted: some distributions, like Ubuntu 20.04 at least, do install
the chrony unit file as chrony.service, like Buildroot does. However,
there does not appear to be any fixup in gpsd for this discrepancy, as
their gpsd install still refers to chronyd.service. So that does not
help us decide what to do.

So, eventually, we decided to go with solution 4, which has the least
impact on the system, and keeps the status-quo for all other use-cases.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
Cc: Alex Suykov <alex.suykov@gmail.com>
---
 package/gpsd/br-chrony.conf | 2 ++
 package/gpsd/gpsd.mk        | 8 ++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 package/gpsd/br-chrony.conf

Comments

Peter Korsgaard Nov. 3, 2022, 1:46 p.m. UTC | #1
>>>>>   <yann.morin@orange.com> writes:

 > From: "Yann E. MORIN" <yann.morin@orange.com>
 > We use gpsd's upstream systemd service unit files, which define a
 > dependency on chronyd.service. And indeed, upstream chrony does
 > provide an example service unit file chronyd.service.

 > However, in Buildroot, we are not using chrony's upstream unit, we are
 > providing our own, much simplified as compared to upstream. We install
 > that unit file as chrony.service. Notice that subtle difference in the
 > name: upstream's is chronyd, with a trailing 'd', while ours just
 > chrony, without the trailing 'd'.

 > As a consequence, in a Buildroot-built system, gpsd does not wait for
 > after chrony is started, which causes all kind of mayhem when gpsd
 > actually needs to talk to chrony.

 > We have multiple options:
 >  1. use chrony's upstream unit file;
 >  2  rename the chrony service file as installed by Buildroot, to match
 >     what chrony would actually do;
 >  3. tweak gpsd's unit file to refer to chrony.service, not
 >     chronyd.service;
 >  4. leverage systemd's flexibility in how units are defined, and provide
 >     a drop-in to complement gpsd's unit to also wait for chrony.service.

 > For 1. it is totally unknown why we do have our unit file to begin with,
 > rather than use upstream's. Since upstream's is much more complex than
 > ours, using it might have unforetold consequences.

 > Going with 2. seems the easiest at first sight, but then it would break
 > systems where users provide their own drop-ins for chrony, as they would
 > no longer match.

 > 3. is relatively easy, but running sed is not entirely nice. Besides, it
 > semantically should be a post-install hook, rather than a systemd-init
 > command, but again that makes things a bit more ugly. Also, some people
 > may have their own gpsd.service in an overlay or whatever, which would
 > break our fixup.

 > Solution 4. is pretty straightforward, although it is not ideal either.

 > To be noted: some distributions, like Ubuntu 20.04 at least, do install
 > the chrony unit file as chrony.service, like Buildroot does. However,
 > there does not appear to be any fixup in gpsd for this discrepancy, as
 > their gpsd install still refers to chronyd.service. So that does not
 > help us decide what to do.

 > So, eventually, we decided to go with solution 4, which has the least
 > impact on the system, and keeps the status-quo for all other use-cases.

 > Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
 > Cc: Bernd Kuhls <bernd.kuhls@t-online.de>
 > Cc: Alex Suykov <alex.suykov@gmail.com>

Committed to 2022.08.x and 2022.02.x, thanks.
diff mbox series

Patch

diff --git a/package/gpsd/br-chrony.conf b/package/gpsd/br-chrony.conf
new file mode 100644
index 0000000000..67a79e5f65
--- /dev/null
+++ b/package/gpsd/br-chrony.conf
@@ -0,0 +1,2 @@ 
+[Unit]
+After=chrony.service
diff --git a/package/gpsd/gpsd.mk b/package/gpsd/gpsd.mk
index b8526c0260..961ebde649 100644
--- a/package/gpsd/gpsd.mk
+++ b/package/gpsd/gpsd.mk
@@ -213,6 +213,14 @@  define GPSD_INSTALL_INIT_SYSV
 	$(SED) 's,^DEVICES=.*,DEVICES=$(BR2_PACKAGE_GPSD_DEVICES),' $(TARGET_DIR)/etc/init.d/S50gpsd
 endef
 
+# When using chrony, wait for after Buildroot's chrony.service
+ifeq ($(BR2_PACKAGE_CHRONY),y)
+define GPSD_INSTALL_INIT_SYSTEMD
+	$(INSTALL) -D -m 0644 $(GPSD_PKGDIR)/br-chrony.conf \
+		$(TARGET_DIR)/usr/lib/systemd/system/gpsd.service.d/br-chrony.conf
+endef
+endif
+
 define GPSD_INSTALL_STAGING_CMDS
 	(cd $(@D); \
 		$(GPSD_SCONS_ENV) \