-
-
Notifications
You must be signed in to change notification settings - Fork 256
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 macosx runpath dynamic library issue freetype lib #216
Fix macosx runpath dynamic library issue freetype lib #216
Conversation
…rom the appropriate value from environment
… path embedded into the binary, and release it from the runpath dependency - as per suggestion from @mbvreddy, do it only for MacOSX builds (hence check for "darwin") Potentially fixes issue adoptium#202
Testing hypotheis via jenkins job - https://ci.adoptopenjdk.net/view/work%20in%20progress/job/openjdk9_build_x86-64_macos-libfreetype/ @smlambert The artifact JDK that comes out of this build can be something you can take over and run, and see if it helps solve the issue, let me know the outcome. Would also help if you record your test steps on this PR, so I can run the tests myself against the same JDK and see whats happening. I'll just need the command-line commands that you run to reproduce your issue. Thanks. |
Some activities around runpath dependency on the libfreetype... library, snippet from the build logs:
|
…ibrary to the console/logs and also displaying the command install_name_tool with args
A bit disappointing (snippets from the logs):
Back to investigating why it says this? The file does exist in that directory and I have checked it and also run the command manually (auto-complete on the CLI), and it says the same error. |
…propriate place in the script
5e6b8f9
to
fd7caab
Compare
#216 (comment) has now been fixed, so we should get messages like the below in the logs:
|
I am actually using the Jenkins test_personal build to rerun the tests that were excluded due to this problem (instructions on how to run the test_personal build are here: https://github.com/AdoptOpenJDK/openjdk-tests/wiki/How-to-Run-a-Personal-Test-Build-on-Jenkins). I re-enabled them in my branch smlambert:reincludeIssue136Tests. Here are the values I used to test this: |
Link to the personal build: https://ci.adoptopenjdk.net/view/work%20in%20progress/job/test_personal/162/ If they all pass, I will ask for a review and merge this PR: adoptium/aqa-tests#238 |
If they all pass feel free to LGTM the PR, I'll add you to the reviewers list, mostly if you don't get any of the dynamic library related MacOSX issues - as the change is very specific to them, other issues may not get resolved with this fix |
Still see the errors: It was your build number 6 that I should have used? |
@smlambert Yes build 6, basically this JDK https://ci.adoptopenjdk.net/view/work%20in%20progress/job/openjdk9_build_x86-64_macos-libfreetype/lastSuccessfulBuild/artifact/OpenJDK9_x64_Mac_201801021417.tar.gz. I see a different error in https://ci.adoptopenjdk.net/view/work%20in%20progress/job/test_personal/162/testReport/junit/java_beans_EventHandler_Test6179222/java/Test6179222/, not |
https://ci.adoptopenjdk.net/view/work%20in%20progress/job/test_personal/162/testReport/junit/java_beans_EventHandler_Test6179222/java/Test6179222 - contains the same message as we saw before: java.lang.UnsatisfiedLinkError: /Users/jenkins/workspace/test_personal/openjdkbinary/j2sdk-image/lib/libfontmanager.dylib: dlopen(/Users/jenkins/workspace/test_personal/openjdkbinary/j2sdk-image/lib/libfontmanager.dylib, 1): Library not loaded: @ rpath/libfreetype.6.dylib |
@smlambert I have a temporary solution to the |
|
Yes, I can confirm what @mbvreddy has already stated. The 2 problems:
DXU751:tmp jenkins$ otool -L /Users/jenkins/workspace/tmp/jdk-9+181/lib/libfontmanager.dylib -Djava.library.path workaround suggestion does not work. Checked against tests, and against a smaller standalone test that calls System.load("/Users/jenkins/workspace/tmp/jdk-9+181/lib/libfontmanager.dylib") A hackety-hack to force 'success' was but I will wait for the actual fix before reincluding tests. |
1b88bb0
to
5db055c
Compare
echo "" | ||
echo "Releasing the runpath dependency of the dynamic library ${TARGET_DYNAMIC_LIB}" | ||
set -x | ||
install_name_tool -id @rpath/libfreetype.6.dylib "${TARGET_DYNAMIC_LIB}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbvreddy After all the conversations on issue #202, here's our current solution, I have reverted the change to include @rpath
in the parameter.
You mentioned it worked for you, could you please share your command history with us, so I can apply them here.
I have seen @smlambert's workaround, unsure if I can include them in the build script.
@smlambert @mbvreddy the answers might appear obvious to you but I'm hesitating to make a change, as, from past experience, I have seen, one fix resolves one issue and creates a new one. I don't follow dynamic libraries on the MacOSX any more than what I have read about them in this issue. I could apply the copy file approach like how you have done and see how far it goes, can't create a symlink as the change must be compatible with others user environemnts. |
1722d5e
to
ffc3d9b
Compare
… in the jdk/lib folder for MacOSX builds only
ffc3d9b
to
03518bb
Compare
…to both the jdk and jre images created in the build sub-folder
Tested https://ci.adoptopenjdk.net/view/work%20in%20progress/job/openjdk9_build_x86-64_macos-libfreetype/12/artifact/OpenJDK9_x64_Mac_201805021710.tar.gz, and the 40 failures relating to missing library are resolved. https://ci.adoptopenjdk.net/view/work%20in%20progress/job/TestBuild_Sandbox/103/testReport/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - tests passing depending
@smlambert @gdams Can you please review, and approve so I can merge the PR - tests related to this fix have passed as per @smlambert's test Jenkins job. |
@smlambert Please raise the other failing tests as a separate issue on git and we can have a look at it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good!
Thanks @neomatrix369 ! The remaining test failures are very likely due to the x11 server setup issue, which I will look at next. |
Confirmed the test failures related to this issue have been fixed, rerun against: |
Following the issue reported on #202, applied the suggestions from @mbvreddy to see if this helps resolve the test execution issues @smlambert and her testing/JCK testing teams are facing.
At the moment limiting the fix to
openjdk/installedfreetype/lib/libfreetype.6.dylib
only.