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

WIP: Migrate to IntelliJ Platform Gradle Plugin 2.0 #879

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

citizenmatt
Copy link
Member

@citizenmatt citizenmatt commented May 9, 2024

This PR migrates to the new IntelliJ Platform Gradle Plugin 2.0 (beta2). This is necessary to target 242, but more importantly, allows us to use "split mode" to reproduce and debug remote development issues. It's also easier to configure, and makes it easy to run the plugin in a different IDE, even locally built or installed IDEs.

It is still a work in progress, and not yet ready to merge. I will be actively updating and force-pushing changes into this branch.

The following build features work as expected:

  • Build, run and debug the plugin, including loading the AceJump (optional) dependency
  • Build, run and debug :test, vim-engine:test, and java-tests:test
  • Separately run long-running-tests and property-tests (there are failures, but these match master)
  • Plugin verifier runs with a default IDE based on the configured settings (using the since-build value, so it's testing 233). The task name has changed from runPluginVerifier to verifyPlugin, so the "IdeaVim full verification" run configuration has been updated
  • The since-build value is specified in both build.gradle.kts and plugin.xml. It would be better to specify it just once, and have the DevKit and the plugin verifier automatically pick this up from a single source. The value currently doesn't match the compile time IntelliJ IDEA dependency (233.11799.241 vs IC 2024.1) and the plugin verifier warns about missing APIs
  • The createOpenApiSourceJar task fails while trying to get the sourceSets from the vim-engine module
  • UI tests aren't currently running. The download robot task is commented out, as it should be moved to a normal dependency. The UI test task is not yet supported by the gradle plugin.

Other changes:

  • settings.gradle has been migrated to settings.gradle.kts
  • The Gradle plugin writes various files to .intellijPlatform/ (Ivy XML files to reference bundled plugins, etc.) This folder is added to .gitignore
  • A new runIdeSplitMode task and equivalent "Start IJ with IdeaVim (Split Mode)" run configuration have been added, to run the IDE in split mode, with the plugin loaded in the frontend, as per remote dev/CWM.
  • Running split mode in the frontend requires 242+ at runtime, so the task currently has a hardcoded EAP version. A better strategy should be figured out.

There are also the following IdeaVim issues, which aren't required for this PR, but should be noted and fixed at some point:

  • The plugin distribution zip currently includes annotation-processors.jar, which is not required at run time
  • The since-build value is 233.11799.241, which is less than the IDE used as a build target (IC 2024.1). The plugin verifier shows a warning about this
  • The CodeQL GitHub action fails due to picking the wrong Java version to compile with. We could update the action, or wait for it to be fixed. Issue 1855 on the github/codeql-action says it's being actively worked on and should therefore be released soon (not linked directly to the issue to prevent an unnecessary pingback on the ticket)
  • There are currently 3 failing tests. This is unrelated to the Gradle plugin migration

// TODO: Robot server task not yet available
// downloadRobotServerPlugin {
// version.set(remoteRobotVersion)
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

What's supposed to happen with this task?

Copy link
Member

Choose a reason for hiding this comment

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

The downloadRobotServerPlugin is removed. The Robot Server Plugin will be added through the dependencies {} block.
ATM, the testIdeUI and testIdePerformance tasks are not available yet, but they will be there soon!

// systemProperty("jb.consents.confirmation.enabled", "false")
// systemProperty("ide.show.tips.on.startup.default.value", "false")
// systemProperty("octopus.handler", System.getProperty("octopus.handler") ?: true)
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

Also this task? I don't know how the current UI tests work...

Copy link
Member

Choose a reason for hiding this comment

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

WIP, tasks will be available soon.

// TODO: Do we need this to be here *and* in plugin.xml?
sinceBuild.set("233.11799.67")
untilBuild.set(provider { null })
}
Copy link
Member Author

Choose a reason for hiding this comment

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

If we set since-build here, will the Plugin DevKit use it for analysis? We have it duplicated in plugin.xml right now, it would be nice to only set in one place

Copy link
Member

Choose a reason for hiding this comment

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

You can set it in Gradle build file only. Those values are used by the patchPluginXml file to update the plugin.xml file. All further checks performed by the Gradle plugin respect those values.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about the DevKit though? Does it look at the patched or unpatched plugin.xml?

Copy link
Member Author

Choose a reason for hiding this comment

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

After some tests, it seems that DevKit only uses the value in the unpatched plugin.xml, so it's still worth keeping both values, even though it would make things easier to specify only in the build scripts

// TODO: This causes errors with sourceSets not be available for this project?
// from(project(":vim-engine").sourceSets.main.get().java) {
// include("**/com/maddyhome/idea/vim/**/*.java")
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

HELP! I have no idea why the build breaks when this is uncommented. The vim-engine module hasn't been modified at all, and this used to work with the old gradle plugin. I don't know what could be wrong.

Copy link
Member

Choose a reason for hiding this comment

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

This project(":vim-engine").sourceSets.main.get().java is basically wrong approach. I'll take a look at this soon.

instrumentationTools()

testFramework(TestFrameworkType.Platform.JUnit4)
testFramework(TestFrameworkType.Platform.JUnit5)
Copy link
Member Author

Choose a reason for hiding this comment

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

It seems unexpected to require two frameworks, but it fails with only one, and runs with two, so...

Copy link
Member

Choose a reason for hiding this comment

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

Fails how?

Copy link
Member Author

Choose a reason for hiding this comment

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

Either unresolved items (the @RunInEdt annotation) or runtime failures, also about the wrong threading models. Is it expected to require two frameworks?

Copy link
Member

Choose a reason for hiding this comment

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

It's unexpected. I'll take a look.

@citizenmatt citizenmatt force-pushed the feature/gradle-intellij-plugin-v2 branch 4 times, most recently from 1583038 to 7ea027b Compare May 15, 2024 09:22
@citizenmatt citizenmatt force-pushed the feature/gradle-intellij-plugin-v2 branch from 7ea027b to f274fff Compare May 15, 2024 09:47
@citizenmatt citizenmatt force-pushed the feature/gradle-intellij-plugin-v2 branch from f274fff to 5356fed Compare May 15, 2024 10:52
@citizenmatt citizenmatt force-pushed the feature/gradle-intellij-plugin-v2 branch from 5356fed to 2ec7a33 Compare May 23, 2024 16:00
@AlexPl292
Copy link
Member

As a not, we HAVE to use 2.0 to target 242: https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html#usage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants