Skip to contentSkip to conversation

libical: Create option to use builtin tzdata

PATCH New Marc Khouri 4 participants 6 messages Aug 9, 2016 – Aug 10, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libical: Create option to use builtin tzdata

libical requires time zone data (either system or internal) in order to use time zone related functions. This commit adds an option to build libical with its internal time zone data. If no time zone data is present, segfaults occur in applications which use time zone functions.

Option documentation:
https://github.com/libical/libical/blob/v1.0.1/CMakeLists.txt#L23

---
 package/libical/Config.in  | 12 ++++++++++++
 package/libical/libical.mk |  4 ++++
 2 files changed, 16 insertions(+)
package/libical/Config.in +12 -0
index 48170e9..e183ad0 100644
@@ -9,6 +9,18 @@ config BR2_PACKAGE_LIBICAL
 
 	  http://libical.github.io/libical
 
+if BR2_PACKAGE_LIBICAL
+
+config BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA
+	bool "use built-in timezone data"
+	help
+	  Build using libical's (instead of the system's) timezone data.
+
+	  libical requires either built-in or system timezone data in order
+	  to support time zone functions.
+
+endif
+
 comment "libical needs a toolchain w/ C++, dynamic library, wchar"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
 		BR2_STATIC_LIBS
package/libical/libical.mk +4 -0
index e8cb3f8..a8bcefb 100644
@@ -14,4 +14,8 @@ LIBICAL_LICENSE_FILES = LICENSE
 # Gentoo/alpinelinux as well
 LIBICAL_CONF_OPTS = -DSHARED_ONLY=true
 
+ifeq ($(BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA),y)
+LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
+endif
+
 $(eval $(cmake-package))
Signature · 1 line
2.8.2
#1 lore raw

PATCH 1/1 libical: Create option to use builtin tzdata

Hi Marc,

Quoted 42 lines · Marc Khouri

libical requires time zone data (either system or internal) in order to use time zone related functions. This commit adds an option to build libical with its internal time zone data. If no time zone data is present, segfaults occur in applications which use time zone functions.

Option documentation:
https://github.com/libical/libical/blob/v1.0.1/CMakeLists.txt#L23

---
 package/libical/Config.in  | 12 ++++++++++++
 package/libical/libical.mk |  4 ++++
 2 files changed, 16 insertions(+)
package/libical/Config.in +12 -0
index 48170e9..e183ad0 100644
@@ -9,6 +9,18 @@ config BR2_PACKAGE_LIBICAL
 
 	  http://libical.github.io/libical
 
+if BR2_PACKAGE_LIBICAL
+
+config BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA
+	bool "use built-in timezone data"
+	help
+	  Build using libical's (instead of the system's) timezone data.
+
+	  libical requires either built-in or system timezone data in order
+	  to support time zone functions.
+
+endif
+
 comment "libical needs a toolchain w/ C++, dynamic library, wchar"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
 		BR2_STATIC_LIBS
package/libical/libical.mk +0 -0
index e8cb3f8..a8bcefb 100644

package/libical/libical.mk LIBICAL_LICENSE_FILES = LICENSE · line 19

Quoted 6 lines
package/libical/libical.mk +3 -0
@@ -14,4 +14,8 @@ LIBICAL_LICENSE_FILES = LICENSE
 # Gentoo/alpinelinux as well
 LIBICAL_CONF_OPTS = -DSHARED_ONLY=true
 
+ifeq ($(BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA),y)
+LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
+endif

Why not detect this automatically? Something like (untested):

ifeq ($(BR2_PACKAGE_TZDATA),)
LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
endif

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
#2 lore raw

PATCH 1/1 libical: Create option to use builtin tzdata

Marc, Baruch, All,

On 2016-08-09 18:55 +0300, Baruch Siach spake thusly:

Quoted 52 lines · Marc Khouri
Quoted 42 lines

libical requires time zone data (either system or internal) in order to use time zone related functions. This commit adds an option to build libical with its internal time zone data. If no time zone data is present, segfaults occur in applications which use time zone functions.

Option documentation:
https://github.com/libical/libical/blob/v1.0.1/CMakeLists.txt#L23

---
 package/libical/Config.in  | 12 ++++++++++++
 package/libical/libical.mk |  4 ++++
 2 files changed, 16 insertions(+)
package/libical/Config.in +12 -0
index 48170e9..e183ad0 100644
@@ -9,6 +9,18 @@ config BR2_PACKAGE_LIBICAL
 
 	  http://libical.github.io/libical
 
