-
Notifications
You must be signed in to change notification settings - Fork 237
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
aarch + PATH + slim-build.sh fixes #33
aarch + PATH + slim-build.sh fixes #33
Conversation
dinogun
commented
Jun 28, 2018
- Fix aarch docker image (pick latest tarball for multiple builds).
- Fix PATH for all builds.
- Update the slim-build.sh to the correct one !
1. Fix aarch docker image (pick latest tarball for multiple builds). 2. Fix PATH for all builds. 3. Update the slim-build.sh to the correct one !
Looks like I checked-in the wrong version of slim-build.sh in the previous PR. This PR updates that to the right file and a few other minor updates. Am testing this on all platforms. Will remove WIP once they go through fine. |
fi | ||
|
||
# Validate prerequisites(tools) necessary for making a slim build | ||
tools="jar jarsigner pack200 strip" |
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.
FYI in case you missed it...
http://openjdk.java.net/jeps/336
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.
This should only be run for Java 8 binaries
This looks good. I wonder if the stripping logic should more naturally form part of the build script logic than the Docker preparation? I can imagine that we'd be interested in this for a wide variety of distribution formats. Over time perhaps we'd consider moving it over there. |
Removing WIP as I've tested this. However I did see an issue with nightly sum mismatch in x64 builds in 1 out of 5 tests. That looks like a separate problem (with how the "latest" tag is associated). Will open a separate issue for that at the build repo. Please review, thanks |
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