RFR: 8357510: [REDO] RunTest variables should always be assigned
Erik Joelsson
erikj at openjdk.org
Wed May 28 14:19:55 UTC 2025
On Tue, 27 May 2025 23:57:03 GMT, Magnus Ihse Bursie <ihse at 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
Marked as reviewed by erikj (Reviewer).
-------------
PR Review: https://git.openjdk.org/jdk/pull/25475#pullrequestreview-2875299813
More information about the build-dev
mailing list