| Message ID | 20240228145642.436509-2-christian@klarinett.li |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] package/zic: add hash for existing patch | expand |
Hello Christian, On Wed, 28 Feb 2024 15:56:41 +0100 Christian Hitz via buildroot <buildroot@buildroot.org> wrote: > From: Christian Hitz <christian.hitz@bbv.ch> > > Signed-off-by: Christian Hitz <christian.hitz@bbv.ch> Thanks, but for a patch like this, we definitely need a non-empty commit log. Why do you need zic on the target, what is the use-case? > diff --git a/package/zic/Config.in b/package/zic/Config.in > new file mode 100644 > index 0000000000..11a8c99a66 > --- /dev/null > +++ b/package/zic/Config.in > @@ -0,0 +1,6 @@ > +config BR2_PACKAGE_ZIC > + bool "zic" > + help No dependency on toolchain-specific features? Did you test this package with ./utils/test-pkg ? > diff --git a/package/zic/zic.mk b/package/zic/zic.mk > index a915f6d256..12bb9e9943 100644 > --- a/package/zic/zic.mk > +++ b/package/zic/zic.mk > @@ -10,6 +10,19 @@ ZIC_SITE = https://www.iana.org/time-zones/repository/releases > ZIC_STRIP_COMPONENTS = 0 > ZIC_LICENSE = Public domain > ZIC_LICENSE_FILES = LICENSE > +ZIC_INSTALL_STAGING = YES > + > +define ZIC_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" -C $(@D) zic Can we use $(TARGET_CONFIGURE_OPTS) instead of CC="$(TARGET_CC)" ? > +define ZIC_INSTALL_STAGING_CMDS > + $(INSTALL) -D -m 644 $(@D)/tzfile.h $(TARGET_DIR)/usr/include/tzfile.h So just the header file is needed, no library is installed? Best regards, Thomas
Hello Thomas Am 2024-03-26 15:44, schrieb Thomas Petazzoni: > Hello Christian, > > On Wed, 28 Feb 2024 15:56:41 +0100 > Christian Hitz via buildroot <buildroot@buildroot.org> wrote: > >> From: Christian Hitz <christian.hitz@bbv.ch> >> >> Signed-off-by: Christian Hitz <christian.hitz@bbv.ch> > > Thanks, but for a patch like this, we definitely need a non-empty > commit log. Why do you need zic on the target, what is the use-case? When we configure a time zone on our device's GUI we use the standard time zones. This gives us the usual benefits e.g. automatic daylight saving time switching. We also need to support an API that wants to set the "time zone" by specifying the offset in minutes from GMT. The given information is not enough to reliably selecting one of the existing time zones. Instead, when the time zone is set over this API we dynamically create and configure a "Custom" time zone with the given offset. For this we use the `zic` tool on target. > >> diff --git a/package/zic/Config.in b/package/zic/Config.in >> new file mode 100644 >> index 0000000000..11a8c99a66 >> --- /dev/null >> +++ b/package/zic/Config.in >> @@ -0,0 +1,6 @@ >> +config BR2_PACKAGE_ZIC >> + bool "zic" >> + help > > No dependency on toolchain-specific features? Did you test this package > with ./utils/test-pkg ? Yes, ./utils/test-pkg -a is successful. > >> diff --git a/package/zic/zic.mk b/package/zic/zic.mk >> index a915f6d256..12bb9e9943 100644 >> --- a/package/zic/zic.mk >> +++ b/package/zic/zic.mk >> @@ -10,6 +10,19 @@ ZIC_SITE = >> https://www.iana.org/time-zones/repository/releases >> ZIC_STRIP_COMPONENTS = 0 >> ZIC_LICENSE = Public domain >> ZIC_LICENSE_FILES = LICENSE >> +ZIC_INSTALL_STAGING = YES >> + >> +define ZIC_BUILD_CMDS >> + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" -C $(@D) zic > > Can we use $(TARGET_CONFIGURE_OPTS) instead of CC="$(TARGET_CC)" ? Yes, this works. > >> +define ZIC_INSTALL_STAGING_CMDS >> + $(INSTALL) -D -m 644 $(@D)/tzfile.h >> $(TARGET_DIR)/usr/include/tzfile.h > > So just the header file is needed, no library is installed? Actually, this is not required at all. I'll drop it in v2. Thanks for the review. I'll update the patch and will send a v2. Regards, Christian
Hello Christian, On Tue, 26 Mar 2024 17:03:20 +0100 christian@klarinett.li wrote: > > Thanks, but for a patch like this, we definitely need a non-empty > > commit log. Why do you need zic on the target, what is the use-case? > > When we configure a time zone on our device's GUI we use the standard > time zones. This gives us the usual benefits e.g. automatic daylight > saving time switching. > We also need to support an API that wants to set the "time zone" by > specifying the offset in minutes from GMT. The given information > is not enough to reliably selecting one of the existing time zones. > Instead, when the time zone is set over this API we dynamically create > and configure a "Custom" time zone with the given offset. For this > we use the `zic` tool on target. Thanks for the explanation. I'm not too familiar with time zone stuff (or in fact, I did have a look a long time ago, but I forgot the details of how it works), bu your explanation seems to make sense :-) > >> +define ZIC_INSTALL_STAGING_CMDS > >> + $(INSTALL) -D -m 644 $(@D)/tzfile.h > >> $(TARGET_DIR)/usr/include/tzfile.h > > > > So just the header file is needed, no library is installed? > > Actually, this is not required at all. I'll drop it in v2. OK, then you can drop ZIC_INSTALL_STAGING = YES as well. > Thanks for the review. I'll update the patch and will send a v2. Perfect, thanks! Thomas
diff --git a/package/Config.in b/package/Config.in index bf0fe078b9..909b716f16 100644 --- a/package/Config.in +++ b/package/Config.in @@ -2237,6 +2237,7 @@ endif source "package/uvw/Config.in" source "package/volk/Config.in" source "package/xapian/Config.in" + source "package/zic/Config.in" endmenu menu "Security" diff --git a/package/zic/Config.in b/package/zic/Config.in new file mode 100644 index 0000000000..11a8c99a66 --- /dev/null +++ b/package/zic/Config.in @@ -0,0 +1,6 @@ +config BR2_PACKAGE_ZIC + bool "zic" + help + timezone information compiler (zic) + + https://www.iana.org/time-zones diff --git a/package/zic/zic.mk b/package/zic/zic.mk index a915f6d256..12bb9e9943 100644 --- a/package/zic/zic.mk +++ b/package/zic/zic.mk @@ -10,6 +10,19 @@ ZIC_SITE = https://www.iana.org/time-zones/repository/releases ZIC_STRIP_COMPONENTS = 0 ZIC_LICENSE = Public domain ZIC_LICENSE_FILES = LICENSE +ZIC_INSTALL_STAGING = YES + +define ZIC_BUILD_CMDS + $(TARGET_MAKE_ENV) $(MAKE) CC="$(TARGET_CC)" -C $(@D) zic +endef + +define ZIC_INSTALL_STAGING_CMDS + $(INSTALL) -D -m 644 $(@D)/tzfile.h $(TARGET_DIR)/usr/include/tzfile.h +endef + +define ZIC_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 755 $(@D)/zic $(TARGET_DIR)/usr/sbin/zic +endef define HOST_ZIC_BUILD_CMDS $(HOST_MAKE_ENV) $(MAKE) -C $(@D) zic @@ -20,6 +33,7 @@ define HOST_ZIC_INSTALL_CMDS $(INSTALL) -D -m 644 $(@D)/tzfile.h $(HOST_DIR)/include/tzfile.h endef +$(eval $(generic-package)) $(eval $(host-generic-package)) ZIC = $(HOST_DIR)/sbin/zic