+if BR2_PACKAGE_LIBICAL
+
+config BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA
+	bool "use built-in timezone data"
+	help
+	  Build using libical's (instead of the system's) timezone data.
+
+	  libical requires either built-in or system timezone data in order
+	  to support time zone functions.
+
+endif
+
 comment "libical needs a toolchain w/ C++, dynamic library, wchar"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
 		BR2_STATIC_LIBS
package/libical/libical.mk +0 -0
index e8cb3f8..a8bcefb 100644

package/libical/libical.mk LIBICAL_LICENSE_FILES = LICENSE · line 19

Quoted 6 lines
package/libical/libical.mk +3 -0
@@ -14,4 +14,8 @@ LIBICAL_LICENSE_FILES = LICENSE
 # Gentoo/alpinelinux as well
 LIBICAL_CONF_OPTS = -DSHARED_ONLY=true
 
+ifeq ($(BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA),y)
+LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
+endif

Why not detect this automatically? Something like (untested):

Quoted 3 lines

ifeq ($(BR2_PACKAGE_TZDATA),)
LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
endif

I am not too fond of using bundle stuff when we already have the required support for it. So, if libical needs TZ info, forcibly enable it (or depend on it).

The only gotcha is that we don;t install TZ for musl. However, from the look of it, it should be ready to support tzdata, so our current limitation is probably just incorrect. I'm trying to confirm that.

In the meantime, you should select BR2_TARGET_TZ_INFO from the libical Config.in, but only if not musl. And in the .mk, use the suggestion by Baruch.

So:

    Config.in:

        config BR2_PACKAGE_LIBICAL
            bool "libical"
            depends on blabla
            select BR2_TARGET_TZ_INFO if BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_UCLIBC

    libical.mk:

        ifeq ($(BR2_TARGET_TZ_INFO),)
        LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
        endif

Which means that on glibc or uClibc, we'd rely on the system-wide tzdata, but on musl we'd use the bundled one.

Regards,
Yann E. MORIN.

Quoted 10 lines

baruch

Signature · 7 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

PATCH 1/1 libical: Create option to use builtin tzdata

Hello,

Quoted 9 lines · Yann E. MORIN
Quoted 5 lines

Why not detect this automatically? Something like (untested):

ifeq ($(BR2_PACKAGE_TZDATA),)
LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
endif

I am not too fond of using bundle stuff when we already have the required support for it. So, if libical needs TZ info, forcibly enable it (or depend on it).

Agreed.

Quoted 3 lines

The only gotcha is that we don;t install TZ for musl. However, from the look of it, it should be ready to support tzdata, so our current limitation is probably just incorrect. I'm trying to confirm that.

Yann has submitted a patch that adds TZ support for musl, and I already merged it. So all what needs to be done is to "select BR2_TARGET_TZ_INFO" in libical. Marc, could you submit such a patch?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#4 lore raw

PATCH 1/1 libical: Create option to use builtin tzdata

In reply to Marc Khouri

Marc, All,

On 2016-08-09 10:02 -0400, Marc Khouri spake thusly:

Quoted 23 lines

libical requires time zone data (either system or internal) in order to use time zone related functions. This commit adds an option to build libical with its internal time zone data. If no time zone data is present, segfaults occur in applications which use time zone functions.

Option documentation:
https://github.com/libical/libical/blob/v1.0.1/CMakeLists.txt#L23

---
 package/libical/Config.in  | 12 ++++++++++++
 package/libical/libical.mk |  4 ++++
 2 files changed, 16 insertions(+)
package/libical/Config.in +1 -0
index 48170e9..e183ad0 100644
@@ -9,6 +9,18 @@ config BR2_PACKAGE_LIBICAL
 
 	  http://libical.github.io/libical
 
+if BR2_PACKAGE_LIBICAL

package/libical/Config.in config BR2_PACKAGE_LIBICAL · line 22

Quoted 10 lines
package/libical/Config.in +10 -0
+
+config BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA
+	bool "use built-in timezone data"
+	help
+	  Build using libical's (instead of the system's) timezone data.
+
+	  libical requires either built-in or system timezone data in order
+	  to support time zone functions.
+
+endif

As already discussed in other replies, we believe it is better to rely on the existing TZ info that can be used system-wide.

A ver recent commit makes this available for all types of toolchains, not just glibc or uClibc.

