Updated in place.
/Erik
On 2016-02-10 14:16, Magnus Ihse Bursie wrote:
On 2016-02-10 14:11, Erik Joelsson wrote:
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.
Almost, at least. ;-)
You lost a colon:
- # Syntax: COMPARE_BUILD=CONF=<configure options>:PATCH=<patch file>:
+ # Syntax: COMPARE_BUILD=CONF=<configure options>:PATCH=<patch file>
If you just restore that colon, I'm fine with the new version.
/Magnus
/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
