Skip to content

openjdk: Ensure rpath for libraries includes server/lib#111255

Closed
deejgregor wants to merge 3 commits intoHomebrew:masterfrom
deejgregor:fix-openjdk-library-rpath
Closed

openjdk: Ensure rpath for libraries includes server/lib#111255
deejgregor wants to merge 3 commits intoHomebrew:masterfrom
deejgregor:fix-openjdk-library-rpath

Conversation

@deejgregor
Copy link
Copy Markdown
Contributor

@deejgregor deejgregor commented Sep 20, 2022

Also:
openjdk@11: Ensure rpath for libraries includes server/lib
openjdk@17: Ensure rpath for libraries includes server/lib

I couldn't build openjdk@8 because I have too new of a version of Xcode installed, so I did not include a change for that formula.

Closes: #111068

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@BrewTestBot BrewTestBot added the automerge-skip `brew pr-automerge` will skip this pull request label Sep 20, 2022
@deejgregor deejgregor force-pushed the fix-openjdk-library-rpath branch from 1f47448 to 24266f9 Compare September 20, 2022 19:31
@BrewTestBot BrewTestBot removed the automerge-skip `brew pr-automerge` will skip this pull request label Sep 20, 2022
@deejgregor
Copy link
Copy Markdown
Contributor Author

Question: I'm not sure if this needs a new revision as mentioned here: https://github.com/Homebrew/homebrew-core/blob/HEAD/CONTRIBUTING.md#to-contribute-a-fix-to-the-foo-formula

@jonchang jonchang added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Sep 20, 2022
@cho-m cho-m added the long build Set a long timeout for formula testing label Sep 20, 2022
@carlocab
Copy link
Copy Markdown
Member

Yes, let's bump the revision.

You can also remove the ignore_missing call inside the on_linux blocks. Those will be fixed by the -rpath configuration.

@cho-m cho-m mentioned this pull request Sep 21, 2022
@carlocab carlocab force-pushed the fix-openjdk-library-rpath branch from 24266f9 to abf1507 Compare September 21, 2022 02:47
Comment on lines 91 to 99
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd personally have a ldflags array that we join(" ") so we don't have this duplication.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I considered that too. Let's do that.

@carlocab carlocab added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Sep 21, 2022
@carlocab carlocab force-pushed the fix-openjdk-library-rpath branch from abf1507 to 8eeb8a2 Compare September 21, 2022 03:14
@carlocab
Copy link
Copy Markdown
Member

Linkage still broken on Linux. The build may not be respecting --with-extra-ldflags.

@carlocab carlocab added CI-skip-recursive-dependents Pass --skip-recursive-dependents to brew test-bot. and removed CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. labels Sep 21, 2022
@carlocab
Copy link
Copy Markdown
Member

Here's the RPATH in libmanagement.so in the Linux bottle for openjdk:

