icu @49.1.2_0 Patch to allow successful build on Tiger

[ccarey@…]

The icu @49.1.2_0 port does not successfully build on Tiger because of

:info:build /usr/bin/g++-4.0 -DU_ATTRIBUTE_DEPRECATED= -DU_COMMON_IMPLEMENTATION    -DU_HAVE_TIMEZONE=0  -I.   "-DDEFAULT_ICU_PLUGINS=\"/opt/local/lib/icu\" " -pipe -O2 -arch ppc -O2 -W -Wall -ansi -pedantic -Wpointer-arith -Wwrite-strings -Wno-long-long   -fno-common -c   -o putil.ao putil.cpp
:info:build putil.cpp: In function ‘const char* uprv_tzname_49(int)’:
:info:build putil.cpp:1083: error: ‘localtime_r’ was not declared in this scope
:info:build putil.cpp: At global scope:
:info:build putil.cpp:1450: warning: ‘const char* age()’ defined but not useddepp
:info:build gnumake[1]: *** [putil.ao] Error 1

The attached patch fixes the problem by providing a “backup” declaration of localtime_r() for Darwin. Note that the platform-specific #defines within ICU only distinguish Darwin from other operating systems; they don’t distinguish between Tiger, Leopard, Snow Leopard, &c. This patch works for Tiger, but could theoretically cause a problem for other versions of Mac OS X. I don’t have access to any other Mac OS X version than Tiger, so please ensure that this patch causes no problem with Leopard and beyond before integrating this patch.

[ccarey@…]

patch to source/common/putil.cpp to allow icu @49.1.2_0 to build successfully on Tiger

[ryandesign (Ryan Carsten Schmidt)]

Replying to ccarey@…:

The icu @49.1.2_0 port does not successfully build on Tiger because of

Have you already reported this problem to the developers of icu? If so please post a URL to the bug report or mailing list post, if possible.

[ryandesign (Ryan Carsten Schmidt)]

Cc Me!

[ultrajoe@…]

For the record, this also holds true building with apple-gcc-4.2.

I'll try the patch.

[ccarey@…]

Replying to ryandesign@…:

Have you already reported this problem to the developers of icu?

No, I haven’t reported it to them yet. I thought that if this patch didn’t cause any problems with Leopard and beyond within MacPorts, that the ICU developers would accept it more readily.

[ultrajoe@…]

"Cleaned up" version useful for average user usage

[drkp (Dan Ports)]

This sounds like the same issue as #35452 (for texlive-bin's version of icu). I wasn't able to make any progress on that one for lack of a Tiger machine.

[ccarey@…]

Replying to dports@…:

This sounds like the same issue as #35452 (for texlive-bin's version of icu).

Yes, it is the same issue. I’ve added a texlive-bin-specific patch to #35452.

[ned-deily (Ned Deily)]

Cc Me!

[ballapete (Peter "Pete" Dyballa)]

Six weeks after the patch to fix this version of icu I still have:

pete 294 /\ l -R /opt/local/var/macports/sources/lil.fr.rsync.macports.org/release/tarballs/ports/devel/icu
/opt/local/var/macports/sources/lil.fr.rsync.macports.org/release/tarballs/ports/devel/icu:
insgesamt 8
-rw-r--r-- 1 root wheel 7720  3. Okt 04:00 Portfile
drwxr-xr-x 5 root wheel  170  3. Okt 04:00 files

/opt/local/var/macports/sources/lil.fr.rsync.macports.org/release/tarballs/ports/devel/icu/files:
insgesamt 12
-rw-r--r-- 1 root wheel  739 21. Sep 22:23 patch-config-mh-darwin.diff
-rw-r--r-- 1 root wheel 1524  3. Okt 04:00 patch-configure.diff
-rw-r--r-- 1 root wheel 1663  3. Okt 04:00 patch-universal.diff

Do I have to manually apply that patch?

[ballapete (Peter "Pete" Dyballa)]

Cc Me!

[ballapete (Peter "Pete" Dyballa)]

Manually applying the patch file putil.cpp.2.diff​ allowed to build icu.

[ccarey@…]

Replying to Peter_Dyballa@…:

Do I have to manually apply that patch?

If you’re compiling on Tiger, then yes, at this point you still have to manually apply this patch.

As noted in this ticket’s description, this patch has only been tried on Tiger — at least by me, since it is the only Mac system to which I have access — but the nature of the patch means that it would be applied to all Mac OS X releases, not just Tiger. If this patch allows successful compilation on releases other than Tiger, then it would be OK to use with those releases. [On those other releases, this patch only adds an extra declaration for the localtime_r () function, so their runtime behavior wouldn’t be affected if the patched source compiles successfully]. Once this patch has been demonstrated to compile successfully against Leopard and beyond, then it could be integrated into icu.

[ballapete (Peter "Pete" Dyballa)]

On PPC Leopard (10.5.8) and on intel Snow Leopard (10.6.8) the putil.cpp.2.diff​ patch was not needed to build icu @49.1.2. On these two platforms it built instantly.

[ryandesign (Ryan Carsten Schmidt)]

Replying to ccarey@…:

Replying to ryandesign@…:

Have you already reported this problem to the developers of icu?

No, I haven’t reported it to them yet. I thought that if this patch didn’t cause any problems with Leopard and beyond within MacPorts, that the ICU developers would accept it more readily.

When I'm looking for patches to commit, I usually see it the other way around: I'm more likely to commit a patch if the developers have already signed off on it. I did apply it locally and verify that it still builds on Mountain Lion, but since I'm not a C programmer and I'm not familiar with the icu source code, I don't feel totally confident committing these changes. Could you report the problem and your patch to the developers so that we can get their input?

[ccarey@…]

Replying to Peter_Dyballa@…:

On PPC Leopard (10.5.8) and on intel Snow Leopard (10.6.8) the putil.cpp.2.diff​ patch was not needed to build icu @49.1.2. On these two platforms it built instantly.

Yes, but the point is that this patch affects compilation on Leopard, Snow Leopard, Lion, and Mountain Lion as well as compilation on Tiger. Ideally, this patch should make no difference on Leopard and beyond, but since I only have access to a Tiger system, I can’t guarantee that that will be the case — which is why I’d asked for the patch to also be compiled on those systems, to ensure that this patch would have no effect on them, before integration of this patch into icu.

[ccarey@…]

Replying to ryandesign@…:

Replying to ccarey@…:

Replying to ryandesign@…:

Have you already reported this problem to the developers of icu?

No, I haven’t reported it to them yet. I thought that if this patch didn’t cause any problems with Leopard and beyond within MacPorts, that the ICU developers would accept it more readily.

When I'm looking for patches to commit, I usually see it the other way around: I'm more likely to commit a patch if the developers have already signed off on it. I did apply it locally and verify that it still builds on Mountain Lion, but since I'm not a C programmer and I'm not familiar with the icu source code, I don't feel totally confident committing these changes. Could you report the problem and your patch to the developers so that we can get their input?

This problem had already been reported by someone else as ​ICU ticket 9367. I’ve added a comment to that ticket directing them here to this patch. Considering the number of outstanding tickets there (as well as the age of many of them), I don’t know what the likelihood might be of getting their input on this patch in a timely fashion.

[ryandesign (Ryan Carsten Schmidt)]

To avoid any surprises I've committed the patch in r100308 for Tiger only. Upstream can take it from here.