RFR: JDK-8149479: Fix compare.sh to have a clean baseline with COMPARE_BUILD

[Magnus Ihse Bursie]

On 2016-02-10 07:13, Tim Bell wrote:
> Erik:
>
>> Please review these fixes for compare.sh and the COMPARE_BUILD flag 
>> for make.
>>
>> The rather new feature COMPARE_BUILD, which builds twice, applying 
>> some kind of change between them, is really neat. Especially when run 
>> through JPRT to check all platforms at once. The problem is that 
>> compare.sh currently isn't clean on all these platforms. There are 
>> also some special peculiarities particular to when JPRT is running 
>> the build which confuses compare.sh.
>>
>> Given that COMPARE_BUILD uses the exact same output directory, and 
>> JPRT sets the version-opt string to a fix value for both builds, we 
>> should be able to achieve a clean and stable baseline for an empty 
>> patch file. Which I now have.
>>
>> I also changed COMPARE_BUILD to fail the build when differences are 
>> found and added an option to COMPARE_BUILD to disable failing.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8149479
>> Webrev: http://cr.openjdk.java.net/~erikj/8149479/webrev.01/
>
> 1) Regarding common/bin/compare.sh lines 336...341 and lines 344...349 
> in the new version - this is not directly related to 8149479 but I 
> wish those regexes were only coded once.  If they ever get out of sync 
> the results will be puzzling.
>
> 2) make/InitSupport.gmk line 366 in the new version - typo? 
> COMPARE_BUILD_FAILE is COMPARE_BUILD_FAIL elsewhere

Also, the comment lists an additional "FAIL_BUILD=<bool>" which should 
be deleted.

Apart from that (and the typo Tim found), the patch looks good to me.

However, I'd like to use the opportunity to talk a bit about the compare 
script. :-)

I think it is a very good initiative to fix the compare script so that 
two consequitive builds on the same machine should compare equal. 
Apparently, that's as close to a reproducible build we'll ever get at 
the moment.

This patch does that, and that's good.

However, the "naive" way to get to a clean compare is to stop comparing 
anything of value. :-) I'm not suggesting that you are doing that (or 
that you should do that), but in a less extreme form, that's my main 
worry. Or to phrase it differently, how can we tell that we're not 
hiding something important? For instance, all these "accepted size 
differences" on solaris. Are they still relevant?

Is there some way to run the compare script without the exceptions file 
(apart from hacking it)? If we do that, what would happen? Could we trim 
down the exception lists?

The dis filters seems a bit non-optimal too. We have a "global" dis 
filter in compare.sh (introduced by me when I didn't know about the 
specific dis filters in the exception file), and a specific dis filter 
per platform in the exception file. The main task for the dis filters is 
to remove differences in hexadecimal numbers, and I think that could be 
generalized enough to just be in the global part. Any highly platform 
specific filtering, like the "literal pool" stuff for macosx, could 
still live in the exception file.

Finally, I still would like to get the string literal comparison into 
the configure script, now I run it manually using:
objdump -s -j .rodata $LIBRARY | grep "^ " | xxd -r | strings
(The main hurdle to do this is adding detection for xxd in configure, I 
think)

/Magnus