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

#888: fixed graalvm gu #890

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

#888: fixed graalvm gu #890

wants to merge 7 commits into from
+11 −1

Conversation

jan-vcapgemini
Copy link
Contributor

Fixes: #888

Implements:

  • added check for graalvm community edition 21 to prevent usage of missing gu command

@jan-vcapgemini
devonfw#888: fixed graalvm gu
34de013 
added check for graalvm community edition 21 to prevent usage of missing gu command
@jan-vcapgemini jan-vcapgemini self-assigned this Dec 11, 2024
@jan-vcapgemini jan-vcapgemini added bugfix graalvm GraalVM, native-image, truffle, etc. native-image GraalVM native image of IDEasy CLI (e.g. for bugs that only happen in binary of CLI) labels Dec 11, 2024
@coveralls
Copy link
Collaborator

coveralls commented Dec 11, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 12302038821

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 11 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.05%) to 67.38%
Files with Coverage Reduction New Missed Lines %
com/devonfw/tools/ide/cli/CliProcessException.java 2 0.0%
com/devonfw/tools/ide/tool/graalvm/GraalVm.java 9 38.46%
Totals Coverage Status
Change from base Build 12300570436: -0.05%
Covered Lines: 6653
Relevant Lines: 9533
💛 - Coveralls

@alfeilex alfeilex self-requested a review December 11, 2024 15:01
@alfeilex
Copy link
Member

I installed the community edition with version 17.0.9 and I got this error after the installation was completed:

Start: Install plugin native-image
Downloading: Component catalog from www.graalvm.org
Processing Component: Native Image
Component Native Image (org.graalvm.native-image) is already installed.
Step 'Install plugin native-image' ended with failure.
Successfully installed graalvm in version 17.0.9 replacing previous version 20.3.6
  Step 'Install plugin native-image' failed: unexpected error
1 step(s) failed out of 3 steps.

Is that error still in the scope of this issue? I can confirm that gu.cmd is inside the bin folder.

hohwille
hohwille requested changes Dec 12, 2024
View reviewed changes
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@jan-vcapgemini thanks for your PR and work to fix #888.
IMHO this current implementation is making it worse not better.
Please check my review comment.

cli/src/main/java/com/devonfw/tools/ide/tool/graalvm/GraalVm.java Outdated
private void doGuPluginCommand(ToolPluginDescriptor plugin, String command) {
this.context.newProcess().errorHandling(ProcessErrorHandling.THROW_CLI).executable(getToolPath().resolve("bin").resolve("gu")).addArgs(command, plugin.name()).run(ProcessMode.DEFAULT);
if (this.getConfiguredEdition().equals("community") && this.getConfiguredVersion().isLess(VersionIdentifier.of("21"))) {
Copy link
Member

Choose a reason for hiding this comment

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

This is incorrect: The default edition is graalvm and community edition is completely obsolete nonsense we just kept for devonfw-ide compatibility. Because of a bug in that community edition in very old version, you now disable the plugin installation for any edition other than community what is IMHO a bug rendering the plugin feature void.

If we want to address this bug properly why dont we check if gu is actually available instead of coding assumptions of editions and versions we just guessed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with your suggestion now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hohwille hohwille hohwille requested changes

@alfeilex alfeilex Awaiting requested review from alfeilex

Assignees

@jan-vcapgemini jan-vcapgemini

Labels
bugfix graalvm GraalVM, native-image, truffle, etc. native-image GraalVM native image of IDEasy CLI (e.g. for bugs that only happen in binary of CLI)
Projects
IDEasy board
Status: Team Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

graalvm is not working on Mac x64
4 participants
@jan-vcapgemini @coveralls @alfeilex @hohwille