Omair,
I'm sorry I did not see this before. I probably would not have noticed
it except I happened to notice this reply.
I'm responsible for maintaining the version of ICU in JDK, and also
(read: main day job) IBM's lead for ICU for C/C++. ( But I speak for
myself here, of course. )
I think this patch is certainly a good direction (especially with
regards HarfBuzz - I wrote that recommendation you cite), at least
longer term, however, I have quite a number of concerns with this patch.
• The big one - is that the ICU and JDK versions of the LE are
divergent, so much so that, to put it mildly, I am a little surprised
that this patch builds at all.
Now, I would like - love - for enough upstream changes to ICU to
happen so that the APIs are compatible enough for this change to be
possible. But we aren't there yet.
• … and when we are there, it won't be compatible with any shipping
ICU, so you will need to wait until the future ICU is picked up.
• You mentioned the issues about which version of the headers to compile
against. You MUST compile against the same version of the headers you
are linking against. Period. This is C++, not Java or even C. The
vtable expected in the ICU version versus the JDK version of the LE are
different, have different parentage.
• As to tests, I would start with everything in
jdk/test/java/awt/font/TextLayout
( NB Phil- I had started a port of ICU's letest.xml conformance test
to the JDK- this would be helpful in making sure we get the same results.)
I'm sorry I didn't see this earlier. I don't always monitor this address
or even 2d-dev as closely as I could.
Regards,
Steven
( If you don't get a response from me, you can always ping me at
s...@icu-project.org … )
On 6/24/13 3:14 p.m., Omair Majid wrote:
Hi Phil,
Updated webrev:
http://cr.openjdk.java.net/~omajid/webrevs/system-icu/01/
It's still against jdk8/build and missing support for the old build system.
On 06/05/2013 02:02 PM, Phil Race wrote:
Since this entirely affects a 2D component, please include 2d-dev in
this discussion.
I would have been 'surprised' to see this change if I hadn't just
spotted this thread.
Mea culpa. I pushed a few similar system-library patches using this
identical process and no one corrected me. So I assumed this was right.
For the future, I will ensure the relevant groups are CC'd.
And I believe this change should be integrated via the 2D forest.
Okay. Are there any guidelines for this? It's not obvious to me when a
change is more appropriate for build vs 2D.
I am not sure what, if any, runtime problems might occur from using a
different
version. We've generally been able to swap in newer versions of ICU without
much trouble, but I recommend to run appropriate tests before shipping ..
Thanks for the suggestions. Do you have any particular tests in mind?
For some background, the goal with this change is two fold:
1. Gain benefits from system-installed libraries. This is one library
where OpenJDK does not lag behind in security updates, but in some other
cases, embedded libraries can lag behind. It also makes it easier to use
the distributions preferred policies (hardening flags on libraries and
so on).
2. Make it easier to switch to HarfBuzz at some later point. ICU itself
strongly recommends switching [1]. Through ice-le-hb [2], only a
recompile should be needed to switch (hopefully).
Note that JDK8 in fact has a very "current" ICU with some security fixes.
So I would not recommend using the native lib on a system with an older
ICU.
Thanks for pointing it out. I see that ICU recently released a security
update [1] too, but probably many distributions will not have this
up-to-date version (my current distro, a little out-of-date, does not :( ).
Thanks,
Omair
[1] http://site.icu-project.org/download/51
[2] https://github.com/behdad/icu-le-hb
