8295231: Move all linking of native libraries to make

[TheShermanTanker]

Some external libraries required by native code are linked via linker comments embedded in pragmas. Searching for which libraries are linked can then become frustrating and confusing since they may be included in an obscure place, and for all relevant compilers there is no difference between specifying them from make and in a source file. The easiest solution is to just always link them from make and remove any source level linkage.

---

Progress

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

Issue

• JDK-8295231: Move all linking of native libraries to make

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10633

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

Using diff file

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

[bridgekeeper[bot]]

:wave: Welcome back jwaters! 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[bot]]

@TheShermanTanker The following labels will be automatically applied to this pull request:

• build
• core-libs
• nio
• security

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

[mlbridge[bot]]

Webrevs

• 05: Full - Incremental (4eb2eb7b)
• 04: Full - Incremental (0779d1fa)
• 03: Full - Incremental (8afaf85d)
• 02: Full - Incremental (fe15f907)
• 01: Full - Incremental (54e6ec10)
• 00: Full (cfa80528)

[tstuefe]

I like this change. I am pretty sure not many know about this feature at all, we don't have that many knowledgeable Windows developers. If the methods are equivalent, I prefer linking via make file.

Pinging @magicus, maybe he can chime in.

Cheers, Thomas

[magicus]

Wow. I did not even know this was possible. Thank you for fixing this! I would have been mighty surprised if I were to learn that a library has more dependencies than the one in the makefile.

@dholmes-ora The point is that we need to be consistent. If we would go that route, then all libraries should be loaded by pragmas. That could of course be an alternative, but I really see no upside to it. To do it like we currently do, load 99% of the libraries via make files, and then have few scattered pragmas, that's just bad.

[magicus]

@TheShermanTanker Question: is this a Windows-specific thing, or are there pragma-loaded libraries for other compilers as well?

[openjdk[bot]]

@TheShermanTanker 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:

8295231: Move all linking of native libraries to make

Reviewed-by: ihse, erikj

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 17 new commits pushed to the master branch:

• ae60599e2ba75d80c3b4279903137b2c549f8066: 8295023: Interpreter(AArch64): Implement -XX:+PrintBytecodeHistogram and -XX:+PrintBytecodePairHistogram options
• 4d37ef2d545c016e6c3ad52171ea961d4406726f: 8295262: Build binutils out of source tree
• 0919a3a0c198a5234b5ed9a3bb999564d2382a56: 8294186: AArch64: VectorMaskToLong failed on SVE2 machine with -XX:UseSVE=1
• ec2981b83bc3ef6977b5f16d5222eb49b0ea49ad: 8293711: Factor out size parsing functions from arguments.cpp
• 5d273b9f040a9884e2ae5b0f1409a3f9075c51c9: 8295278: Add parallel class loading tests
• 172006c0e9433046252bd79e8864890ab7c0ce56: 8295333: G1: Remove unnecessary check in G1Policy::calculate_desired_eden_length_by_mmu
• 7743345f6f73398f280fd18364b4cea10a6b0f2f: 8294314: Minimize disabled warnings in hotspot
• 552d8a2821f03046896a728d6e4cec0ef754d3f4: 8295192: Use original configure command line when called from a script
• cf07eaeb9291da725181832b8bb1dc54957ba886: 8295020: javac emits incorrect code for for-each on an intersection type.
• b3bb3e6ed89f3abcaae584fcbe75688141e886cb: 8295325: tools/jlink/plugins/SaveJlinkArgfilesPluginTest.java fails on Linux ppc64le
• ... and 7 more: https://git.openjdk.org/jdk/compare/0043d58c5d52c3b299a4b6dfcec34a7db5041aea...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@dholmes-ora, @magicus, @alexeysemenyukoracle, @erikj79) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[TheShermanTanker]

@TheShermanTanker Question: is this a Windows-specific thing, or are there pragma-loaded libraries for other compilers as well?

To my knowledge only Visual C++ has the ability to perform linking through pragmas, the comment pragma works by leaving a linker comment in the object file (hence the name), meaning this only works with the Visual C++ linker, so while clang does support this pragma with clang-cl, for the use cases in the JDK it wouldn't matter. gcc does not have an equivalent pragma (or provide support for linking from inside source files at all, for that matter)

If the methods are equivalent, I prefer linking via make file.

There isn't any difference between the 2 unless the library is tampered with by -nodefaultlib, but I can only find that specified in https://github.com/openjdk/jdk/blob/a033aa5a3d9c63d72d11af218b9896b037fbd8de/make/autoconf/flags-other.m4#L38 and https://github.com/openjdk/jdk/blob/392f35df4be1a9a8d7a67a25ae01230c7dd060ac/make/autoconf/lib-hsdis.m4#L45, neither of which have an effect on the libraries in this changeset

[TheShermanTanker]

Thanks all for the reviews, I placed comments where the pragmas used to be detailing the library required by the source file to address David's concerns

[TheShermanTanker]

/integrate

[openjdk[bot]]

@TheShermanTanker Your change (at version 4eb2eb7baeeb47fb54c604e94b749de5752659f2) is now ready to be sponsored by a Committer.

[erikj79]

The change looks harmless. Howevere I don't understand how searching for the standard Windows libs can then become frustrating.

I believe this is part of the effort for https://bugs.openjdk.org/browse/JDK-8288293.

[alexeysemenyukoracle]

I believe this is part of the effort for https://bugs.openjdk.org/browse/JDK-8288293.

Agree. I'd prefer to have a different description of the bug though to make it clear that this is necessary for decoupling a compiler and an OS.

[magicus]

/sponsor

[openjdk[bot]]

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

• f300ec8631b781938e6e96165ba23cda14a20f24: 8294546: document where javac differs when invoked via launcher and ToolProvider
• b269c51d10c353d9b7143b2239beb23c01352182: 8295395: Linux Alpha Zero builds fail after JDK-8292591
• ae60599e2ba75d80c3b4279903137b2c549f8066: 8295023: Interpreter(AArch64): Implement -XX:+PrintBytecodeHistogram and -XX:+PrintBytecodePairHistogram options
• 4d37ef2d545c016e6c3ad52171ea961d4406726f: 8295262: Build binutils out of source tree
• 0919a3a0c198a5234b5ed9a3bb999564d2382a56: 8294186: AArch64: VectorMaskToLong failed on SVE2 machine with -XX:UseSVE=1
• ec2981b83bc3ef6977b5f16d5222eb49b0ea49ad: 8293711: Factor out size parsing functions from arguments.cpp
• 5d273b9f040a9884e2ae5b0f1409a3f9075c51c9: 8295278: Add parallel class loading tests
• 172006c0e9433046252bd79e8864890ab7c0ce56: 8295333: G1: Remove unnecessary check in G1Policy::calculate_desired_eden_length_by_mmu
• 7743345f6f73398f280fd18364b4cea10a6b0f2f: 8294314: Minimize disabled warnings in hotspot
• 552d8a2821f03046896a728d6e4cec0ef754d3f4: 8295192: Use original configure command line when called from a script
• ... and 9 more: https://git.openjdk.org/jdk/compare/0043d58c5d52c3b299a4b6dfcec34a7db5041aea...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@magicus @TheShermanTanker Pushed as commit 8d751de3198675b22704cdccafaff2fc0fdd3f59.

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