Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Another attempt to fix libcrypto.dylib loading issue #112

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

advancedxy
Copy link
Contributor

Which issue does this PR close?

Closes #76

Rationale for this change

The previous fix(#55) is not correct as it doesn't propagate DYLD_LIBRARY_PATH to the test phase: the export statement is executed in a bash shell, which is lost in the new shell command in the test phase.

Q: If the library path is not correctly set, how does most of the test instances are passed?
A: It turns out that Github's mac runners have inconsistent state of SIP enable status. See actions/runner-images#9091 for related reports, and I also observed related output in my personal debug branch. When SIP is disabled, the X86 mac runners will try to find libcrypto.dylib in /usr/lib/local on macos-latest(a.k.a: macos-12), which are already linked by homebrew's openssl lib. Below is a screenshot that listing /usr/local/lib:
image
So all the tests passed are ran on Mac with SIP disabled.


In theory, even if SIP is turned on, the java binary from zulu distribution should also try to find libraries in /usr/local/lib if it's unrestricted[1]. I am not sure and didn't find a clue why it doesn't and fails the test.


Q: How does upgrade to macos-13 fix this problem?
A: macos-13's SIP is turned off, See: actions/runner-images#8162. So it's possible to manipulate the DYLD_LIBARARY_PATH for our Java tests for now.

What changes are included in this PR?

  1. upgrade X86 mac runners to macos-13
  2. correctly setup DYLD_LIBRARY_PATH

How are these changes tested?

Manual verification

Notes:
1: you can look up related documentation with man dlopen on your Mac.

Copy link
Member

@sunchao sunchao left a comment

Choose a reason for hiding this comment

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

LGTM

echo "openssl lib path is: ${OPENSSL_LIB_PATH}"
export DYLD_LIBRARY_PATH=$OPENSSL_LIB_PATH:$DYLD_LIBRARY_PATH
echo "DYLD_LIBRARY_PATH=$OPENSSL_LIB_PATH:$DYLD_LIBRARY_PATH" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

👍

@sunchao sunchao merged commit 7be5a18 into apache:main Feb 26, 2024
15 checks passed
@sunchao
Copy link
Member

sunchao commented Feb 26, 2024

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MacOS (x86_64) CI is flaky with libcrypto issue
2 participants