RFR: JDK-8241602 jlink does not produce reproducible jimage files

[Jim Laskey]

Thank you. Notes below.


updated webrev: http://cr.openjdk.java.net/~jlaskey/8241602/webrev-00 <http://cr.openjdk.java.net/~jlaskey/8241602/webrev-00>


> On May 6, 2020, at 4:14 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 05/05/2020 20:56, Jim Laskey wrote:
>> This fix addresses the inconsistent ordering by jimage content by jlink from run to run. Bottom line, the implementer was using HashSet without defining hashcode/equals for the Set entry classes.
>> 
>> webrev: http://cr.openjdk.java.net/~jlaskey/8241602/webrev-00 <http://cr.openjdk.java.net/~jlaskey/8241602/webrev-00>
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8241602 <https://bugs.openjdk.java.net/browse/JDK-8241602>
>> 
> DirArchive and JmodArchive look okay. They could use Objects.hash/equals but what you have is okay too. Can you re-check JarArchive as it is created with a runtime version so equals should be checking 3 fields.

Opted to use Objects. Added version for JarArchive (not convinced needed but should be benign.)

> 
> The existing test for reproducible builds is JLinkReproducibleTest. Adding a new test is okay too but we should at least try to keep the names consistent. A couple of suggestions for the test in the webrev:
> - the @modules tag needs to include java.desktop as it is needed by the test. Better still would be to change to java.se so that there are more modules in the images.

Switched to  java.se, only a second slower than java.desktop . Overall better test than JLinkReproducibleTest for increased likelihood of variance.

> - Did you mean to open jdk.tools.jlink.internal? Maybe its a leftover from a previous iteration of the test?

Nope, Yep.

> - You can use Files.mismatch to compare the lib/modules files are identical (like JLinkReproducibleTest).  It's okay to use the jimage ImageReaderFactory to check the names tables too but I think it's more important to check that the lib/modules files are identical before probing further.

I had it in my mind there was a timestamp in the file, but then I remembered there wasn't. Switched to Files.mismatch. Everything else is redundant.

Aside: The order in the file is still somewhat scattered, based on archive and archive content.  We have the a pattern based sorting plugin which we don't use and we never did any locality vs performance testing.

> 
> -Alan