On 2018-09-12 19:33, naoto.s...@oracle.com wrote:
Hi Magnus, Erik,

Thank you for your comments. I updated the webrev according to your suggestions:

http://cr.openjdk.java.net/~naoto/8209167/webrev.02/
Looks good to me.

/Magnus


As to the duplicated "common" in the dependency, yes that's a separate issue. Since it's obvious, I included the fix with this changeset (it was actually described as a comment in the jira issue).

Naoto

On 9/12/18 9:08 AM, Erik Joelsson wrote:
On 2018-09-12 03:19, Magnus Ihse Bursie wrote:


On 2018-09-10 23:34, Naoto Sato wrote:
Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8209167

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8209167/webrev.01/

Some comments:
In make/copy/Copy-java.base.gmk:
+ifneq ($(findstring $(OPENJDK_TARGET_OS), aix),)

The findstring construct is hard to read and only needed when we test more than one OS. Please rewrite as a single ifeq test for aix.

In GensrcCLDR.gmk:
I don't understand the CLDR_WINTZMAPPINGS file? There's no rule to generate it, there's just a dependency..?

It's generated by the same rule as the other file. This is the easiest workaround for two files generated from the same rule. It sort of works (for clean builds and if the input is chagned), but won't handle all cases where one file is removed externally and the other is not. I suggested this construct to Naoto. Perhaps a comment explaining this would be good.
The removal of the duplicate "common", that seems like a separate bug fix?

It does indeed. Before this fix, the wildcards must have returned empty.

/Erik
/Magnus


This fix is to remove the hand crafted map file (lib/tzmappings) on Windows, which maps Windows time zones to Java's tzid. It is now dynamically generated at the build time, using CLDR's data file (windowsZones.xml)

Naoto



Reply via email to