On Tue, 27 May 2025 23:57:03 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> This is a redo of [JDK-8357048](https://bugs.openjdk.org/browse/JDK-8357048), > which had to backed out since it caused testing errors in higher tiers. > > The problem was that `JTREG_PROBLEM_LIST_PREFIX` was not defined before it > was used, and when `JTREG_BASIC_OPTIONS` were no longer implicitly declared > as a macro, but instead got a definite assignment, the value of > JTREG_PROBLEM_LIST_PREFIX was empty at the time of evaluation. > > I have now manually checked each and every `+=` assignment to > `$1_JTREG_BASIC_OPTIONS`, and verified that all variables present is defined > earlier. > > Here is the original description from JBS: > > When building `$1_JTREG_BASIC_OPTIONS`, it is assumed that the variable is > recursively defined and that thus `+=` is lazy. > > > $1_JTREG_BASIC_OPTIONS += -$$($1_JTREG_TEST_MODE) \ > -verbose:$$(JTREG_VERBOSE) -retain:$$(JTREG_RETAIN) \ > -concurrency:$$($1_JTREG_JOBS) > -timeoutFactor:$$(JTREG_TIMEOUT_FACTOR) \ > -vmoption:-XX:MaxRAMPercentage=$$($1_JTREG_MAX_RAM_PERCENTAGE) \ > -vmoption:-Dtest.boot.jdk="$$(BOOT_JDK)" \ > -vmoption:-Djava.io.tmpdir="$$($1_TEST_TMP_DIR)" > ``` > > If `+=` is eagerly evaluated, the option -timeoutFactor: will get an empty > argument and fail. > > The problem is the line: `$$(eval $$(call > SetJtregValue,$1,JTREG_BASIC_OPTIONS))` might create the variable > `$1_JTREG_BASIC_OPTIONS` "simply expanded" (the expansion will create an > assignment using `:=`). Whereas if the variable is not created the first `+=` > will be recursive and will work as expected. > > One solution to this problem is replacing the three assignments in > `SetJtregValue` from `:=` to `=`. This might have other side effects. > > A more conservative solution might be to create another macro (thus not > changing behaviour where strict evaluation might be needed): > > define SetJtregRecursiveValue > ifneq ($$($2), ) > $1_$2 = $$($2) > else > ifneq ($$($$($1_COMPONENT)_$2), ) > $1_$2 = $$($$($1_COMPONENT)_$2) > else > ifneq ($3, ) > $1_$2 = $3 > endif > endif > endif > endef This pull request has now been integrated. Changeset: a4f870df Author: Magnus Ihse Bursie <i...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/a4f870df553e4d7669edf6e454e147526ff2fae7 Stats: 18 lines in 1 file changed: 11 ins; 6 del; 1 mod 8357510: [REDO] RunTest variables should always be assigned Reviewed-by: erikj ------------- PR: https://git.openjdk.org/jdk/pull/25475
