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



Reply via email to