Hello Tim,

New webrev: http://cr.openjdk.java.net/~erikj/8149479/webrev.02/

I agree about 1, so I started looking at it. Got confused why any changes to it didn't seem to have any effect until I realized the docs are no longer compared, which is what that was added for. So I fixed comparing of docs and reduced the sed expressions to what is actually necessary today, which was much less.

2 was indeed a typo and it could have hidden compare failures in my JPRT runs. Luckily, it was still clean.

I also fixed the comment Magnus pointed out.

/Erik

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

Looks good otherwise.

Tim


Reply via email to