-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
adapting comparable_patch.sh to new signature of removeExcludedFiles #3995
Conversation
fc8e1e3
to
be4d88b
Compare
be4d88b
to
1e17c96
Compare
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.
Spelling / Grammar pass on the comment please.
1e17c96
to
dc25090
Compare
Thanx, I had found only one and half of small issue. hth |
dc25090
to
7235997
Compare
@@ -338,7 +338,8 @@ echo "Successfully removed all Signatures from ${JDK_DIR}" | |||
removeExcludedFiles | |||
|
|||
# Needed due to vendor variation in jmod re-packing after signing, putting attributes in different order | |||
processModuleInfo | |||
# Comparable patch, as per read-me, requires java on path | |||
processModuleInfo "${JDK_DIR}" "${OS}" "$(dirname "$(dirname "$(readlink -f "$(which java)")")")" |
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.
Using "which java" maybe ok, but wondering if we ought to take a "backup" at the start of the JDK being compared, thus ensuring the class_version is good enough for the "javap" processing. This would be similar as was done here:
mkdir "${JDK_DIR}_BK" |
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.
To make backup of jdk ok. My redundant top level wrapper and also the (probably) existing one are doing backup of jdk/allowing to set up 3rd jdk . So that is ok. Unluckily I have not seen the way to pass it into the comparable_patch.sh properly.
Comparable_patch.sh is clear, it needs java/javac on path. Whoever modifed processModuleInfo, have violated that.
Comparable patch should remain as it is, and should not be doing any backups of jdk. Thats the task for its wrappers to deal with it as it is appropriate.
There can be follow up on this set of patches, which will elaborate on usage of path by comaprable_patch, to add additional solution, but right now the comaprable_patch.sh is broken, and this path is fixing it.
@karianna @andrew-m-leonard ping please? The codebase is currently dead without this. A better solution may be elaborated on the fly |
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.
yep lets get this in as is, thanks
tyvm! |
/integrate |
rerunning the failed job |
08621c7#diff-65e4993f80b74f0a4078e853b798cbf6770215b4c31ae1377aa2704eec01ae16R254