-
-
Notifications
You must be signed in to change notification settings - Fork 251
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 - JDK8 #225
Fix macosx runpath dynamic library issue freetype lib - JDK8 #225
Conversation
…variable as different versions of Java call jre image folder name with a different name
…f not break the build or warn the user about it and continue the process
Ill be running this against as many JDK builders I can on Jenkins and posting back the links |
…to colour-codes.sh where it actually belongs
8fb5763
to
770394d
Compare
…nt lib for the JDK image folder for JDK8
… folder for all JDKs 10 and onwards
… failed previously, should work fine now
…NT_LIB_FOR_JDK_FLAG and COPY_MACOSX_FREE_FONT_LIB_FOR_JRE_FLAG
…NT_LIB_FOR_JDK_FLAG and COPY_MACOSX_FREE_FONT_LIB_FOR_JRE_FLAG
…or all JDK versions after JDK9, and also specific settings for JDK9
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.
2 small spelling typos.
sbin/build.sh
Outdated
SOURCE_LIB_NAME="${IMAGE_DIRECTORY}/lib/libfreetype.dylib.6" | ||
if [ ! -f "${SOURCE_LIB_NAME}" ]; then | ||
echo "${error}[Error] ${SOURCE_LIB_NAME} does not exists in the ${IMAGE_DIRECTORY} folder, please check if this is the right folder to refer to, aborting copy process...${normal}" |
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.
exists --> exist
sbin/build.sh
Outdated
otool -L "${INVOKED_BY_FONT_MANAGER}" | ||
else | ||
# shellcheck disable=SC2154 | ||
echo "${warning}[Warning] ${INVOKED_BY_FONT_MANAGER} does not exists in the ${IMAGE_DIRECTORY} folder, please check if this is the right folder to refer to, this may cause runtime issues, please beware...${normal}" |
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.
exists --> exist
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.
In discussion in testing slack channel, we will verify this fix on the main builds once its merged, and then remove the test excludes that relate to this problem.
We have already verified the fix in @neomatrix369 WIP builds for Java8/9 (https://ci.adoptopenjdk.net/view/work%20in%20progress/job/openjdk8_build_x86-64_macos_libfreefont_fix/, https://ci.adoptopenjdk.net/view/work%20in%20progress/job/openjdk9_build_x86-64_macos-libfreetype/).
…reetype-lib # Conflicts: # makejdk-any-platform.sh
Are we OK to merge this now? |
@smlambert and Muneer came across the same test failure reported via issue #202, see https://ci.adoptopenjdk.net/view/work%20in%20progress/job/test_personal/266/testReport/junit/javax_sound_midi_Devices_InitializationHang/java/InitializationHang/
This is due to the fact JDK8 has its JDK and JRE folders named slightly differently than the other JDKs. This has now been corrected.
The build will break if the one of dynamic lib files are not found, there will be a warning for the other one:
libfreetype.6.dylib (error, build breaks)
libfontmanager.dylib (warning, build continues)
Let me know if this flow is good and acceptable.
Please also test this out in other versions of the JDK wherever relevant so we know that the fix works for all MacOSX compliant JDKs.
Scripts have been adapted that it does the respective actions for the different versions of the JDK:
JDK8: only copy to JRE folder
JDK9: copy to JDK and JRE folder
JDK10 onwards: skip this copy process
@smlambert and Muneer can you please confirm if your tests fail for any of these versions after the fix has been applied. Have you had a chance to test it for JDK10 and JDK11?