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.

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

[JDK 22/23] Fix installation/copy of svm-foreign.jar #399

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

jerboaa
Copy link
Collaborator

@jerboaa jerboaa commented Feb 1, 2024

The copying for svm-foreign.jar in master is broken it copied the directory substratevm/mxbuild/dists/jdk23 to file $MANDREL_HOME/lib/svm/svm-preview/svm-foreign.jar which is broken for two reasons:

  1. It's not actually a jar that gets intalled, but a directory
  2. It installs into the wrong (old JDK 21) installation path: lib/svm/builder/svm-preview over JDK 22+ location next to svm.jar in lib/svm/builder directory.

This patch fixes this and also copies the corresponding src.zip which we do for other artefacts as well.

Thoughts?

@jerboaa jerboaa requested a review from zakkak February 1, 2024 15:56
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Feb 1, 2024
Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerboaa why not add svm-foreign.jar to Mx.artifacts and let the "Copy jars" step in line 112 take care of it as it does for the rest?

@jerboaa
Copy link
Collaborator Author

jerboaa commented Feb 6, 2024

@jerboaa why not add svm-foreign.jar to Mx.artifacts and let the "Copy jars" step in line 112 take care of it as it does for the rest?

Good point. Do you know which artefact coordinates I need to use?

@zakkak
Copy link
Collaborator

zakkak commented Feb 6, 2024

Good point. Do you know which artefact coordinates I need to use?

I'd say org.graalvm.nativeimage:svm-foreign.jar, but it shouldn't really matter as we are not publishing it as a maven artifact, i.e. the coordinates won't be used except in a potential error message if the jar file is not found.

@jerboaa
Copy link
Collaborator Author

jerboaa commented Feb 6, 2024

Good point. Do you know which artefact coordinates I need to use?

I'd say org.graalvm.nativeimage:svm-foreign.jar, but it shouldn't really matter as we are not publishing it as a maven artifact, i.e. the coordinates won't be used except in a potential error message if the jar file is not found.

OK

@jerboaa
Copy link
Collaborator Author

jerboaa commented Feb 6, 2024

@zakkak Done now. Please have another look.

Copy link
Collaborator

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @jerboaa

@zakkak zakkak merged commit 8ff5371 into graalvm:master Feb 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants