On Thu, 6 Mar 2025 13:53:31 GMT, Antonio Vieiro <d...@openjdk.org> wrote:

>> Replace hardcoded instances of `-lpthread` with `$(LIBPTHREAD)`, so that 
>> it's possible to parameterize this for platforms that use different flags 
>> for enabling posix threads.
>> 
>> This work is a continuation of the work done by Greg Lewis in [1], but 
>> generalized for the full JDK, and set at the configure stage.
>> 
>> Sponsored by: The FreeBSD Foundation
>> Co-authored-by: Greg Lewis <gle...@eyesbeyond.com>
>> 
>> [1]: 
>> https://github.com/battleblow/jdk23u/commit/dbd90aa8ab0b7f5e4865864a7c63d975daacabf4
>
> make/test/JtregNativeHotspot.gmk line 886:
> 
>> 884:   BUILD_HOTSPOT_JTREG_LIBRARIES_JDK_LIBS_libnativeStack := 
>> java.base:libjvm
>> 885: else
>> 886:   BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libbootclssearch_agent += 
>> $(LIBPTREAD)
> 
> Hi. Should this read `$(LIBPTHREAD)` instead (i.e., missing `H`)? 
> Could be me too, I need new reading glasses...

You're absolutely right. 

@snake66 Making changes to makefiles is tricky, since a misspelled variable do 
not get any warning but is just silently ignored. 

For a change like this, that is a pure refactoring that is not supposed to 
change the output, I highly recommend using the `COMPARE_BUILD` system. 
Unfortunately, this is severely underdocumented. There is a short paragraph at 
https://openjdk.org/groups/build/doc/building.html under "Developing the Build 
System Itself", but in short, what I'd do is to create a diff files of these 
changes compared to a baseline (e.g. master, a specific build or commit), make 
sure you have the baseline checked out, and then run `make jdk-image test-image 
COMPARE_BUILD=PATCH=my_patch.diff`. This will build the targets twice, one 
without the patch and one with the patch, and then automatically run the 
`compare` script to analyze any differences. For this particular patch, there 
should be none. This would likely have caught this typo. (Given that the 
test-image is actually compared; I suddenly got a bit uncertain about that...)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23930#discussion_r1983618250

Reply via email to