* [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