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.

Sign up for GitHub

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

Jump to bottom

adapting comparable_patch.sh to new signature of removeExcludedFiles #3995

Merged
merged 1 commit into from
Oct 17, 2024
Merged

adapting comparable_patch.sh to new signature of removeExcludedFiles #3995

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion tooling/reproducible/comparable_patch.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)")")")"
Copy link
Contributor

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:

temurin-build/tooling/reproducible/repro_compare.sh

Line 24 in 5035fa3

mkdir "${JDK_DIR}_BK"

Copy link
Contributor Author

@judovana judovana Oct 15, 2024 •
edited
Loading

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.


# Patch Windows VS_VERSION_INFO[COMPANY_NAME]
if [[ "$OS" =~ CYGWIN* ]] && [[ "$PATCH_VS_VERSION_INFO" = true ]]; then
Expand Down
Loading