On Tue, 17 Jan 2023 22:18:59 GMT, Erik Joelsson <er...@openjdk.org> wrote:

> Here is my attempt at solving Coleen's logging issue. This patch changes the 
> log level for the "build info" log messages for all native test libs and 
> executables to `LogInfo`. It also adds a new meta log message for each call 
> to SetupTestFilesCompilation, which is kept on LogWarn level, which prints a 
> single line with the number of test files being compiled in this call. For 
> `make test-image`, we have 5 such calls so the output will look like this, 
> which I think is quite reasonable:
> 
> 
> Creating 1 test executable file(s) for BUILD_LIBTEST_JTREG_EXECUTABLES
> Creating 45 test library file(s) for BUILD_JDK_JTREG_LIBRARIES
> Creating 6 test executable file(s) for BUILD_JDK_JTREG_EXECUTABLES
> Creating 853 test library file(s) for BUILD_HOTSPOT_JTREG_LIBRARIES
> Creating 16 test executable file(s) for BUILD_HOTSPOT_JTREG_EXECUTABLES

Other than that, it looks good. Thank you for fixing this!

make/common/TestFilesCompilation.gmk line 114:

> 112:     $$(eval $1 += $$(BUILD_TEST_$$(name)) ) \
> 113:     $$(eval $1_BUILD_INFO_DEPS += 
> $$(BUILD_TEST_$$(name)_BUILD_INFO_DEPS)) \
> 114:     $$(eval $$(BUILD_TEST_$$(name)_BUILD_INFO) :| $$($1_BUILD_INFO)) \

Suggestion:

    $$(eval $$(BUILD_TEST_$$(name)_BUILD_INFO): | $$($1_BUILD_INFO)) \


The formatting threw me off here, wondering what kind of operator `:|` were. We 
usually have no space between target name and `:`, and a space before `|` for 
order-only, so I recommend to keep that style here.

-------------

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12052

Reply via email to