$ORIGIN
/homebrew/linuxbrew/.linuxbrew/Cellar/openjdk/18.0.2.1_1/lib
/homebrew/linuxbrew/.linuxbrew/opt/gcc/lib/gcc/current
/homebrew/linuxbrew/.linuxbrew/opt/alsa-lib/lib
/homebrew/linuxbrew/.linuxbrew/opt/openssl@1.1/lib
/homebrew/linuxbrew/.linuxbrew/opt/bison/lib
/homebrew/linuxbrew/.linuxbrew/opt/krb5/lib
/homebrew/linuxbrew/.linuxbrew/opt/zlib/lib
/homebrew/linuxbrew/.linuxbrew/opt/gmp/lib
/homebrew/linuxbrew/.linuxbrew/opt/bdw-gc/lib
/homebrew/linuxbrew/.linuxbrew/opt/libtool/lib
/homebrew/linuxbrew/.linuxbrew/opt/libunistring/lib
/homebrew/linuxbrew/.linuxbrew/opt/ncurses/lib
/homebrew/linuxbrew/.linuxbrew/opt/readline/lib
/homebrew/linuxbrew/.linuxbrew/opt/libffi/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxcrypt/lib
/homebrew/linuxbrew/.linuxbrew/opt/guile/lib
/homebrew/linuxbrew/.linuxbrew/opt/icu4c/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxml2/lib
/homebrew/linuxbrew/.linuxbrew/opt/gettext/lib
/homebrew/linuxbrew/.linuxbrew/opt/libidn2/lib
/homebrew/linuxbrew/.linuxbrew/opt/libtasn1/lib
/homebrew/linuxbrew/.linuxbrew/opt/nettle/lib
/homebrew/linuxbrew/.linuxbrew/opt/p11-kit/lib
/homebrew/linuxbrew/.linuxbrew/opt/libevent/lib
/homebrew/linuxbrew/.linuxbrew/opt/libnghttp2/lib
/homebrew/linuxbrew/.linuxbrew/opt/expat/lib
/homebrew/linuxbrew/.linuxbrew/opt/unbound/lib
/homebrew/linuxbrew/.linuxbrew/opt/gnutls/lib
/homebrew/linuxbrew/.linuxbrew/opt/cups/lib
/homebrew/linuxbrew/.linuxbrew/opt/libpng/lib
/homebrew/linuxbrew/.linuxbrew/opt/bzip2/lib
/homebrew/linuxbrew/.linuxbrew/opt/freetype/lib
/homebrew/linuxbrew/.linuxbrew/opt/util-linux/lib
/homebrew/linuxbrew/.linuxbrew/opt/fontconfig/lib
/homebrew/linuxbrew/.linuxbrew/opt/isl/lib
/homebrew/linuxbrew/.linuxbrew/opt/mpfr/lib
/homebrew/linuxbrew/.linuxbrew/opt/libmpc/lib
/homebrew/linuxbrew/.linuxbrew/opt/lz4/lib
/homebrew/linuxbrew/.linuxbrew/opt/xz/lib
/homebrew/linuxbrew/.linuxbrew/opt/zstd/lib
/homebrew/linuxbrew/.linuxbrew/opt/binutils/lib
/homebrew/linuxbrew/.linuxbrew/opt/gcc/lib
/homebrew/linuxbrew/.linuxbrew/opt/libpthread-stubs/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxau/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxdmcp/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxcb/lib
/homebrew/linuxbrew/.linuxbrew/opt/libx11/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxext/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxrender/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxrandr/lib
/homebrew/linuxbrew/.linuxbrew/opt/libice/lib
/homebrew/linuxbrew/.linuxbrew/opt/libsm/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxt/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxfixes/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxi/lib
/homebrew/linuxbrew/.linuxbrew/opt/libxtst/lib
/homebrew/linuxbrew/.linuxbrew/lib

This is missing $ORIGIN/server, which means our flag isn't being respected.

@carlocab carlocab added the linux Linux is specifically affected label Sep 21, 2022
@carlocab carlocab force-pushed the fix-openjdk-library-rpath branch from 8eeb8a2 to b78b059 Compare September 21, 2022 14:01
@carlocab
Copy link
Copy Markdown
Member

Ok, not sure what's going on with Linux, so I'll keep the ignore_missing_libraries for now. CC @Homebrew/linux in case anyone has any ideas.

@carlocab carlocab added CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. and removed CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. labels Sep 21, 2022
@carlocab
Copy link
Copy Markdown
Member

Dependent tests succeeded, but openjdk@11 failed to build on Linux. To save CI time let's re-run but skip dependents.

@carlocab carlocab added CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. CI-skip-dependents Pass --skip-dependents to brew test-bot. and removed CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Sep 22, 2022
@carlocab carlocab added CI-test-bot-fail-fast Pass --fail-fast to brew test-bot. and removed CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. labels Sep 22, 2022
Copy link
Copy Markdown
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@BrewTestBot
Copy link
Copy Markdown
Contributor

:shipit: @carlocab has triggered a merge.

@danielnachun
Copy link
Copy Markdown
Contributor

The ignore_missing_libraries is needed because OpenJDK finds these libraries using dlopen at runtime. So to the best of my knowledge this is expected and not a problem.

@carlocab
Copy link
Copy Markdown
Member

ignore_missing_libraries is something we carry around for essentially just OpenJDK, though, and it wouldn't be needed if we just had $ORIGIN/server in RPATH. It would be good to be able to get rid of it.

@github-actions github-actions bot added the outdated PR was locked due to age label Oct 26, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2022
@deejgregor deejgregor deleted the fix-openjdk-library-rpath branch December 4, 2023 02:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI-skip-dependents Pass --skip-dependents to brew test-bot. CI-skip-recursive-dependents Pass --skip-recursive-dependents to brew test-bot. CI-test-bot-fail-fast Pass --fail-fast to brew test-bot. linux Linux is specifically affected long build Set a long timeout for formula testing outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openjdk: dlopen/rpath issues when loading an internal openjdk .so after loading an external .so

7 participants