#1676 import extra sdks automatically into ide - #2073
Conversation
- Load extra SDKs from ide-extra-tools.json and register supported ones in IntelliJ. - The IntelliJ merge template is expected from the settings repository.
…utomatically-into-IDE-' into feature/1676-Import-extra-SDKs-automatically-into-IDE-
…atically-into-IDE-
Coverage Report for CI Build 30243693167Coverage increased (+0.1%) to 72.69%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions51 previously-covered lines in 7 files lost coverage.
Coverage Stats💛 - Coveralls |
…med attribute placeholders in jdk-extra-java.xml
hohwille
left a comment
There was a problem hiding this comment.
@vivu001 thanks for your PR and sorry to interrupt before team review.
I had a look into your diff and found some misunderstandings and before moving any further, I wanted to guide the way to the proper solution.
You did a lot perfectly correct, but there are some misunderstandings between #1166 and #1676 that seemed not clear.
Also preparing this implementation for VSCode, Eclipse and other potential IDEs or even future SDKs I suggest some more generic approach and according refactoring.
Please have a look and see if you can follow my idea and suggestions.
If there are any questions do not hesitate to reach out.
…-into-IDE- # Conflicts: # CHANGELOG.adoc
…elliJ sees available SDKs
|
I went through the testing instructions and was able to reproduce the expected behavior successfully. The extra SDKs were imported correctly, and ide update reinstalled and synchronized the SDKs as expected. |
| @Override | ||
| public ToolInstallation install(ToolInstallRequest request) { | ||
|
|
||
| configureWorkspace(); |
There was a problem hiding this comment.
Ah great finding. So the big picture gets clearer.
But as I already wrote maybe this was done on purpose.
Can you test that without this still the plugins get installed into the correct plugins folder inside the IDEasy project and are found as installed when I launch ide intellij?
If that is the case, then this should solve the problem and we simply do not need to configure the workspace at all during installation and you can also remove the extra loop in AbstractUpdateCommandlet.
There was a problem hiding this comment.
I tested it: without configureWorkspace() in IdeToolCommandlet.install() the extra SDK installation is still unaffected. The extra SDKs are still installed by the create/update flow and the workspace synchronization is only needed later when ide intellij imports them.
In the AbstractUpdateCommandlet, removing the extra loop shows that extra SDKs still work when launching ide intellij, but the plugins are NOT installed into $IDE_HOME/plugins/intellij. So this workspace sync is still needed for plugin provisioning, even though the SDK part can already be handled at IDE launch.
There was a problem hiding this comment.
What is the current state and how can we finally resolve this.
IMHO this change is logically correct and also if noting is left in this method you can even remove the method override since it is pointless now.
However, as you also found out and confirmed the plugin installation requires somehow the workspace configuration to be triggered.
Reading your PR diff again, I now found that you are triggering this in IdeaBasedToolCommandlet when a plugin is to be installed.
However, then we are still not done:
- How about other IDEs like VSCode or Eclipse - can we install plugins there without
configureWorkspace()being invoked. Or is this only an IDEA/JetBrains specific thing not needed for other IDEs (because duringconfigureWorkspace()we create theidea.propertiesin the workspace what makes this different from non-IDEA based IDEs)? IMHO for Eclipse and VSCode we will now never configure the workspace any more when we change your PR as is so it would be broken. - Performance impact: If we install 20 plugins, we will now configure the workspace 20 times. This seems to be a waste and in that case the old approach to trigger it once during the installation still seems more reasonable unless we somehow can remember that we already configured the workspace and do not rerun it for every plugin. Besides if there is an error or warning for a workspace template the user will be annoyed if he gets the same message 20 times so it is not only about performance.
I really like to merge this PR finally but IMHO this is still not 100% cleared to the end.
Maybe we have to clarify this at the end of the next daily since review ping-pong seems to take way too long...
…-into-IDE- # Conflicts: # CHANGELOG.adoc
This PR fixes #1676
Implemented changes:
1. Register the IntelliJ template
2. Read extra tool installations
$IDE_HOME/settings/ide-extra-tools.json.For example:
3. Keep SDKs synchronized
ide createoride updateand before the IDE starts.4. Validate configuration and handle errors
java.5. Added GraalVM reflection metadata to reflect-config.json
IntellijviaIdeToolCommandlet-based lookup.Testing instructions
1. Build the native executable
IDEasy/cli/target.2. Create a test project with custom settings
IDEasy/cli/target, create a new test project using the modified settings from my test branch foride-settings.3. Verify automatic SDK import in IntelliJ
All additional SDKs should be automatically imported into IntelliJ.
This step validates that workspace configuration and IDE settings are correctly applied (as IDEasy manages IDE configuration via workspace templates).
4. Validate
ide updatebehavior4.1 Prepare test scenario
4.2 Run update
4.3 Verify result
All previously removed SDKs are re-downloaded and reinstalled.
IntelliJ configuration is updated accordingly, as shown at the end of step 3.
The file
$IDE_HOME/workspaces/main/.intellij/config/options/jdk.table.xmlhas been updated with the extra SDK entries.Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal