RFC newpackage: fake-hwclock
PATCH 1/1 RFC newpackage: fake-hwclock
Save/restore system clock on machines without working RTC hardware
https://git.einval.com/cgi-bin/gitweb.cgi?p=fake-hwclock.git
This is a RFC, am I missing anything?
The version is 0.11, but I am not sure where this should be noted.
--- package/Config.in | 1 + package/fake-hwclock/001-start-stop.patch | 20 ++++++++++++++++++++ package/fake-hwclock/Config.in | 9 +++++++++ package/fake-hwclock/fake-hwclock.cronjob | 1 + package/fake-hwclock/fake-hwclock.mk | 31 +++++++++++++++++++++++++++++++ 5 files changed, 62 insertions(+) create mode 100644 package/fake-hwclock/001-start-stop.patch create mode 100644 package/fake-hwclock/Config.in create mode 100644 package/fake-hwclock/fake-hwclock.cronjob create mode 100644 package/fake-hwclock/fake-hwclock.mk
package/Config.in +1 -0
@@ -1981,6 +1981,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"
package/fake-hwclock/001-start-stop.patch new file +20 -0
@@ -0,0 +1,20 @@ +--- a/fake-hwclock.orig 2018-07-18 07:14:29.943169961 -0700 ++++ a/fake-hwclock 2018-07-18 07:14:50.894364970 -0700 +@@ -30,7 +30,7 @@ + fi + + case $COMMAND in +- save) ++ save|stop) + if [ -e $FILE ] ; then + NOW_SEC=$(date -u '+%s') + if $FORCE || [ $NOW_SEC -ge $HWCLOCK_EPOCH_SEC ] ; then +@@ -45,7 +45,7 @@ + date -u '+%Y-%m-%d %H:%M:%S' > $FILE + fi + ;; +- load) ++ start|load) + if [ -e $FILE ] ; then + SAVED="$(cat $FILE)" + SAVED_SEC=$(date -u -d "$SAVED" '+%s')
package/fake-hwclock/Config.in new file +9 -0
@@ -0,0 +1,9 @@ +config BR2_PACKAGE_FAKE_HWCLOCK + bool "fake-hwclock" + depends on BR2_INIT_SYSV + help + a script to load and save creenet time for systems without a + hardware RTC + +comment "fake-hwclock needs SysV init" + depends on !BR2_INIT_SYSV
package/fake-hwclock/fake-hwclock.cronjob new file +1 -0
@@ -0,0 +1 @@ +date -u '+%Y-%m-%d %H:%M:%S' > /etc/fake-hwclock.data
package/fake-hwclock/fake-hwclock.mk new file +31 -0
@@ -0,0 +1,31 @@ +################################################################################ +# +# fake-hwclock +# +################################################################################ + +FAKE_HWCLOCK_VERSION = f889fd09f2d8d55819dd53785e8bd895866e6628 +FAKE_HWCLOCK_SITE = https://git.einval.com/git/fake-hwclock.git +FAKE_HWCLOCK_SITE_METHOD=git +FAKE_HWCLOCK_LICENSE_FILES = COPYING + +# add a daily cron job to save current time +ifeq ($(BR2_PACKAGE_DCRON),y) +FAKE_HWCLOCK_DEPENDENCIES += dcron +define FAKE_HWCLOCK_CROBJOB + $(INSTALL) -D -m 755 $(FAKE_HWCLOCK_PKGDIR)/fake-hwclock.cronjob $(TARGET_DIR)/etc/cron.daily/fake-hwclock +endef +FAKE_HWCLOCK_POST_INSTALL_TARGET_HOOKS += FAKE_HWCLOCK_CROBJOB +endif + +# This should run before everything else +define FAKE_HWCLOCK_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 $(@D)/fake-hwclock $(TARGET_DIR)/etc/init.d/S00fake-hwclock +endef + +# Use buildroot buildtime as a seed. Not ideal, but better than 1970-01-01 UTC +define FAKE_HWCLOCK_INSTALL_TARGET_CMDS + date -u '+%Y-%m-%d %H:%M:%S' > $(TARGET_DIR)/etc/fake-hwclock.data +endef + +$(eval $(generic-package))
Signature · 1 line
2.14.4
PATCH 1/1 RFC newpackage: fake-hwclock
Hello Christopher,
Thanks for this contribution.
Quoted 5 lines · Christopher McCrory
Save/restore system clock on machines without working RTC hardware
https://git.einval.com/cgi-bin/gitweb.cgi?p=fake-hwclock.git
This is a RFC, am I missing anything?
See the review comments below. But first and foremost, you should not include "personal" questions/messages in the commit description itself. Such questions/comments should go after the "---" sign, i.e
Quoted 5 lines
The version is 0.11, but I am not sure where this should be noted.
Signed-off-by Christopher McCrory <[email protected]>---
... here. This
Indeed, the commit description is preserved in the Buildroot Git history, while comments after the "---" sign are not.
Another formatting comment: the title of the commit should be just:
fake-hwclock: new package
Quoted 5 lines
package/Config.in | 1 + package/fake-hwclock/001-start-stop.patch | 20 ++++++++++++++++++++ package/fake-hwclock/Config.in | 9 +++++++++ package/fake-hwclock/fake-hwclock.cronjob | 1 + package/fake-hwclock/fake-hwclock.mk | 31 +++++++++++++++++++++++++++++++
We will need an entry added in the DEVELOPERS file for this package.
package/fake-hwclock/001-start-stop.patch
Quoted 17 lines
package/Config.in +1
-0@@ -1981,6 +1981,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"package/fake-hwclock/001-start-stop.patch new file +0
-0
Patches should have a description and a Signed-off-by line. Since the upstream project uses Git, we like to have Git-formatted patches (i.e patches generated by git format-patch).
Quoted 27 lines
package/fake-hwclock/001-start-stop.patch new file +20
-0@@ -0,0 +1,20 @@ +--- a/fake-hwclock.orig 2018-07-18 07:14:29.943169961 -0700 ++++ a/fake-hwclock 2018-07-18 07:14:50.894364970 -0700 +@@ -30,7 +30,7 @@ + fi + + case $COMMAND in +- save) ++ save|stop) + if [ -e $FILE ] ; then + NOW_SEC=$(date -u '+%s') + if $FORCE || [ $NOW_SEC -ge $HWCLOCK_EPOCH_SEC ] ; then +@@ -45,7 +45,7 @@ + date -u '+%Y-%m-%d %H:%M:%S' > $FILE + fi + ;; +- load) ++ start|load) + if [ -e $FILE ] ; then + SAVED="$(cat $FILE)" + SAVED_SEC=$(date -u -d "$SAVED" '+%s')package/fake-hwclock/Config.in new file +0
-0
package/fake-hwclock/Config.in line 3
Quoted 3 lines
package/fake-hwclock/Config.in new file +3
-0@@ -0,0 +1,9 @@ +config BR2_PACKAGE_FAKE_HWCLOCK + bool "fake-hwclock" + depends on BR2_INIT_SYSV
Why only BR2_INIT_SYSV and not BR2_INIT_BUSYBOX ? Do we really a dependency on the init system here ?
package/fake-hwclock/fake-hwclock.cronjob line 1
Quoted 13 lines
package/fake-hwclock/Config.in new file +6
-0+ help + a script to load and save creenet time for systems without a + hardware RTC + +comment "fake-hwclock needs SysV init" + depends on !BR2_INIT_SYSVpackage/fake-hwclock/fake-hwclock.cronjob new file +1
-0@@ -0,0 +1 @@ +date -u '+%Y-%m-%d %H:%M:%S' > /etc/fake-hwclock.data
What about using debian/fake-hwclock.cron.hourly from the upstream Git repository instead ?
This package will also need a .hash file, with the hash of the tarball, and the hash of the license file.
package/fake-hwclock/fake-hwclock.mk line 7
Quoted 13 lines
package/fake-hwclock/fake-hwclock.mk new file +7
-0@@ -0,0 +1,31 @@ +################################################################################ +# +# fake-hwclock +# +################################################################################ + +FAKE_HWCLOCK_VERSION = f889fd09f2d8d55819dd53785e8bd895866e6628
What about using the v0.11 tag instead, which makes it clear which version is being used ?
package/fake-hwclock/fake-hwclock.mk line 9
Quoted 2 lines
package/fake-hwclock/fake-hwclock.mk new file +2
-0+FAKE_HWCLOCK_SITE = https://git.einval.com/git/fake-hwclock.git +FAKE_HWCLOCK_SITE_METHOD=git
Spaces around the "=" sign.
package/fake-hwclock/fake-hwclock.mk line 10
Quoted 1 line
package/fake-hwclock/fake-hwclock.mk new file +1
-0+FAKE_HWCLOCK_LICENSE_FILES = COPYING
Please add FAKE_HWCLOCK_LICENSE = GPL-2.0
package/fake-hwclock/fake-hwclock.mk line 12
Quoted 1 line
package/fake-hwclock/fake-hwclock.mk new file +1
-0+# add a daily cron job to save current time
The Debian packaging makes this an hourly cronjob, perhaps we should do the same ?
package/fake-hwclock/fake-hwclock.mk line 14
Quoted 2 lines
package/fake-hwclock/fake-hwclock.mk new file +2
-0+ifeq ($(BR2_PACKAGE_DCRON),y) +FAKE_HWCLOCK_DEPENDENCIES += dcron
I don't think you need dcron to be built before.
package/fake-hwclock/fake-hwclock.mk line 15
Quoted 1 line
package/fake-hwclock/fake-hwclock.mk new file +1
-0+define FAKE_HWCLOCK_CROBJOB
CROBJOB or CRONJOB ? :-)
package/fake-hwclock/fake-hwclock.mk line 19
Quoted 4 lines
package/fake-hwclock/fake-hwclock.mk new file +4
-0+ $(INSTALL) -D -m 755 $(FAKE_HWCLOCK_PKGDIR)/fake-hwclock.cronjob $(TARGET_DIR)/etc/cron.daily/fake-hwclock +endef +FAKE_HWCLOCK_POST_INSTALL_TARGET_HOOKS += FAKE_HWCLOCK_CROBJOB +endif
Also, I'm not sure why this is limited to dcron. There is a cron/crontab implementation in Busybox, which should be sufficient for this. However, I don't think we have other packages that install cronjobs for the moment, so it's a bit of a new thing.
Perhaps we should just unconditionally install the cronjob file to /etc/cron.hourly/ ? Or only do so when either Busybox or dcron are enabled ? Are there other cron implementations besides Busybox and dcron ?
package/fake-hwclock/fake-hwclock.mk line 31
Quoted 11 lines
package/fake-hwclock/fake-hwclock.mk new file +11
-0+# This should run before everything else +define FAKE_HWCLOCK_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 $(@D)/fake-hwclock $(TARGET_DIR)/etc/init.d/S00fake-hwclock +endef + +# Use buildroot buildtime as a seed. Not ideal, but better than 1970-01-01 UTC +define FAKE_HWCLOCK_INSTALL_TARGET_CMDS + date -u '+%Y-%m-%d %H:%M:%S' > $(TARGET_DIR)/etc/fake-hwclock.data +endef + +$(eval $(generic-package))
The rest looks good to me.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com