Hello,
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.
/Erik
/Magnus
David
Bug: https://bugs.openjdk.java.net/browse/JDK-8187642
Webrev: http://cr.openjdk.java.net/~erikj/8187642/webrev.01/
/Erik