So, your change can now easily be changed to use:

config BR2_PACKAGE_LIBICAL
bool "libical"
depends on exiting-blabla
select BR2_TARGET_TZ_INFO

Quoted 8 lines
package/libical/Config.in +0 -0
 comment "libical needs a toolchain w/ C++, dynamic library, wchar"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
 		BR2_STATIC_LIBS
package/libical/libical.mk +0 -0
index e8cb3f8..a8bcefb 100644

package/libical/libical.mk LIBICAL_LICENSE_FILES = LICENSE · line 19

Quoted 6 lines
package/libical/libical.mk +3 -0
@@ -14,4 +14,8 @@ LIBICAL_LICENSE_FILES = LICENSE
 # Gentoo/alpinelinux as well
 LIBICAL_CONF_OPTS = -DSHARED_ONLY=true
 
+ifeq ($(BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA),y)
+LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
+endif

And of course you can now drop this change, although I would still keep it as:

    LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=OFF

to be sure it does not get used.

Thanks!

Regards,
Yann E. MORIN.

Quoted 8 lines
package/libical/libical.mk +0 -0
 $(eval $(cmake-package))
Signature · 6 lines
2.8.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

PATCH 1/1 libical: Create option to use builtin tzdata

Yann, Thomas, Baruch,

Thank you for the feedback. I will submit a modified patch tomorrow.

Regards,
Marc

Quoted 99 lines · On Tue, Aug 9, 2016, 16:56 Yann E. MORIN <[email protected]> wrote:

Marc, All,

On 2016-08-09 10:02 -0400, Marc Khouri spake thusly:

Quoted 4 lines

libical requires time zone data (either system or internal) in order to use time zone related functions. This commit adds an option to build libical with its internal time zone data. If no time zone data is present, segfaults occur in applications which use time zone

functions.

Quoted 19 lines

Option documentation:
https://github.com/libical/libical/blob/v1.0.1/CMakeLists.txt#L23

---
 package/libical/Config.in  | 12 ++++++++++++
 package/libical/libical.mk |  4 ++++
 2 files changed, 16 insertions(+)
package/libical/Config.in +1 -0
index 48170e9..e183ad0 100644
@@ -9,6 +9,18 @@ config BR2_PACKAGE_LIBICAL
 
        http://libical.github.io/libical
 
+if BR2_PACKAGE_LIBICAL

package/libical/Config.in config BR2_PACKAGE_LIBICAL · line 22

Quoted 10 lines
package/libical/Config.in +10 -0
+
+config BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA
+     bool "use built-in timezone data"
+     help
+       Build using libical's (instead of the system's) timezone data.
+
+       libical requires either built-in or system timezone data in order
+       to support time zone functions.
+
+endif

As already discussed in other replies, we believe it is better to rely on the existing TZ info that can be used system-wide.

A ver recent commit makes this available for all types of toolchains, not just glibc or uClibc.

So, your change can now easily be changed to use:

config BR2_PACKAGE_LIBICAL
bool "libical"
depends on exiting-blabla
select BR2_TARGET_TZ_INFO

Quoted 8 lines
package/libical/Config.in +0 -0
 comment "libical needs a toolchain w/ C++, dynamic library, wchar"
      depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
              BR2_STATIC_LIBS
package/libical/libical.mk +0 -0
index e8cb3f8..a8bcefb 100644

package/libical/libical.mk LIBICAL_LICENSE_FILES = LICENSE · line 19

Quoted 6 lines
package/libical/libical.mk +3 -0
@@ -14,4 +14,8 @@ LIBICAL_LICENSE_FILES = LICENSE
 # Gentoo/alpinelinux as well
 LIBICAL_CONF_OPTS = -DSHARED_ONLY=true
 
+ifeq ($(BR2_PACKAGE_LIBICAL_BUILTIN_TZDATA),y)
+LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=true
+endif

And of course you can now drop this change, although I would still keep it as:

    LIBICAL_CONF_OPTS += -DUSE_BUILTIN_TZDATA=OFF

to be sure it does not get used.

Thanks!

Regards,
Yann E. MORIN.

Quoted 8 lines
package/libical/libical.mk +0 -1
 $(eval $(cmake-package))
--

2.8.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

--

.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
     |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is
no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
 conspiracy.  |
Quoted 2 lines

'------------------------------^-------^------------------^--------------------'

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160810/45c546f9/attachment.html>