| Message ID | 1404406659-31109-7-git-send-email-eric.le.bihan.dev@free.fr |
|---|---|
| State | Superseded |
| Headers | show |
On 03/07/14 18:57, Eric Le Bihan wrote: > If support for systemd-timesyncd is selected, install the associated > service. > > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > --- > package/systemd/systemd.mk | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > index 2da65e6..3454e43 100644 > --- a/package/systemd/systemd.mk > +++ b/package/systemd/systemd.mk > @@ -112,6 +112,11 @@ endif > > ifeq ($(BR2_PACKAGE_SYSTEMD_TIMESYNCD),y) > SYSTEMD_CONF_OPT += --enable-timesyncd > +define SYSTEMD_INSTALL_SERVICE_TIMESYNC > + mkdir -p $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants > + ln -sf ../../../../lib/systemd/system/systemd-timesyncd.service \ > + $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants/systemd-timesyncd.service > +endef Not for this patch, but perhaps a cleaner way to do this would be to add SYSTEMD_INSTALL_INIT_SYSTEMD += $(SYSTEMD_INSTALL_SERVICE_TIMESYNC) instead having the list explicit below? Or wouldn't that work because there is no separator between networkd and timesync? Regards, Arnout > else > SYSTEMD_CONF_OPT += --disable-timesyncd > endif > @@ -180,6 +185,7 @@ endef > > define SYSTEMD_INSTALL_INIT_SYSTEMD > $(SYSTEMD_INSTALL_SERVICE_NETWORK) > + $(SYSTEMD_INSTALL_SERVICE_TIMESYNC) > endef > > $(eval $(autotools-package)) >
On Mon, Jul 07, 2014 at 06:46:12PM +0200, Arnout Vandecappelle wrote: > On 03/07/14 18:57, Eric Le Bihan wrote: > > If support for systemd-timesyncd is selected, install the associated > > service. > > > > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> > > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > > > --- > > package/systemd/systemd.mk | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > > index 2da65e6..3454e43 100644 > > --- a/package/systemd/systemd.mk > > +++ b/package/systemd/systemd.mk > > @@ -112,6 +112,11 @@ endif > > > > ifeq ($(BR2_PACKAGE_SYSTEMD_TIMESYNCD),y) > > SYSTEMD_CONF_OPT += --enable-timesyncd > > +define SYSTEMD_INSTALL_SERVICE_TIMESYNC > > + mkdir -p $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants > > + ln -sf ../../../../lib/systemd/system/systemd-timesyncd.service \ > > + $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants/systemd-timesyncd.service > > +endef > > Not for this patch, but perhaps a cleaner way to do this would be to add > > SYSTEMD_INSTALL_INIT_SYSTEMD += $(SYSTEMD_INSTALL_SERVICE_TIMESYNC) > > instead having the list explicit below? Or wouldn't that work because there is > no separator between networkd and timesync? Exactly! I tried the proposed solution and it does not work because of the missing separator. In order to have such a way of installing systemd unit files, we should do the same as for the pre/post installation hooks. The following change should be done in package/pkg-generic.mk: $(if $(BR2_INIT_SYSTEMD),\ - $($(PKG)_INSTALL_INIT_SYSTEMD)) + $(foreach hook,$($(PKG)_INIT_SYSTEMD_HOOKS),$(call $(hook))$(sep))) Then, we could use: SYSTEMD_INIT_SYSTEMD_HOOKS += SYSTEMD_INSTALL_SERVICE_TIMESYNC This would require updating all the Makefiles of the packages installing systemd unit files (and maybe do the same for SysV/Busybox to be coherent). Best regards, ELB
On 17/07/14 14:46, Eric Le Bihan wrote: > On Mon, Jul 07, 2014 at 06:46:12PM +0200, Arnout Vandecappelle wrote: >> On 03/07/14 18:57, Eric Le Bihan wrote: >>> If support for systemd-timesyncd is selected, install the associated >>> service. >>> >>> Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> >> >> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> >> >>> --- >>> package/systemd/systemd.mk | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk >>> index 2da65e6..3454e43 100644 >>> --- a/package/systemd/systemd.mk >>> +++ b/package/systemd/systemd.mk >>> @@ -112,6 +112,11 @@ endif >>> >>> ifeq ($(BR2_PACKAGE_SYSTEMD_TIMESYNCD),y) >>> SYSTEMD_CONF_OPT += --enable-timesyncd >>> +define SYSTEMD_INSTALL_SERVICE_TIMESYNC >>> + mkdir -p $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants >>> + ln -sf ../../../../lib/systemd/system/systemd-timesyncd.service \ >>> + $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants/systemd-timesyncd.service >>> +endef >> >> Not for this patch, but perhaps a cleaner way to do this would be to add >> >> SYSTEMD_INSTALL_INIT_SYSTEMD += $(SYSTEMD_INSTALL_SERVICE_TIMESYNC) >> >> instead having the list explicit below? Or wouldn't that work because there is >> no separator between networkd and timesync? > > Exactly! I tried the proposed solution and it does not work because of the > missing separator. > > In order to have such a way of installing systemd unit files, we should do the > same as for the pre/post installation hooks. > > The following change should be done in package/pkg-generic.mk: > > $(if $(BR2_INIT_SYSTEMD),\ > - $($(PKG)_INSTALL_INIT_SYSTEMD)) > + $(foreach hook,$($(PKG)_INIT_SYSTEMD_HOOKS),$(call $(hook))$(sep))) > > Then, we could use: > > SYSTEMD_INIT_SYSTEMD_HOOKS += SYSTEMD_INSTALL_SERVICE_TIMESYNC > > This would require updating all the Makefiles of the packages installing > systemd unit files (and maybe do the same for SysV/Busybox to be coherent). No, it's not worth it. There won't be more than one or two packages that need to do more than one thing for the INSTALL_INIT things, so complicating all packages in order for this to work is a bit too much. So your original patch (which you reposted) is fine. Regards, Arnout
diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk index 2da65e6..3454e43 100644 --- a/package/systemd/systemd.mk +++ b/package/systemd/systemd.mk @@ -112,6 +112,11 @@ endif ifeq ($(BR2_PACKAGE_SYSTEMD_TIMESYNCD),y) SYSTEMD_CONF_OPT += --enable-timesyncd +define SYSTEMD_INSTALL_SERVICE_TIMESYNC + mkdir -p $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants + ln -sf ../../../../lib/systemd/system/systemd-timesyncd.service \ + $(TARGET_DIR)/etc/systemd/system/sysinit.target.wants/systemd-timesyncd.service +endef else SYSTEMD_CONF_OPT += --disable-timesyncd endif @@ -180,6 +185,7 @@ endef define SYSTEMD_INSTALL_INIT_SYSTEMD $(SYSTEMD_INSTALL_SERVICE_NETWORK) + $(SYSTEMD_INSTALL_SERVICE_TIMESYNC) endef $(eval $(autotools-package))
If support for systemd-timesyncd is selected, install the associated service. Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> --- package/systemd/systemd.mk | 6 ++++++ 1 file changed, 6 insertions(+)