RFR: 8326587: Separate out Microsoft toolchain linking [v4]
Magnus Ihse Bursie
ihse at openjdk.org
Tue Feb 27 11:08:48 UTC 2024
On Tue, 27 Feb 2024 05:20:11 GMT, Julian Waters <jwaters at openjdk.org> wrote:
>> 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
This was actually an important part of this PR, trying to combat the all too general solution we had when trying to combine microsoft and non-microsoft linking. On all Unix-style linkers, the option to mark the output file is `-o `. That is such a fundemental option that we do not want to make it general, especially not considering the additional difficulty that the corresponding option for the microsoft `link.exe` is to specify `/out:<filename>`, without a space. This forced us to define LD_OUT_OPTION as `-o$(SPACE)` on unix linkers, and it forced us to setup the linker command line with `$(LD_OUT_OPTION)$$($1_TARGET_RELOCATABLE)` with no space between.
That is in clear contrast with how we can add multiple sets of command line options like e.g. `(LDFLAGS_CXX_PARTIAL_LINKING) $$($1_SYSROOT_LDFLAGS)` where we can just stack them after each other with a space between.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17987#discussion_r1504049839
More information about the build-dev
mailing list