On Tue, 27 Feb 2024 05:20:11 GMT, Julian Waters <jwat...@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
