[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[Christopher Baines]

Replace the hardcoded /usr/share/zoneinfo file with a reference to the tzdata
package.

* gnu/packages/icu4c.scm (icu4c)[inputs]: Add tzdata.
[arguments]: Add a patch-zoneinfo-directory phase.
---
 gnu/packages/icu4c.scm | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
index 57e8fffb3..d34b61745 100644
--- a/gnu/packages/icu4c.scm
+++ b/gnu/packages/icu4c.scm
@@ -22,6 +22,7 @@
 
 (define-module (gnu packages icu4c)
   #:use-module (gnu packages)
+  #:use-module (gnu packages base)
   #:use-module (gnu packages perl)
   #:use-module (guix licenses)
   #:use-module (guix packages)
@@ -45,7 +46,8 @@
              (base32 "065l3n0q9wqaw8dz20x82srshhm6i987fr9ync5xf9mr6n7ylwzh"))))
    (build-system gnu-build-system)
    (inputs
-    `(("perl" ,perl)))
+    `(("perl" ,perl)
+      ("tzdata" ,tzdata)))
    (arguments
     `(#:configure-flags
       '("--enable-rpath"
@@ -58,7 +60,13 @@
       #:phases
       (modify-phases %standard-phases
         (add-after 'unpack 'chdir-to-source
-          (lambda _ (chdir "source") #t)))))
+          (lambda _ (chdir "source") #t))
+        (add-after 'chdir-to-source 'patch-zoneinfo-directory
+          (lambda* (#:key inputs #:allow-other-keys)
+            (substitute* "common/putil.cpp"
+              (("\\/usr\\/share\\/zoneinfo")
+               (string-append (assoc-ref inputs "tzdata")
+                              "/share/zoneinfo"))))))))
    (synopsis "International Components for Unicode")
    (description
     "ICU is a set of C/C++ and Java libraries providing Unicode and
-- 
2.16.3

[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[Ludovic Courtès]

Hello,

Christopher Baines <mail@cbaines.net> skribis:

> Replace the hardcoded /usr/share/zoneinfo file with a reference to the tzdata
> package.
>
> * gnu/packages/icu4c.scm (icu4c)[inputs]: Add tzdata.
> [arguments]: Add a patch-zoneinfo-directory phase.

Good catch.

I wonder if icu4c should instead depend on ‘tzdata-for-tests’, which is
changed less frequently to avoid full rebuilds?  Maybe Leo can comment.

I hope icu4c also honors the TZDIR environment variable, which would
allow users to have it refer to the latest version.  Do you know if
that’s the case?

Last thing: this would probably be for ‘core-updates-next’.

Thanks,
Ludo’.

[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

On Sat, May 19, 2018 at 10:21:42PM +0200, Ludovic Courtès wrote:
> Hello,
> 
> Christopher Baines <mail@cbaines.net> skribis:
> 
> > Replace the hardcoded /usr/share/zoneinfo file with a reference to the tzdata
> > package.
> >
> > * gnu/packages/icu4c.scm (icu4c)[inputs]: Add tzdata.
> > [arguments]: Add a patch-zoneinfo-directory phase.

Thanks for noticing this, Chris.

> I wonder if icu4c should instead depend on ‘tzdata-for-tests’, which is
> changed less frequently to avoid full rebuilds?  Maybe Leo can comment.

tzdata-for-tests is meant to be used by packages that don't need current
time zone info. Thus, "for test suites only". We added tzdata-for-tests
to make it cheaper to update tzdata.

If icu4c needs current time zone info, it should not use
tzdata-for-tests. Chris, can you give more context about how icu4c uses
time zone information?

> I hope icu4c also honors the TZDIR environment variable, which would
> allow users to have it refer to the latest version.  Do you know if
> that’s the case?

Yeah, it would be great if icu4c could just use TZDIR, which would allow
us to continue updating the time zones relatively frequently... if icu4c
actually needs the time zone database.

Already, we have problems keeping them up to date because too many
packages depend on them, and that's a problem for Guix users (clocks
will be wrong in some time zones). If we let icu4c depend on tzdata,
then we will only be able to update the time zones during core-updates,
which is not frequent enough. Currently, we can at least update tzdata
on 'staging' braches.

It indicates two general problems, IMO:

1) the lack of build farm capacity
2) it should be possible to update the time zone database without
rebuilding packages (the packages should use TZDIR)

I wonder what is done in Nixpkgs...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[Ludovic Courtès]

Heya,

Leo Famulari <leo@famulari.name> skribis:

> It indicates two general problems, IMO:
>
> 1) the lack of build farm capacity
> 2) it should be possible to update the time zone database without
> rebuilding packages (the packages should use TZDIR)

I think tzdata is an example where dynamic composition (i.e., TZDIR) is
what we should use.  That way we can update tzdata as frequently as we
want.

For the case where TZDIR is unset, programs can still use
tzdata-for-tests, which could be slightly outdated, but I consider it a
“degraded” mode and I think it’s fine.

> I wonder what is done in Nixpkgs...

It may be that the majority of their users don’t live in the regions
affected by recent timezone changes…

Ludo’.

[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Tue, May 22, 2018 at 02:06:53PM +0200, Ludovic Courtès wrote:
> For the case where TZDIR is unset, programs can still use
> tzdata-for-tests, which could be slightly outdated, but I consider it a
> “degraded” mode and I think it’s fine.

Okay, I'll rename it tzdata-old in core-updates to better describe the
full range of use cases.

> > I wonder what is done in Nixpkgs...
> 
> It may be that the majority of their users don’t live in the regions
> affected by recent timezone changes…

Yes, and these last minute changes tend not to affect the wealthier
parts of the world, where such problems would have a greater impact.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#31484] [PATCH] gnu: icu4c: Patch zoneinfo directory.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> Replace the hardcoded /usr/share/zoneinfo file with a reference to the tzdata
>> package.
>>
>> * gnu/packages/icu4c.scm (icu4c)[inputs]: Add tzdata.
>> [arguments]: Add a patch-zoneinfo-directory phase.
>
> Good catch.
>
> I wonder if icu4c should instead depend on ‘tzdata-for-tests’, which is
> changed less frequently to avoid full rebuilds?  Maybe Leo can comment.
>
> I hope icu4c also honors the TZDIR environment variable, which would
> allow users to have it refer to the latest version.  Do you know if
> that’s the case?

All I remember about this was that I don't think it fixed the problem I
was trying to solve :) I had the patch in a stash for a while before
submitting it.

I don't even know what icu4c actually does, so I'm not very well placed
to look at this further, but I'm glad you and Leo seem to be getting
something from it :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]