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

Refactor Initialization of Configuration File Merger #586

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dnestoro
Copy link
Collaborator

Remove macro from merger init and throw better error if executable does not exist

@dnestoro dnestoro requested a review from fniephaus March 21, 2024 13:39
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 21, 2024

private void initializeMergerExecutable() throws MojoExecutionException {
Path nativeImage = NativeImageConfigurationUtils.getNativeImage(logger);
File nativeImageExecutable = nativeImage.toAbsolutePath().toFile();
File mergerExecutable = new File(nativeImageExecutable.getParentFile(), nativeImageConfigureFileName());
Copy link
Member

Choose a reason for hiding this comment

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

Use a variable for nativeImageConfigureFileName() so it's only called once.

Comment on lines 76 to 77
throw new MojoExecutionException("'" + nativeImageConfigureFileName() + "' tool was not found in your " + nativeImage + "." +
"This probably means that the JDK at '" + nativeImage + "' is not a GraalVM distribution."
Copy link
Member

Choose a reason for hiding this comment

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

getNativeImage() already checks whether native-image exists, so if native-image-configure is missing, then it is an incomplete GraalVM distribution. Maybe something like this:

Suggested change
throw new MojoExecutionException("'" + nativeImageConfigureFileName() + "' tool was not found in your " + nativeImage + "." +
"This probably means that the JDK at '" + nativeImage + "' is not a GraalVM distribution."
throw new MojoExecutionException("'" + nativeImageConfigureFileName() + "' tool was not found in the GraalVM JDK at '" + nativeImageExecutable.getParentFile().getParentFile() + "'." +
"This probably means that you are using a GraalVM distribution that is not fully supported by the Native Build Tools."

@dnestoro dnestoro requested review from olpaw and melix April 10, 2024 13:47
@dnestoro dnestoro self-assigned this Apr 12, 2024
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.

None yet

2 participants