* [Buildroot] [PATCH 1/1] tzdata: Improve right/posix installation @ 2016-10-25 21:48 Mylène Josserand 2016-10-25 22:09 ` Thomas Petazzoni 0 siblings, 1 reply; 3+ messages in thread From: Mylène Josserand @ 2016-10-25 21:48 UTC (permalink / raw) To: buildroot Improve the right/posix timezone installation by creating new options to choose between these two versions. As one version is necessary, the default one is posix. Now, the user can install both versions or one of both. It reduces the image size when only one version is needed. Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com> --- package/tzdata/tzdata.mk | 26 ++++++++++++++++++++------ system/Config.in | 15 +++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/package/tzdata/tzdata.mk b/package/tzdata/tzdata.mk index de7d1be..d1ed21c 100644 --- a/package/tzdata/tzdata.mk +++ b/package/tzdata/tzdata.mk @@ -32,10 +32,20 @@ TZDATA_EXTRACT_CMDS = define TZDATA_INSTALL_TARGET_CMDS $(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/share/zoneinfo cp -a $(HOST_DIR)/usr/share/zoneinfo/* $(TARGET_DIR)/usr/share/zoneinfo - cd $(TARGET_DIR)/usr/share/zoneinfo; \ - for zone in posix/*; do \ - ln -sfn "$${zone}" "$${zone##*/}"; \ - done + cd $(TARGET_DIR)/usr/share/zoneinfo; \ + if [ -n "$(BR2_TARGET_TZ_POSIX)" ]; then \ + for zone in posix/*; do \ + ln -sfn "$${zone}" "$${zone##*/}"; \ + done \ + fi; + # If "right" is the only version enabled + cd $(TARGET_DIR)/usr/share/zoneinfo; \ + if [ -n "$(BR2_TARGET_TZ_RIGHT)" ] && [ -z "$(BR2_TARGET_TZ_POSIX)" ]; then \ + for zone in right/*; do \ + ln -sfn "$${zone}" "$${zone##*/}"; \ + done \ + fi; + if [ -n "$(TZDATA_LOCALTIME)" ]; then \ 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" \ @@ -51,8 +61,12 @@ endef define HOST_TZDATA_BUILD_CMDS (cd $(@D); \ for zone in $(TZDATA_ZONELIST); do \ - $(ZIC) -d _output/posix -y yearistype.sh $$zone || exit 1; \ - $(ZIC) -d _output/right -L leapseconds -y yearistype.sh $$zone || exit 1; \ + if [ -n "$(BR2_TARGET_TZ_POSIX)" ]; then \ + $(ZIC) -d _output/posix -y yearistype.sh $$zone || exit 1; \ + fi; \ + if [ -n "$(BR2_TARGET_TZ_RIGHT)" ]; then \ + $(ZIC) -d _output/right -L leapseconds -y yearistype.sh $$zone || exit 1; \ + fi; \ done; \ ) endef diff --git a/system/Config.in b/system/Config.in index f9a3bda..dcc9f6b 100644 --- a/system/Config.in +++ b/system/Config.in @@ -429,6 +429,7 @@ config BR2_TARGET_TZ_INFO select BR2_PACKAGE_TZDATA if BR2_TOOLCHAIN_USES_GLIBC select BR2_PACKAGE_TZDATA if BR2_TOOLCHAIN_USES_MUSL select BR2_PACKAGE_TZ if BR2_TOOLCHAIN_USES_UCLIBC + select BR2_TARGET_TZ_POSIX if ! BR2_TARGET_TZ_RIGHT help Say 'y' here to install timezone info. @@ -461,6 +462,20 @@ config BR2_TARGET_LOCALTIME If empty, no local time will be set, and the dates will be expressed in UTC. +config BR2_TARGET_TZ_POSIX + tristate "Install the Posix timezone" + help + The time zone can be provided by two versions : a posix and a right version. + The "posix" version is based on the Coordinated Universal Time (UTC). + This is the default one. + +config BR2_TARGET_TZ_RIGHT + tristate "Install the Right timezone" + help + The time zone can be provided by two versions : a posix and a right version. + The "right" version is based on the International Atomic Time (TAI) and + includes the leap seconds. + endif # BR2_TARGET_TZ_INFO config BR2_ROOTFS_USERS_TABLES -- 2.9.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Buildroot] [PATCH 1/1] tzdata: Improve right/posix installation 2016-10-25 21:48 [Buildroot] [PATCH 1/1] tzdata: Improve right/posix installation Mylène Josserand @ 2016-10-25 22:09 ` Thomas Petazzoni 2016-10-27 20:04 ` Mylene Josserand 0 siblings, 1 reply; 3+ messages in thread From: Thomas Petazzoni @ 2016-10-25 22:09 UTC (permalink / raw) To: buildroot Hello, On Tue, 25 Oct 2016 23:48:26 +0200, Myl?ne Josserand wrote: > Improve the right/posix timezone installation by creating new options > to choose between these two versions. As one version is necessary, the > default one is posix. > Now, the user can install both versions or one of both. It reduces the > image size when only one version is needed. > > Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com> Thanks Myl?ne for your patch. It would be good to explain in the commit log why you are proposing this change. > diff --git a/package/tzdata/tzdata.mk b/package/tzdata/tzdata.mk > index de7d1be..d1ed21c 100644 > --- a/package/tzdata/tzdata.mk > +++ b/package/tzdata/tzdata.mk > @@ -32,10 +32,20 @@ TZDATA_EXTRACT_CMDS = > define TZDATA_INSTALL_TARGET_CMDS > $(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/share/zoneinfo > cp -a $(HOST_DIR)/usr/share/zoneinfo/* $(TARGET_DIR)/usr/share/zoneinfo > - cd $(TARGET_DIR)/usr/share/zoneinfo; \ > - for zone in posix/*; do \ > - ln -sfn "$${zone}" "$${zone##*/}"; \ > - done > + cd $(TARGET_DIR)/usr/share/zoneinfo; \ > + if [ -n "$(BR2_TARGET_TZ_POSIX)" ]; then \ > + for zone in posix/*; do \ > + ln -sfn "$${zone}" "$${zone##*/}"; \ > + done \ > + fi; > + # If "right" is the only version enabled > + cd $(TARGET_DIR)/usr/share/zoneinfo; \ > + if [ -n "$(BR2_TARGET_TZ_RIGHT)" ] && [ -z "$(BR2_TARGET_TZ_POSIX)" ]; then \ > + for zone in right/*; do \ > + ln -sfn "$${zone}" "$${zone##*/}"; \ > + done \ > + fi; It's a bit late now, so I'm not sure what's going on here. But for sure, I'd like this to be handled using make conditionals and not shell conditionals. > + > if [ -n "$(TZDATA_LOCALTIME)" ]; then \ > 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" \ > @@ -51,8 +61,12 @@ endef > define HOST_TZDATA_BUILD_CMDS > (cd $(@D); \ > for zone in $(TZDATA_ZONELIST); do \ > - $(ZIC) -d _output/posix -y yearistype.sh $$zone || exit 1; \ > - $(ZIC) -d _output/right -L leapseconds -y yearistype.sh $$zone || exit 1; \ > + if [ -n "$(BR2_TARGET_TZ_POSIX)" ]; then \ > + $(ZIC) -d _output/posix -y yearistype.sh $$zone || exit 1; \ > + fi; \ > + if [ -n "$(BR2_TARGET_TZ_RIGHT)" ]; then \ > + $(ZIC) -d _output/right -L leapseconds -y yearistype.sh $$zone || exit 1; \ > + fi; \ > done; \ Ditto here, we need to use more the make language and less the shell language. Perhaps: ifeq ($(BR2_TARGET_TZ_POSIX),y) define HOST_TZDATA_BUILD_TZ_POSIX cd $(@D) ; \ $(foreach zone,$(TZDATA_ZONELIST),\ $(ZIC) -d output/posix -y yearistype.sh $(zone) ) endef endif ifeq ($(BR2_TARGET_TZ_RIGHT),y) define HOST_TZDATA_BUILD_TZ_RIGHT cd $(@D) ; \ $(foreach zone,$(TZDATA_ZONELIST),\ $(ZIC) -d output/right -L leapseconds -y yearistype.sh $(zone) ) endef endif define HOST_TZDATA_BUILD_CMDS $(HOST_TZDATA_BUILD_TZ_POSIX) $(HOST_TZDATA_BUILD_TZ_RIGHT) endef > ) > endef > diff --git a/system/Config.in b/system/Config.in > index f9a3bda..dcc9f6b 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -429,6 +429,7 @@ config BR2_TARGET_TZ_INFO > select BR2_PACKAGE_TZDATA if BR2_TOOLCHAIN_USES_GLIBC > select BR2_PACKAGE_TZDATA if BR2_TOOLCHAIN_USES_MUSL > select BR2_PACKAGE_TZ if BR2_TOOLCHAIN_USES_UCLIBC > + select BR2_TARGET_TZ_POSIX if ! BR2_TARGET_TZ_RIGHT No space after ! > help > Say 'y' here to install timezone info. > > @@ -461,6 +462,20 @@ config BR2_TARGET_LOCALTIME > If empty, no local time will be set, and the dates will be > expressed in UTC. > > +config BR2_TARGET_TZ_POSIX > + tristate "Install the Posix timezone" Don't use tristate, there is no meaning in Buildroot for an option to be enabled as a module. Also, you want to have: default y in order to preserve the existing behavior. > + help > + The time zone can be provided by two versions : a posix and a right version. > + The "posix" version is based on the Coordinated Universal Time (UTC). > + This is the default one. Lines are too long, they should be wrapper at 72 characters. Also, I think a better wording would be: Each time zone is provided in two versions: a "posix" version and a "right" version. This option enables the installation of the "posix" version of all time zone data. The "posix" version is based on the Coordinated Universal Time (UTC), and does not take leap seconds into account. > + > +config BR2_TARGET_TZ_RIGHT > + tristate "Install the Right timezone" Same comments as above: use bool, and add "default y". > + help > + The time zone can be provided by two versions : a posix and a right version. > + The "right" version is based on the International Atomic Time (TAI) and > + includes the leap seconds. Similar rework of the help text would be useful. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 3+ messages in thread
* [Buildroot] [PATCH 1/1] tzdata: Improve right/posix installation 2016-10-25 22:09 ` Thomas Petazzoni @ 2016-10-27 20:04 ` Mylene Josserand 0 siblings, 0 replies; 3+ messages in thread From: Mylene Josserand @ 2016-10-27 20:04 UTC (permalink / raw) To: buildroot Hello, On 26/10/2016 00:09, Thomas Petazzoni wrote: > Hello, > > On Tue, 25 Oct 2016 23:48:26 +0200, Myl?ne Josserand wrote: >> Improve the right/posix timezone installation by creating new options >> to choose between these two versions. As one version is necessary, the >> default one is posix. >> Now, the user can install both versions or one of both. It reduces the >> image size when only one version is needed. >> >> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com> > > Thanks Myl?ne for your patch. It would be good to explain in the commit > log why you are proposing this change. Okay, I will try to be more explicit in the V2. >> diff --git a/package/tzdata/tzdata.mk b/package/tzdata/tzdata.mk >> index de7d1be..d1ed21c 100644 >> --- a/package/tzdata/tzdata.mk >> +++ b/package/tzdata/tzdata.mk >> @@ -32,10 +32,20 @@ TZDATA_EXTRACT_CMDS = >> define TZDATA_INSTALL_TARGET_CMDS >> $(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/share/zoneinfo >> cp -a $(HOST_DIR)/usr/share/zoneinfo/* $(TARGET_DIR)/usr/share/zoneinfo >> - cd $(TARGET_DIR)/usr/share/zoneinfo; \ >> - for zone in posix/*; do \ >> - ln -sfn "$${zone}" "$${zone##*/}"; \ >> - done >> + cd $(TARGET_DIR)/usr/share/zoneinfo; \ >> + if [ -n "$(BR2_TARGET_TZ_POSIX)" ]; then \ >> + for zone in posix/*; do \ >> + ln -sfn "$${zone}" "$${zone##*/}"; \ >> + done \ >> + fi; >> + # If "right" is the only version enabled >> + cd $(TARGET_DIR)/usr/share/zoneinfo; \ >> + if [ -n "$(BR2_TARGET_TZ_RIGHT)" ] && [ -z "$(BR2_TARGET_TZ_POSIX)" ]; then \ >> + for zone in right/*; do \ >> + ln -sfn "$${zone}" "$${zone##*/}"; \ >> + done \ >> + fi; > > It's a bit late now, so I'm not sure what's going on here. But for > sure, I'd like this to be handled using make conditionals and not shell > conditionals. The current version creates links to have /usr/share/zoneinfo/Etc/ which is linked to /usr/share/zoneinfo/posix/Etc/, for example. In case we have only "right" version installed, these links must be updated to use the "right" folder instead of the "posix" one. This is what I added here. >> + >> if [ -n "$(TZDATA_LOCALTIME)" ]; then \ >> 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" \ >> @@ -51,8 +61,12 @@ endef >> define HOST_TZDATA_BUILD_CMDS >> (cd $(@D); \ >> for zone in $(TZDATA_ZONELIST); do \ >> - $(ZIC) -d _output/posix -y yearistype.sh $$zone || exit 1; \ >> - $(ZIC) -d _output/right -L leapseconds -y yearistype.sh $$zone || exit 1; \ >> + if [ -n "$(BR2_TARGET_TZ_POSIX)" ]; then \ >> + $(ZIC) -d _output/posix -y yearistype.sh $$zone || exit 1; \ >> + fi; \ >> + if [ -n "$(BR2_TARGET_TZ_RIGHT)" ]; then \ >> + $(ZIC) -d _output/right -L leapseconds -y yearistype.sh $$zone || exit 1; \ >> + fi; \ >> done; \ > > Ditto here, we need to use more the make language and less the shell > language. > > Perhaps: > > ifeq ($(BR2_TARGET_TZ_POSIX),y) > define HOST_TZDATA_BUILD_TZ_POSIX > cd $(@D) ; \ > $(foreach zone,$(TZDATA_ZONELIST),\ > $(ZIC) -d output/posix -y yearistype.sh $(zone) > ) > endef > endif > > ifeq ($(BR2_TARGET_TZ_RIGHT),y) > define HOST_TZDATA_BUILD_TZ_RIGHT > cd $(@D) ; \ > $(foreach zone,$(TZDATA_ZONELIST),\ > $(ZIC) -d output/right -L leapseconds -y yearistype.sh $(zone) > ) > endef > endif > > define HOST_TZDATA_BUILD_CMDS > $(HOST_TZDATA_BUILD_TZ_POSIX) > $(HOST_TZDATA_BUILD_TZ_RIGHT) > endef > >> ) >> endef okay, I see. I did not know that the make language should be preferred. I will keep it in mind for next times. >> diff --git a/system/Config.in b/system/Config.in >> index f9a3bda..dcc9f6b 100644 >> --- a/system/Config.in >> +++ b/system/Config.in >> @@ -429,6 +429,7 @@ config BR2_TARGET_TZ_INFO >> select BR2_PACKAGE_TZDATA if BR2_TOOLCHAIN_USES_GLIBC >> select BR2_PACKAGE_TZDATA if BR2_TOOLCHAIN_USES_MUSL >> select BR2_PACKAGE_TZ if BR2_TOOLCHAIN_USES_UCLIBC >> + select BR2_TARGET_TZ_POSIX if ! BR2_TARGET_TZ_RIGHT > > No space after ! > Noted. >> help >> Say 'y' here to install timezone info. >> >> @@ -461,6 +462,20 @@ config BR2_TARGET_LOCALTIME >> If empty, no local time will be set, and the dates will be >> expressed in UTC. >> >> +config BR2_TARGET_TZ_POSIX >> + tristate "Install the Posix timezone" > > Don't use tristate, there is no meaning in Buildroot for an option to > be enabled as a module. > > Also, you want to have: > > default y > > in order to preserve the existing behavior. > Okay >> + help >> + The time zone can be provided by two versions : a posix and a right version. >> + The "posix" version is based on the Coordinated Universal Time (UTC). >> + This is the default one. > > Lines are too long, they should be wrapper at 72 characters. Also, I > think a better wording would be: > > Each time zone is provided in two versions: a "posix" version > and a "right" version. This option enables the installation > of the "posix" version of all time zone data. The "posix" > version is based on the Coordinated Universal Time (UTC), and > does not take leap seconds into account. > Thank you for the new description. >> + >> +config BR2_TARGET_TZ_RIGHT >> + tristate "Install the Right timezone" > > Same comments as above: use bool, and add "default y". > >> + help >> + The time zone can be provided by two versions : a posix and a right version. >> + The "right" version is based on the International Atomic Time (TAI) and >> + includes the leap seconds. > > Similar rework of the help text would be useful. Thanks for the review! Best regards, -- Myl?ne Josserand, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-27 20:04 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-25 21:48 [Buildroot] [PATCH 1/1] tzdata: Improve right/posix installation Mylène Josserand 2016-10-25 22:09 ` Thomas Petazzoni 2016-10-27 20:04 ` Mylene Josserand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox