RFR: JDK-8241616: Timestamps on ct.sym entries lead to non-reproducible builds

[Magnus Ihse Bursie]

On 2020-04-30 10:21, Jan Lahoda wrote:
> On 30. 04. 20 9:49, Magnus Ihse Bursie wrote:
>> On 2020-04-30 09:03, Jan Lahoda wrote:
>>> Hi,
>>>
>>> The building of lib/ct.sym is not reproducible, due to timestamps of 
>>> files inside the file (which is basically a zip file).
>>>
>>> The proposed solution is to use a constant timestamp for the files 
>>> inside the ct.sym file. To simplify the construction, the 
>>> CreateSymbols tool does not produce files on the filesystem anymore, 
>>> but rather constructs the ct.sym directly.
>> Thanks for helping us address reproducibility issues!
>>
>> If this truly makes ct.sym reproducible, you can include this patch 
>> as well to make the compare script test this:
>>
>> diff --git a/make/scripts/compare.sh b/make/scripts/compare.sh
>> --- a/make/scripts/compare.sh
>> +++ b/make/scripts/compare.sh
>> @@ -394,7 +394,7 @@
>>
>>       GENERAL_FILES=$(cd $THIS_DIR && $FIND . -type f ! -name "*.so" 
>> ! -name "*.jar" \
>>           ! -name "*.zip" ! -name "*.debuginfo" ! -name "*.dylib" ! 
>> -name "jexec" \
>> -        ! -name "modules" ! -name "ct.sym" ! -name "*.diz" ! -name 
>> "*.dll" \
>> +        ! -name "modules" ! -name "*.diz" ! -name "*.dll" \
>>           ! -name "*.cpl" ! -name "*.pdb" ! -name "*.exp" ! -name 
>> "*.ilk" \
>>           ! -name "*.lib" ! -name "*.war" ! -name "*.jmod" ! -name 
>> "*.exe" \
>>           ! -name "*.obj" ! -name "*.o" ! -name "jspawnhelper" ! 
>> -name "*.a" \
>
> Sure, will do.
>
>>
>>>
>>> Proposed webrev: http://cr.openjdk.java.net/~jlahoda/8241616/webrev.00/
>> The build aspect of the changes look good.
>>
>> I have a question that is not directly related to the changes you are 
>> doing in this fix. Why do we need to call two consecutive tools? (And 
>> I notice you changed the calling order in this patch). Do the first 
>> tool write a temporary file (where, if so?), and the second one makes 
>> the final ct.sym? Or does the first tool create ct.sym and the second 
>> one updates it?
>
> It is not completely necessary to have two tools, just 
> TransitiveDependencies take variable number of arguments; which is 
> less convenient to do into "CreateSymbols build-ctsym", but I think it 
> can be done. Let me look into that. (But it is definitely the first 
> variant - TransitiveDependencies builds system-modules file, which 
> CreateSymbols then incorporates into ct.sym).

Okay, if that's the case then everything is fine. You need to do no 
changes in the build tool. You can leave things as they are right now, 
and I can split it up in two SetupExecute calls instead later on.

I can't see how the output from TransitiveDependencies is specified. 
Maybe it's hard-coded? If anything, I'd appreciate that the input and 
output files from build tools are specified explicitly on the 
command-line. That makes is so much easier to follow what happens.

/Magnus

>
> Jan
>
>>
>> If it's the former case, it would be better to split the make rule 
>> into two: one where the target of the rule is the temporary file fir 
>> the first command, and one where the real ct.sym is the target, and 
>> the temporary file is a dependency.
>>
>> If it's the second case, I'd really like to have this fixed. Build 
>> tools that modify stuff in place is the worst. If the build e.g. 
>> breaks down in the second command, an incremental re-run will think 
>> the target is done, and not retry this command. Since both commands 
>> are classes in the same package, maybe they could be merged into a 
>> single call?
>>
>> If you believe a change like this would be out of scope for this bug, 
>> I understand.
>>
>> /Magnus
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8241616
>>>
>>> How does this look?
>>>
>>> Thanks,
>>>     Jan
>>