#1676: import extra sdks automatically into ide - #2320
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-
…med attribute placeholders in jdk-extra-java.xml
…-into-IDE- # Conflicts: # CHANGELOG.adoc
…atically-into-IDE-
…atically-into-IDE-
…elliJ sees available SDKs
…-into-IDE- # Conflicts: # CHANGELOG.adoc
…obally but where missing when workspace got configured
Coverage Report for CI Build 31963985344Warning No base build found for commit Coverage: 72.993%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats💛 - Coveralls |
- review requests
|
|
||
| // Synchronize extra tools into all IDE workspaces (always run, even if extraTools is null - | ||
| // synchronizeExtraToolInstallations() handles the no-op case internally) | ||
| for (ToolCommandlet toolCommandlet : toolCommandlets) { | ||
| if (toolCommandlet instanceof IdeToolCommandlet ideToolCommandlet) { | ||
| ideToolCommandlet.configureWorkspace(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I suggest this solution for your comment @hohwille: #2073 (comment)
There was a problem hiding this comment.
This will still not fix the bug:
Assume you create a new project and have intellij in IDE_TOOLS variable then intellij will still be installed with its plugins before the workspace is configured.
Also users can manually add intellij as I showed in my testing instructions how I reproduced this bug.
Then also this will not fix the problem.
A real fix would be to ensure that we create idea.properties before we install the plugins. Currently this only happens inside configureWorkspace() and that only happens when the IDE is launched.
That causes the bug:
- install will install IDE with the plugins but due to missing
idea.propertiesin the wrong location - launching IDE configures the workspace and relocates the plugins directory via
idea.propertiesand then finds no plugins.
I hope now everything is clear.
There was a problem hiding this comment.
I think this fix addresses a different issue.
The change in AbstractUpdateCommandlet is intended for the update scenario, e.g. when IntelliJ is already running and an update synchronizes extra SDKs into an existing workspace. In this case, users do not need to restart IntelliJ via ide intellij, but can simply restart it from within IntelliJ.
The issue you describe regarding plugin installation during the initial IDE installation should rather be addressed by the change discussed here: #2320 (comment)
My intention there was to ensure that configureWorkspace() is executed before plugins are installed, so that idea.properties already exists and points to the correct plugin directory.
That said, I totally agree with your OOP concern. Having PluginBasedCommandlet know about IdeToolCommandlet via instanceof is not a clean design, and your suggested postInstall() approach would be preferable.
There was a problem hiding this comment.
I would still question this added loop.
The workspace configuration will be updated whenever we launch Intellij for the next time.
What will actually happen, if both the end-user changes preferences and therefore IntelliJ overwrites some workspace config file while also the end-user launched ide update in some terminal?
However, I now understand your change with postInstall() that will also trigger the workspace config update.
This is indeed a fix for what I was requesting for.
In the end, I want to summarise our main problems we were facing:
- In order to install plugins, we already need to have
idea.propertiespopulated for the workspace as otherwise the plugins are installed into the wrong location and do not apply when Intellij is launched. - If we configure the workspace during the tool installation then on use-cases like
ide createthe workspace config can be wrong because the referenced extra tools may not yet installed. For that reason we actually wanted that the workspace configuration does not get triggered during the installation but only when the IDE gets started. This again conflicted with 1.
IMHO the cleanest approach would be to somehow extract the creation of the Jetbrains specific IDEA properties file always (when installed and when launched) but do the rest of the workspace configuration only when the IDE gets launched. This is also specific only for IdeaBasedIdeToolCommandlet and its subclasses, so all tweaks we consider here do not affect e.g. Eclipse or VSCode (where it already works if we do not configure the workspace during installation).
The thing is that our workspace configuration is solved fully generic and we decided to reuse that generic process also for these properties. Example:
https://github.com/devonfw/ide-settings/blob/main/intellij/workspace/update/idea.properties
I would now like question this decision: Did we take a bad choice that we should change?
In IdeaBasedIdeToolCommandlet we already have custom logic for composing an VM_OPTIONs variable depending on the IDE type (intellij, android-studio, pycharm).
In the same way, we could create this properties file here explicitly and remove it from the settings.
Using the generic approach with the idea.properties as workspace template also gives some flexibility.
However, in this specific case I would rather think that this is more an implementation detail of IDEasy than something that the end-user should have arbitrary control over because in most cases, he will only break the proper IDE integration. We would then also have to clarify if we somehow want to ignore this specific properties template in the future when IDEasy is changed accordingly and only render a warning.
So to come to an end we should:
- agree if we want to keep or remove this extra loop on workspace configuration during
ide update/ide create. I currently would vote for removing it but am still open to keep it if it holds my challenging questions. - create a new issue of my current concern and finally decide what we want to do about this but get this PR finally done and merged and consider this latest discussion as a new feature to be addressed in a separate PR.
There was a problem hiding this comment.
- I removed the additional workspace configuration loop again.
IMHO, it is acceptable that after an ide update which installs or updates extra SDKs, the user may need to close IntelliJ and start it again via ide intellij in order to pick up the updated configuration.
Given the concerns raised about running workspace configuration during ide update and ide create, I think removing the additional loop is the safer approach for now.
- I have create a separate research issue: Reconsider treating idea.properties as a user-configurable workspace template #2336 to trach the architectural discussion around
idea.properties
The issue covers the questions raised in this discussion, especially whether idea.properties should remain part of the settings repository and generic workspace configuration process, or whether it should instead become an implementation detail of IdeaBasedIdeToolCommandlet.
Since there is no clear decision yet on the preferred long-term approach, I created it as a research task so the topic can be evaluated independently from this PR.
- review requests
- removed configureworkspace loop in abstractUpdateCommandlet
…-extra-SDKs-automatically-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