RFR: JDK-8187642: The consolidated repo test makefile disables CONCURRENCY setting to jtreg
erik.joelsson at oracle.com
Tue Sep 19 15:31:01 UTC 2017
On 2017-09-19 03:33, Magnus Ihse Bursie wrote:
> On 2017-09-19 03:05, David Holmes wrote:
>> Hi Erik,
>> On 19/09/2017 10:53 AM, Erik Joelsson wrote:
>>> When invoking the test/Makefile using the jtreg_tests target in the
>>> consolidated repo, the CONCURRENCY variable gets overridden with an
>>> empty value. This was introduced with my changes to handle the new
>>> test roots. I believe the proper fix is to surround the override of
>>> CONCURRENCY with a conditional.
>> Why isn't this handled by the preceding block:
>> 51 ifeq ($(shell $(EXPR) $(JOBS) \> 50), 1)
>> 52 # JTReg cannot handle more than 50 in concurrency
>> 53 JDK_TEST_JOBS=50
>> 54 else
>> 55 JDK_TEST_JOBS=$(JOBS)
>> 56 endif
>> 57 else
>> 58 JDK_TEST_JOBS=$(TEST_JOBS)
>> 59 endif
> At this point, it's probably not worth the effort to clean up the
> logic in test/Makefile.
We were in quite a hurry getting this fix in before the first post
consolidation promotion so I missed your comment before pushing, David.
I agree in principle that the conditional should be made once instead of
inlined in four places, but since Magnus is working actively to replace
these makefiles in JDK 10, I don't think it's worth spending time trying
to clean up minor details when the whole thing is in such a messy state.
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8187642
>>> Webrev: http://cr.openjdk.java.net/~erikj/8187642/webrev.01/
More information about the build-dev