On Mon, 21 Sep 2020 14:17:18 GMT, Ian Graves <igra...@openjdk.org> wrote:
>> test/jdk/tools/jlink/JLinkReproducible3Test.java line 112: >> >>> 110: } >>> 111: } >>> 112: >> >> The update to ImageFileCreator looks good. >> >> A few nits in JLinkReproducible3Test: >> 1. Invoking copyJDKs with two destination locations looks a bit strange. I >> think would be easier for future maintainers >> if it does one copy and is called twice. 2. Files.copy will create a >> directory, no need for Files.createDirectories. >> 3. Are you sure that bin/jlink is executable, I just wonder if the test runs >> with the patch have ended up depending the >> umask at the time. It is possible to specify COPY_ATTRIBUTES to the copy >> method and it will attempt to copy the >> permissions. 4. The variable naming is more like C style, so a bit >> inconsistent with the naming usually used in Java >> programs. > > 1. I agree that it's awkward and I'm not a huge fan. I'm okay doing it this > way, but this is recreating the reproducer > for https://bugs.openjdk.java.net/browse/JDK-8252730 which is two identical > JDK's in different directories. I can > document this in the test further or we can change it. 2-4: Noted. Will > clean these up and loop back. Thanks! Whatever is easiest but 2 x copy tree would be simpler and the result should be two identical JDKs as reported in JDK-8252730. ------------- PR: https://git.openjdk.java.net/jdk/pull/156
