Looks good.
/Erik
On 2018-09-12 10: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/
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
