8300111: Add rpath for common lib locations for jpackageapplauncher

[davidgiga1993]

This patch adds the -headerpad_max_install_names linker argument to the build of the jpackageapplauncher binary (osx only)

Adding this argument allows the user to use the install_name_tool to add search paths for the dynamic linker to the launcher binary. This is required for certain use cases where a native library loads other dynamic native libraries via dlopen.

The change has been successfully tested on osx on aarch64:

make jdk.jpackage
install_name_tool -add_rpath @executable_path/../Frameworks/ jpackageapplauncher
otool -l jpackageapplauncher

Load command 19
          cmd LC_RPATH
      cmdsize 48
         path @executable_path/../Frameworks/ (offset 12)

---

Progress

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

Issue

• JDK-8300111: Add rpath for common lib locations for jpackageapplauncher

Reviewers

• Magnus Ihse Bursie (@magicus - Reviewer)
• Alexey Semenyuk (@alexeysemenyukoracle - Reviewer)
• Alexander Matveev (@sashamatveev - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11922

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

Using diff file

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

[bridgekeeper]

Hi @davidgiga1993, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user davidgiga1993" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

[openjdk]

⚠️ @davidgiga1993 a branch with the same name as the source branch for this pull request (master) is present in the target repository. If you eventually integrate this pull request then the branch master in your personal fork will diverge once you sync your personal fork with the upstream repository.

To avoid this situation, create a new branch for your changes and reset the master branch. You can do this by running the following commands in a local repository for your personal fork. Note: you do not have to name the new branch NEW-BRANCH-NAME.

$ git checkout -b NEW-BRANCH-NAME
$ git branch -f master 5f37cbec942d081a87fc7ef49a0a3d9c932774fc
$ git push -f origin master

Then proceed to create a new pull request with NEW-BRANCH-NAME as the source branch and close this one.

[openjdk]

@davidgiga1993 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.

[sashamatveev]

What additional paths except "@executable_path/../Frameworks/" your application requires? If you just need @executable_path/../Frameworks/, then I think we should just add this to rpath permanently, instead of using -headerpad_max_install_names and install_name_tool. "Frameworks" and "PlugIns" are two standard folders to keep additional dynamic libraries, so I think we need to add both of them and do not use -headerpad_max_install_names.

[davidgiga1993]

What additional paths except "@executable_path/../Frameworks/" your application requires? If you just need @executable_path/../Frameworks/, then I think we should just add this to rpath permanently, instead of using -headerpad_max_install_names and install_name_tool. "Frameworks" and "PlugIns" are two standard folders to keep additional dynamic libraries, so I think we need to add both of them and do not use -headerpad_max_install_names.

For my specific use case it would just be Frameworks. But I agree that adding both of them would be an option as well which should cover most/all use cases.

I've adjusted the pr

[davidgiga1993]

/signed

[bridgekeeper]

You are already a known contributor!

[sashamatveev]

Looks good. You will need to file a JBS issue for this.

[openjdk]

@davidgiga1993 Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[davidgiga1993]

Looks good. You will need to file a JBS issue for this.

Since I'm not a member of any JDK project, I've used https://bugreport.java.com/, thus the ID starts with a 9. Is that fine or should I wait until this gets transferred to an JDK- number?

[mlbridge]

Webrevs

• 00: Full (036addef)

[magicus]

Hi David,
Thank you for your contribution!

I updated the title of the JBS bug issue to match this PR (matching titles is a requirement, and the PR had a more descriptive title).

The change itself look sane to me, but I'd like for @alexeysemenyukoracle to have a look at it as well.

Also, if you ever is going to want to contribute additional PRs to OpenJDK, please see the bot's advice above. Otherwise you are entering a world of pain.

/reviewers 2

[davidgiga1993]

Hi David, Thank you for your contribution!

I updated the title of the JBS bug issue to match this PR (matching titles is a requirement, and the PR had a more descriptive title).

The change itself look sane to me, but I'd like for @alexeysemenyukoracle to have a look at it as well.

Also, if you ever is going to want to contribute addition PRs to OpenJDK, please see the bot's advice above. Otherwise you are entering a world of pain.

/reviewers 2

Thanks for your feedback!
Will do, overlooked that part in the contribution guide

[openjdk]

⚠️ @davidgiga1993 the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout master
$ git commit --author='Preferred Full Name <you@example.com>' --allow-empty -m 'Update full name'
$ git push

[openjdk]

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

8300111: Add rpath for common lib locations for jpackageapplauncher

Reviewed-by: ihse, asemenyuk, almatvee

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

• 5a4945c: 8299975: Limit underflow protection CMoveINode in PhaseIdealLoop::do_unroll must also protect type from underflow
• f307e8c: 8299795: Relativize locals in interpreter frames
• 11aadc9: 8244400: MenuItem may cache the size and did not update it when the screen DPI is changed
• 836198a: 8300591: @SuppressWarnings option "lossy-conversions" missing from jdk.compiler module javadoc
• 45e4e00: 8300079: SIGSEGV in LibraryCallKit::inline_string_copy due to constant NULL src argument
• 030b071: 8300207: Add a pre-check for the number of canonical equivalent permutations in j.u.r.Pattern
• 7ced08d: 8300638: Tier1 IR Test failure after JDK-8298632 on macosx-x64-debug
• 3ea4eac: 8300817: The build is broken after JDK-8294693
• cbfc069: 8300731: Avoid unnecessary array fill after creation in PaletteBuilder
• 67b1c89: 8294693: Add Collections.shuffle overload that accepts RandomGenerator interface
• ... and 212 more: https://git.openjdk.org/jdk/compare/5f37cbec942d081a87fc7ef49a0a3d9c932774fc...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 (@magicus, @alexeysemenyukoracle, @sashamatveev) 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).

[openjdk]

@magicus
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[magicus]

Also, if you ever is going to want to contribute addition PRs to OpenJDK, please see the bot's advice above. Otherwise you are entering a world of pain.

Thanks for your feedback! Will do, overlooked that part in the contribution guide

It might even be missing from the contribution guide. Updating and expanding it is an ever ongoing project. checking

It says: "The recommendation is to always create a new branch for any change you intend to implement. By doing that you can easily work on many different changes in parallel in the same code repository." The recommendation is correct, but should be worded more definitely: Things will break if you do not create a branch. I'll propose an update to the guide. Thanks for your assistance in making OpenJDk more approachable for new developers.

[alexeysemenyukoracle]

Looks good.
@magicus, @sashamatveev is the best to ask for macosx-related reviews in jpackage.

[sashamatveev]

@davidgiga1993 Do you have JBS account, so https://bugs.openjdk.org/browse/JDK-8300111 can be assign to you?

[davidgiga1993]

@sashamatveev No since this is my first contribution (I assume) the account creation/password reset doesn't work. Thus I don't have one :)

