8345744: Use C++ LINK_TYPE with SetupBuildLauncher in StaticLibs.gmk

[jianglizhou]

Please review following fix for linking static JDK launcher, thanks.

• Set LINK_TYPE to C++ when building the static launcher to make sure LDCXX instead of LD is used for linking.

• Remove -l:libstdc++. For linux, -static-libstdc++ is already set in STATIC_STDCXX_FLAGS. See

  jdk/make/autoconf/lib-std.m4

  Line 50 in e0d6398

  STATIC_STDCXX_FLAGS="-static-libstdc++ -static-libgcc"

  .

See more details in https://bugs.openjdk.org/browse/JDK-8345744.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8345744: Use C++ LINK_TYPE with SetupBuildLauncher in StaticLibs.gmk (Bug - P4)

Reviewers

• Magnus Ihse Bursie (@magicus - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22624/head:pull/22624
$ git checkout pull/22624

Update a local copy of the PR:
$ git checkout pull/22624
$ git pull https://git.openjdk.org/jdk.git pull/22624/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22624

View PR using the GUI difftool:
$ git pr show -t 22624

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22624.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jiangli! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@jianglizhou This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8345744: Use C++ LINK_TYPE with SetupBuildLauncher in StaticLibs.gmk

Reviewed-by: ihse

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 20 new commits pushed to the master branch:

• fe0173d: 8345803: Update copyright year to 2024 for security in files where it was missed
• 2207749: 8344629: SSLSocketNoServerHelloClientShutdown test timeout
• 0b9e749: 8345390: [ubsan] systemDictionaryShared.cpp:964: member call on null pointer
• eff20a3: 8345794: Backout doc change introduced by JDK-8235786
• 48e22ba: 8345693: Update JCov for class file version 69
• cc628a1: 8345287: C2: live in computation is broken
• 480b508: 8345156: C2: Add bailouts next to a few asserts
• b120404: 8345793: Update copyright year to 2024 for the build system in files where it was missed
• 7aa0cbc: 8345614: Improve AnnotationFormatError message for duplicate annotation interfaces
• 35c0053: 8345405: Add JMH showing the regression in 8341649
• ... and 10 more: https://git.openjdk.org/jdk/compare/e0d639878346946d0627a57b0eeb0cac8ca533fc...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@jianglizhou The following label will be automatically applied to this pull request:

• build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (81e5c8b2)
• 00: Full (2012cbe7)

[magicus]

Thanks @jianglizhou, this was a much better solution. I don't know why I didn't realize that this was the proper solution when I wrote the code.

You can remove STANDARD_LIBS += -lstdc++ from macOS as well. This in turn means that STANDARD_LIBS is not used at all, and can be fully removed.

[jianglizhou]

Thanks @jianglizhou, this was a much better solution. I don't know why I didn't realize that this was the proper solution when I wrote the code.

You can remove STANDARD_LIBS += -lstdc++ from macOS as well. This in turn means that STANDARD_LIBS is not used at all, and can be fully removed.

llvm/llvm-project#76945 indicates -static-libstdc++ could be unused on macOS. According to https://docs.brew.sh/C++-Standard-Libraries#:~:text=There%20are%20two%20C++%20standard,you're%20trying%20to%20build., "The default for 10.9 and later is libc++, which is also the default for clang on older platforms when building C++11 code".

Let's not mix the Linux fix with macos and instead address them separately.

[magicus]

I verified locally that removing -lstdc++ works fine.

In fact, you are already changing macOS, since the LINK_TYPE affects all platforms.

[jianglizhou]

I verified locally that removing -lstdc++ works fine.

Thanks for testing it. I also need to re-setup my local macos build environment after changing machine.

In fact, you are already changing macOS, since the LINK_TYPE affects all platforms.

Ok. Removed for now.

[magicus]

Looks good now!

[jianglizhou]

Thanks for the review, @magicus!

[jianglizhou]

/integrate

[openjdk]

Going to push as commit a606836.
Since your change was applied there have been 20 commits pushed to the master branch:

• fe0173d: 8345803: Update copyright year to 2024 for security in files where it was missed
• 2207749: 8344629: SSLSocketNoServerHelloClientShutdown test timeout
• 0b9e749: 8345390: [ubsan] systemDictionaryShared.cpp:964: member call on null pointer
• eff20a3: 8345794: Backout doc change introduced by JDK-8235786
• 48e22ba: 8345693: Update JCov for class file version 69
• cc628a1: 8345287: C2: live in computation is broken
• 480b508: 8345156: C2: Add bailouts next to a few asserts
• b120404: 8345793: Update copyright year to 2024 for the build system in files where it was missed
• 7aa0cbc: 8345614: Improve AnnotationFormatError message for duplicate annotation interfaces
• 35c0053: 8345405: Add JMH showing the regression in 8341649
• ... and 10 more: https://git.openjdk.org/jdk/compare/e0d639878346946d0627a57b0eeb0cac8ca533fc...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jianglizhou Pushed as commit a606836.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.