| Message ID | 20191116091004.23726-1-yann.morin.1998@free.fr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | system: allow not setting a default, system-wide time zone | expand |
Hi Yann, On 16/11/2019 10:10, Yann E. MORIN wrote: > It is valid that there is no system-wide default time zone defined, in > which case Etc/UTC is assumed. > > Fixes: #12316 > > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > Cc: Martin Bark <martin@barkynet.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Richard Braun <rbraun@sceen.net> > Cc: Andrew Trapani <andrew.trapani@ontera.bio> [snip] > diff --git a/system/Config.in b/system/Config.in > index c87266f431..c8c5be40e0 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -494,6 +494,8 @@ config BR2_TARGET_LOCALTIME > Pacific/Wallis > ... > > + Set to empty to not install a default time zone. Maybe we should then also change the default to empty? Then at least this use case gets tested in the autobuilders. Oh, maybe adding a few possible values for this config in genrandconfig would be nice as well... BTW, I notice now that BR2_TARGET_TZ_ZONELIST is not used for uClibc (tz). Is that expected? Maybe it should depend on !UCLIBC then? Regards, Arnout > + > endif # BR2_TARGET_TZ_INFO > > config BR2_ROOTFS_USERS_TABLES >
On Sun, 17 Nov 2019 15:41:59 +0100 Arnout Vandecappelle <arnout@mind.be> wrote: > > + Set to empty to not install a default time zone. > > Maybe we should then also change the default to empty? Then at least this use > case gets tested in the autobuilders. Oh, maybe adding a few possible values for > this config in genrandconfig would be nice as well... I think this kind of feature gets better tested by having runtime tests, which can specifically test the interesting configurations. Thomas
On 18/11/2019 17:11, Thomas Petazzoni wrote: > On Sun, 17 Nov 2019 15:41:59 +0100 > Arnout Vandecappelle <arnout@mind.be> wrote: > >>> + Set to empty to not install a default time zone. >> >> Maybe we should then also change the default to empty? Then at least this use >> case gets tested in the autobuilders. Oh, maybe adding a few possible values for >> this config in genrandconfig would be nice as well... > > I think this kind of feature gets better tested by having runtime > tests, which can specifically test the interesting configurations. I didn't express myself very well... The change default to empty was independent of the testing. I just thought that it makes more sense to not install a default timezone than to install UTC as default (net result is equivalent, but semantically no timezone makes more sense IMO). And then I thought: that's nice, that way the feature also gets tested in the autobuilders. Regards, Arnout
Arnout, All, On 2019-11-17 15:41 +0100, Arnout Vandecappelle spake thusly: > On 16/11/2019 10:10, Yann E. MORIN wrote: > > It is valid that there is no system-wide default time zone defined, in > > which case Etc/UTC is assumed. > > > > Fixes: #12316 > > > > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > > Cc: Martin Bark <martin@barkynet.com> > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Cc: Richard Braun <rbraun@sceen.net> > > Cc: Andrew Trapani <andrew.trapani@ontera.bio> > [snip] > > diff --git a/system/Config.in b/system/Config.in > > index c87266f431..c8c5be40e0 100644 > > --- a/system/Config.in > > +++ b/system/Config.in > > @@ -494,6 +494,8 @@ config BR2_TARGET_LOCALTIME > > Pacific/Wallis > > ... > > > > + Set to empty to not install a default time zone. > > Maybe we should then also change the default to empty? I disagree. I prefer there is an explicit default, which makes it obvious what it means, rather than use an implicit setting. > Then at least this use > case gets tested in the autobuilders. Oh, maybe adding a few possible values for > this config in genrandconfig would be nice as well... If you enable the randomisation in genrandconfig, then there is no longer any reason to set the default to empty, is there? > BTW, I notice now that BR2_TARGET_TZ_ZONELIST is not used for uClibc (tz). Is > that expected? Maybe it should depend on !UCLIBC then? I think this should be done in a further patch. Having a value that is not used in the uClibc case is not a regression: it's been like that for years now, since 2014 with commit 337fbd549c. On the other hand, this patch fixes an existing issue (#12316). Regards, Yann E. MORIN.
On Sat, 16 Nov 2019 10:10:04 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > It is valid that there is no system-wide default time zone defined, in > which case Etc/UTC is assumed. > > Fixes: #12316 > > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > Cc: Martin Bark <martin@barkynet.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Richard Braun <rbraun@sceen.net> > Cc: Andrew Trapani <andrew.trapani@ontera.bio> > --- > package/tz/tz.mk | 17 +++++++++++------ > package/tzdata/tzdata.mk | 19 ++++++++++++------- > system/Config.in | 2 ++ > 3 files changed, 25 insertions(+), 13 deletions(-) Applied to master, thanks. Thomas
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > It is valid that there is no system-wide default time zone defined, in > which case Etc/UTC is assumed. > Fixes: #12316 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > Cc: Martin Bark <martin@barkynet.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Richard Braun <rbraun@sceen.net> > Cc: Andrew Trapani <andrew.trapani@ontera.bio> Committed to 2019.02.x and 2019.08.x, thanks.
diff --git a/package/tz/tz.mk b/package/tz/tz.mk index 1e797046d2..135726c2ce 100644 --- a/package/tz/tz.mk +++ b/package/tz/tz.mk @@ -8,6 +8,16 @@ TZ_DEPENDENCIES = host-tzdata host-tzdump TZ_LICENSE = Public domain TZ_LOCALTIME = $(call qstrip,$(BR2_TARGET_LOCALTIME)) +ifneq ($(TZ_LOCALTIME),) +define TZ_SET_LOCALTIME + if [ ! -f $(TARGET_DIR)/usr/share/zoneinfo/uclibc/$(TZ_LOCALTIME) ]; then \ + printf "Error: '%s' is not a valid timezone, check your BR2_TARGET_LOCALTIME setting\n" \ + "$(TZ_LOCALTIME)"; \ + exit 1; \ + fi + ln -sf ../usr/share/zoneinfo/uclibc/$(TZ_LOCALTIME) $(TARGET_DIR)/etc/TZ +endef +endif define TZ_BUILD_CMDS (cd $(HOST_DIR)/share/zoneinfo/posix/; \ @@ -25,12 +35,7 @@ define TZ_INSTALL_TARGET_CMDS $(TARGET_DIR)/usr/share/zoneinfo/iso3166.tab mkdir -p $(TARGET_DIR)/usr/share/zoneinfo/uclibc cp -a $(@D)/output/* $(TARGET_DIR)/usr/share/zoneinfo/uclibc - if [ ! -f $(TARGET_DIR)/usr/share/zoneinfo/uclibc/$(TZ_LOCALTIME) ]; then \ - printf "Error: '%s' is not a valid timezone, check your BR2_TARGET_LOCALTIME setting\n" \ - "$(TZ_LOCALTIME)"; \ - exit 1; \ - fi - ln -sf ../usr/share/zoneinfo/uclibc/$(TZ_LOCALTIME) $(TARGET_DIR)/etc/TZ + $(TZ_SET_LOCALTIME) endef $(eval $(generic-package)) diff --git a/package/tzdata/tzdata.mk b/package/tzdata/tzdata.mk index 4d097dbee9..b656bc7f7f 100644 --- a/package/tzdata/tzdata.mk +++ b/package/tzdata/tzdata.mk @@ -26,6 +26,17 @@ TZDATA_ZONELIST = $(call qstrip,$(BR2_TARGET_TZ_ZONELIST)) endif TZDATA_LOCALTIME = $(call qstrip,$(BR2_TARGET_LOCALTIME)) +ifneq ($(TZDATA_LOCALTIME),) +define TZDATA_SET_LOCALTIME + if [ ! -f $(TARGET_DIR)/usr/share/zoneinfo/$(TZDATA_LOCALTIME) ]; then \ + printf "Error: '%s' is not a valid timezone, check your BR2_TARGET_LOCALTIME setting\n" \ + "$(TZDATA_LOCALTIME)"; \ + exit 1; \ + fi + ln -sf ../usr/share/zoneinfo/$(TZDATA_LOCALTIME) $(TARGET_DIR)/etc/localtime + echo "$(TZDATA_LOCALTIME)" >$(TARGET_DIR)/etc/timezone +endef +endif # No need to extract for target, we're using the host-installed files TZDATA_EXTRACT_CMDS = @@ -37,13 +48,7 @@ define TZDATA_INSTALL_TARGET_CMDS for zone in posix/*; do \ ln -sfn "$${zone}" "$${zone##*/}"; \ done - if [ ! -f $(TARGET_DIR)/usr/share/zoneinfo/$(TZDATA_LOCALTIME) ]; then \ - printf "Error: '%s' is not a valid timezone, check your BR2_TARGET_LOCALTIME setting\n" \ - "$(TZDATA_LOCALTIME)"; \ - exit 1; \ - fi - ln -sf ../usr/share/zoneinfo/$(TZDATA_LOCALTIME) $(TARGET_DIR)/etc/localtime - echo "$(TZDATA_LOCALTIME)" >$(TARGET_DIR)/etc/timezone + $(TZDATA_SET_LOCALTIME) endef define HOST_TZDATA_BUILD_CMDS diff --git a/system/Config.in b/system/Config.in index c87266f431..c8c5be40e0 100644 --- a/system/Config.in +++ b/system/Config.in @@ -494,6 +494,8 @@ config BR2_TARGET_LOCALTIME Pacific/Wallis ... + Set to empty to not install a default time zone. + endif # BR2_TARGET_TZ_INFO config BR2_ROOTFS_USERS_TABLES
It is valid that there is no system-wide default time zone defined, in which case Etc/UTC is assumed. Fixes: #12316 Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> Cc: Martin Bark <martin@barkynet.com> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Richard Braun <rbraun@sceen.net> Cc: Andrew Trapani <andrew.trapani@ontera.bio> --- package/tz/tz.mk | 17 +++++++++++------ package/tzdata/tzdata.mk | 19 ++++++++++++------- system/Config.in | 2 ++ 3 files changed, 25 insertions(+), 13 deletions(-)