| Message ID | 20180810144635.16182-1-chrismcc@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] fake-hwclock: new package | expand |
Hello, Yann, Arnout, Peter, there is a question below. Christopher, there are some review comments as well. On Fri, 10 Aug 2018 07:46:34 -0700, Christopher McCrory wrote: > +if BR2_PACKAGE_FAKE_HWCLOCK > +config BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB > + bool "install fake-hwclock cronjob" > + help > + Hourly cronjob to save current timestamp What should be our policy for cronjobs ? I'm not sure we currently have packages that install cronjobs. What should we do? Install unconditionally? Have a sub-option? I would personally lean towards installing unconditionally, but I'd like to have others opinions before applying. > +FAKE_HWCLOCK_VERSION = v0.11 > +FAKE_HWCLOCK_SITE = https://git.einval.com/git/fake-hwclock.git > +FAKE_HWCLOCK_SITE_METHOD = git > +FAKE_HWCLOCK_LICENSE_FILES = COPYING > +FAKE_LICENSE = GPL-2.0 This variable is wrong: it should be FAKE_HWCLOCK_LICENSE. Also, we prefer to have it before the LICENSE_FILES variable. > +define FAKE_HWCLOCK_NO_LSB > + $(SED) 's@. /lib/lsb/init-functions@#. /lib/lsb/init-functions@' \ > + $(@D)/debian/fake-hwclock.init > +endef This should be done by a patch. Thanks! Thomas
Thomas, Christopher, All, On 2018-08-14 16:00 +0200, Thomas Petazzoni spake thusly: > On Fri, 10 Aug 2018 07:46:34 -0700, Christopher McCrory wrote: > > +if BR2_PACKAGE_FAKE_HWCLOCK > > +config BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB > > + bool "install fake-hwclock cronjob" > > + help > > + Hourly cronjob to save current timestamp > > What should be our policy for cronjobs ? I'm not sure we currently have > packages that install cronjobs. What should we do? Install > unconditionally? Have a sub-option? > > I would personally lean towards installing unconditionally, but I'd > like to have others opinions before applying. Don't add an option; always install it. Unless there is a *very* good reason not to (e.g. security considerations), in which case a rationale must be provided. Regards, Yann E. MORIN.
On Tue, Aug 14, 2018, 7:00 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello, > > Yann, Arnout, Peter, there is a question below. > > Christopher, there are some review comments as well. > > On phone, sorry for formatting. I'll look into this and the others tomorrow. My other job got in the way today. > > On Fri, 10 Aug 2018 07:46:34 -0700, Christopher McCrory wrote: > > > +if BR2_PACKAGE_FAKE_HWCLOCK > > +config BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB > > + bool "install fake-hwclock cronjob" > > + help > > + Hourly cronjob to save current timestamp > > What should be our policy for cronjobs ? I'm not sure we currently have > packages that install cronjobs. What should we do? Install > unconditionally? Have a sub-option? > > I would personally lean towards installing unconditionally, but I'd > like to have others opinions before applying. > > > +FAKE_HWCLOCK_VERSION = v0.11 > > +FAKE_HWCLOCK_SITE = https://git.einval.com/git/fake-hwclock.git > > +FAKE_HWCLOCK_SITE_METHOD = git > > +FAKE_HWCLOCK_LICENSE_FILES = COPYING > > +FAKE_LICENSE = GPL-2.0 > > This variable is wrong: it should be FAKE_HWCLOCK_LICENSE. Also, we > prefer to have it before the LICENSE_FILES variable. > > > +define FAKE_HWCLOCK_NO_LSB > > + $(SED) 's@. /lib/lsb/init-functions@#. /lib/lsb/init-functions@' \ > > + $(@D)/debian/fake-hwclock.init > > +endef > > This should be done by a patch. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com > <div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 14, 2018, 7:00 AM Thomas Petazzoni <<a href="mailto:thomas.petazzoni@bootlin.com">thomas.petazzoni@bootlin.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello,<br> <br> Yann, Arnout, Peter, there is a question below.<br> <br> Christopher, there are some review comments as well.<br><br></blockquote></div></div><div dir="auto">On phone, sorry for formatting. I'll look into this and the others tomorrow. My other job got in the way today.</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br><br> On Fri, 10 Aug 2018 07:46:34 -0700, Christopher McCrory wrote:<br> <br> > +if BR2_PACKAGE_FAKE_HWCLOCK<br> > +config BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB<br> > + bool "install fake-hwclock cronjob"<br> > + help<br> > + Hourly cronjob to save current timestamp<br> <br> What should be our policy for cronjobs ? I'm not sure we currently have<br> packages that install cronjobs. What should we do? Install<br> unconditionally? Have a sub-option?<br> <br> I would personally lean towards installing unconditionally, but I'd<br> like to have others opinions before applying.<br> <br> > +FAKE_HWCLOCK_VERSION = v0.11<br> > +FAKE_HWCLOCK_SITE = <a href="https://git.einval.com/git/fake-hwclock.git" rel="noreferrer noreferrer" target="_blank">https://git.einval.com/git/fake-hwclock.git</a><br> > +FAKE_HWCLOCK_SITE_METHOD = git<br> > +FAKE_HWCLOCK_LICENSE_FILES = COPYING<br> > +FAKE_LICENSE = GPL-2.0<br> <br> This variable is wrong: it should be FAKE_HWCLOCK_LICENSE. Also, we<br> prefer to have it before the LICENSE_FILES variable.<br> <br> > +define FAKE_HWCLOCK_NO_LSB<br> > + $(SED) 's@. /lib/lsb/init-functions@#. /lib/lsb/init-functions@' \<br> > + $(@D)/debian/fake-hwclock.init<br> > +endef<br> <br> This should be done by a patch.<br> <br> Thanks!<br> <br> Thomas<br> -- <br> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)<br> Embedded Linux and Kernel engineering<br> <a href="https://bootlin.com" rel="noreferrer noreferrer" target="_blank">https://bootlin.com</a><br> </blockquote></div></div></div>
On 14-08-18 16:00, Thomas Petazzoni wrote: > Hello, > > Yann, Arnout, Peter, there is a question below. > > Christopher, there are some review comments as well. > > On Fri, 10 Aug 2018 07:46:34 -0700, Christopher McCrory wrote: > >> +if BR2_PACKAGE_FAKE_HWCLOCK >> +config BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB >> + bool "install fake-hwclock cronjob" >> + help >> + Hourly cronjob to save current timestamp > > What should be our policy for cronjobs ? I'm not sure we currently have > packages that install cronjobs. We don't, for a good reason: we don't start a cron daemon. Actually the 'at' package does install crontab entries, but it's kind of pointless without crond... > What should we do? Install > unconditionally? Have a sub-option? I agree with unconditional. Ideally, though, as part of the _INSTALL_INIT, because for systemd you really want to use systemd.timer units instead of crontabs. > > I would personally lean towards installing unconditionally, but I'd > like to have others opinions before applying. > >> +FAKE_HWCLOCK_VERSION = v0.11 >> +FAKE_HWCLOCK_SITE = https://git.einval.com/git/fake-hwclock.git >> +FAKE_HWCLOCK_SITE_METHOD = git >> +FAKE_HWCLOCK_LICENSE_FILES = COPYING >> +FAKE_LICENSE = GPL-2.0 > > This variable is wrong: it should be FAKE_HWCLOCK_LICENSE. Also, we > prefer to have it before the LICENSE_FILES variable. > >> +define FAKE_HWCLOCK_NO_LSB >> + $(SED) 's@. /lib/lsb/init-functions@#. /lib/lsb/init-functions@' \ >> + $(@D)/debian/fake-hwclock.init >> +endef > > This should be done by a patch. Actually I think we'd prefer a Buildroot-specific init script instead of the bloaty Debian one. Regards, Arnout > > Thanks! > > Thomas >
diff --git a/DEVELOPERS b/DEVELOPERS index 215506c71c..9ea934fa57 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -479,6 +479,9 @@ F: package/python-rtslib-fb/ F: package/python-urwid/ F: package/targetcli-fb/ +N: Christopher McCrory <chrismcc@gmail.com> +F: package/fake-hwclock/ + N: Clayton Shotwell <clayton.shotwell@rockwellcollins.com> F: package/audit/ F: package/checkpolicy/ diff --git a/package/Config.in b/package/Config.in index f5a17492c7..42c61c242e 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1982,6 +1982,7 @@ comment "Utilities" source "package/dialog/Config.in" source "package/dtach/Config.in" source "package/easy-rsa/Config.in" + source "package/fake-hwclock/Config.in" source "package/file/Config.in" source "package/gnupg/Config.in" source "package/gnupg2/Config.in" diff --git a/package/fake-hwclock/Config.in b/package/fake-hwclock/Config.in new file mode 100644 index 0000000000..f7e01dfb46 --- /dev/null +++ b/package/fake-hwclock/Config.in @@ -0,0 +1,12 @@ +config BR2_PACKAGE_FAKE_HWCLOCK + bool "fake-hwclock" + help + Save/restore system clock on machines without working RTC hardware + +if BR2_PACKAGE_FAKE_HWCLOCK +config BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB + bool "install fake-hwclock cronjob" + help + Hourly cronjob to save current timestamp + +endif diff --git a/package/fake-hwclock/fake-hwclock.hash b/package/fake-hwclock/fake-hwclock.hash new file mode 100644 index 0000000000..7bf147d7a6 --- /dev/null +++ b/package/fake-hwclock/fake-hwclock.hash @@ -0,0 +1,2 @@ +# Locally calculated +sha256 b658d6d130f66eeaa79bae6943ebae45a85bc6a932260befff75c630a8cfe428 fake-hwclock-v0.11.tar.gz diff --git a/package/fake-hwclock/fake-hwclock.mk b/package/fake-hwclock/fake-hwclock.mk new file mode 100644 index 0000000000..caae8605b9 --- /dev/null +++ b/package/fake-hwclock/fake-hwclock.mk @@ -0,0 +1,40 @@ +################################################################################ +# +# fake-hwclock +# +################################################################################ + +FAKE_HWCLOCK_VERSION = v0.11 +FAKE_HWCLOCK_SITE = https://git.einval.com/git/fake-hwclock.git +FAKE_HWCLOCK_SITE_METHOD = git +FAKE_HWCLOCK_LICENSE_FILES = COPYING +FAKE_LICENSE = GPL-2.0 + +define FAKE_HWCLOCK_NO_LSB + $(SED) 's@. /lib/lsb/init-functions@#. /lib/lsb/init-functions@' \ + $(@D)/debian/fake-hwclock.init +endef + +FAKE_HWCLOCK_POST_PATCH_HOOKS += FAKE_HWCLOCK_NO_LSB + +ifeq ($(BR2_PACKAGE_FAKE_HWCLOCK_CRONJOB),y) +define FAKE_HWCLOCK_INSTALL_CRONJOB + $(INSTALL) -D -m 755 $(@D)/debian/fake-hwclock.cron.hourly $(TARGET_DIR)/etc/cron.hourly/fake-hwclock +endef +endif + +define FAKE_HWCLOCK_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 755 $(@D)/fake-hwclock $(TARGET_DIR)/sbin/fake-hwclock + # Use buildroot buildtime as a seed. + # Not ideal, but better than 1970-01-01 UTC + date -u '+%Y-%m-%d %H:%M:%S' > $(TARGET_DIR)/etc/fake-hwclock.data + $(FAKE_HWCLOCK_INSTALL_CRONJOB) +endef + +# This should run before everything else, so use S00 +define FAKE_HWCLOCK_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 $(@D)/debian/fake-hwclock.init $(TARGET_DIR)/etc/init.d/S00fake-hwclock + $(INSTALL) -D -m 755 $(@D)/etc/default/fake-hwclock $(TARGET_DIR)/etc/default/fake-hwclock +endef + +$(eval $(generic-package))
Save/restore system clock on machines without working RTC hardware Signed-off-by: Christopher McCrory <chrismcc@gmail.com> --- DEVELOPERS | 3 +++ package/Config.in | 1 + package/fake-hwclock/Config.in | 12 ++++++++++ package/fake-hwclock/fake-hwclock.hash | 2 ++ package/fake-hwclock/fake-hwclock.mk | 40 ++++++++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+) create mode 100644 package/fake-hwclock/Config.in create mode 100644 package/fake-hwclock/fake-hwclock.hash create mode 100644 package/fake-hwclock/fake-hwclock.mk