[magicus]

The JBS assignment is not important.
@davidgiga1993 You can now integrate this PR. You do that by giving the "PR comment" /integrate on single line in a github comment. (We don't use the usual Github process where a "maintainer" merges a PR using Githubs GUI.) After that, someone with committer permissions will sponsor your integration. (I can do that)

[davidgiga1993]

/integrate

[openjdk]

@davidgiga1993
Your change (at version 036adde) is now ready to be sponsored by a Committer.

[alexeysemenyukoracle]

/sponsor

[openjdk]

@alexeysemenyukoracle @davidgiga1993 Unable to acquire the integration lock; aborting sponsored integration. The error has been logged and will be investigated.

[erikj79]

Something is causing high contention in the bot, it's under investigation. In the meantime, all you can do is retry this /sponsor command until it goes through.

[alexeysemenyukoracle]

/sponsor

[openjdk]

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

• 2da2e5a: 8300946: Add sun/security/provider/certpath/OCSP/OCSPNoContentLength to ProblemList
• 0323609: 8300706: Use @snippet in java.text
• b5ee3d1: 8299497: Usage of constructors of primitive wrapper classes should be avoided in java.desktop API docs
• 77a5010: 8286775: Remove identical per-compiler definitions of unsigned integral jtypes
• f79e587: 8300828: Avoid unnecessary array fill after creation in com.sun.media.sound
• 56dc3b0: Merge
• fd75217: 8290919: Update nroff pages in JDK 20 before RC
• 0ea2dd1: 8146132: Excessive output from make test-image
• dcf1523: 8300077: Refactor code examples to use @snippet in java.text.ChoiceFormat
• d117350: 8300866: Declare some classes final in java.io
• ... and 230 more: https://git.openjdk.org/jdk/compare/5f37cbec942d081a87fc7ef49a0a3d9c932774fc...master

Your commit was automatically rebased without conflicts.

[openjdk]

@alexeysemenyukoracle @davidgiga1993 Pushed as commit 937ba1c.

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