On Tue, 27 Feb 2024 01:30:17 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

>> Magnus Ihse Bursie has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix indentation
>
> make/common/native/Link.gmk line 131:
> 
>> 129:             $(if $$($1_LINK_OBJS_RELATIVE), $$(CD) $$(OUTPUTDIR) ; ) \
>> 130:               $$($1_LD) $(LDFLAGS_CXX_PARTIAL_LINKING) 
>> $$($1_SYSROOT_LDFLAGS) \
>> 131:                 -o $$($1_TARGET_RELOCATABLE) \
> 
> I noticed this change that replaces `$(LD_OUT_OPTION)` with `-o` when 
> reviewing our integration changes. $1_LINK_OBJS_RELATIVE is currently only 
> supported on Linux/clang, it still seems good to not take away the 
> flexibility of specifying the non-linker specific option string here. Any 
> thoughts?

I tend to agree, this should not have been changed to specifying -o directly. 
We generally keep options inside Makefile variables rather than directly 
passing them like this, much like how $(OBJ_SUFFIX) was recently used to 
replace directly specifying the object file suffix in the make system

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17987#discussion_r1503652472

Reply via email to