feat: Gradle 2.x plugin SDK-based build #761
Conversation
|
@SCWells72 thanks for this new one ! i'm looking at it right now. |
There was a problem hiding this comment.
don't think this file is necessary.
where it's coming from ?
There was a problem hiding this comment.
We usually don't push our gradle run configurations. please add a gitignore rule for them.
There was a problem hiding this comment.
This file is also new. as it's linked with legals, we should request our legal team to review it.
i suggest to simply remove it from now.
| import org.jetbrains.intellij.tasks.RunPluginVerifierTask.VerificationReportsFormats.* | ||
| import org.jetbrains.intellij.tasks.RunPluginVerifierTask.FailureLevel.* | ||
|
|
||
| fun properties(key: String) = providers.gradleProperty(key) |
There was a problem hiding this comment.
please leave those 3 function here, for history reasons ( still used below )
| plugins { | ||
| java // Java support |
There was a problem hiding this comment.
no need to change this one, this is strictly equivalent.
| alias(libs.plugins.changelog) // Gradle Changelog Plugin | ||
| alias(libs.plugins.testLogger) // Nice test logs |
There was a problem hiding this comment.
please keep testlogger plugin to provide nice tests output ( diffcult to read without it ) .
also, remove qodana as it's not used by our extensio nto review idea inspections.
| alias(libs.plugins.kover) // Gradle Kover Plugin | ||
| jacoco // Code coverage |
There was a problem hiding this comment.
@angelozerr @fbricon : will it be a sonarqube review in the future for lsp4ij ? is yes, jacoco is needed.
if not, this can be removed.
| } | ||
|
|
||
| group = prop("pluginGroup") |
There was a problem hiding this comment.
revert to use above functions..
|
|
||
| val lsp4jVersion = prop("lsp4jVersion") | ||
| val flexmarkVersion = prop("flexmarkVersion") |
There was a problem hiding this comment.
please also revert those versions that are still used the same way.
|
|
||
| // Configure project's dependencies | ||
| repositories { | ||
| mavenLocal() |
There was a problem hiding this comment.
ok this one is tricky, but the way we declare repositories are important. first we use mavenCentral to resolve our snapshot, because we can install locally-built third parties for example.
we had issues in the past not doing so.
| defaultRepositories() | ||
| } | ||
|
|
||
| mavenLocal() | ||
| maven { url = uri("https://repository.jboss.org/nexus/content/repositories/snapshots") } | ||
| maven { url = uri("https://repository.jboss.org/nexus/content/groups/public") } | ||
| maven { url = uri("https://packages.jetbrains.team/maven/p/ij/intellij-dependencies") } |
There was a problem hiding this comment.
this line can be safely removed because it's actually included in the default intellij platform repository declaration above.
| val lsp: Configuration by configurations.creating | ||
| // Dependencies are managed with Gradle version catalog - read more: https://docs.gradle.org/current/userguide/platforms.html#sub:version-catalog | ||
| dependencies { | ||
| testImplementation(libs.junit) |
There was a problem hiding this comment.
please move this to the bottom along with other testimplementations
| compileClasspath += sourceSets.main.get().output | ||
| compileClasspath += configurations.testImplementation.get() | ||
| runtimeClasspath += compileClasspath + sourceSets.test.get().output | ||
| instrumentationTools() |
There was a problem hiding this comment.
when runnig locally a build, there is a warning about this usage :
w: file:///home/sbouchet/IdeaProjects/lsp4ij/build.gradle.kts:51:9: 'instrumentationTools(): Unit' is deprecated. Calling instrumentationTools() is no longer necessary.
|
@sbouchet apologies if I didn't explain the purpose of this PR clearly. It's not intended for direct merge. It's really a proof-of-concept for the migration to a Gradle 2.x-based build for whoever will be doing that work. Anything that can be used from it, please do so. But I don't have the ability to verify things like the plugin publishing process and such so someone who does will need to take it from here. Again, sorry if that wasn't clear. |
| dependencies { | ||
| implementation("org.zeroturnaround:zt-zip:1.14") | ||
| implementation("org.jsoup:jsoup:1.17.1") | ||
| val lsp4jVersion = providers.gradleProperty("lsp4jVersion").get() |
There was a problem hiding this comment.
please revert this to use previous identical mechanism.
| // Required by lsp4j as the version from IJ is incompatible | ||
| implementation("com.google.code.gson:gson:2.10.1") | ||
|
|
||
| val flexmarkVersion = providers.gradleProperty("flexmarkVersion").get() |
| // Plugin Dependencies. Uses `platformPlugins` property from the gradle.properties file. | ||
| plugins = properties("platformPlugins").map { it.split(',').map(String::trim).filter(String::isNotEmpty) } | ||
| } | ||
| // Extract the <!-- Plugin description --> section from README.md and provide for the plugin's manifest |
There was a problem hiding this comment.
@angelozerr @fbricon below is an addition to use README and CHANGELOG files contents to populate plugin description, made manually for now. is it something we want to do ?
| isDownloadSources = true | ||
| ideaVersion { | ||
| sinceBuild = providers.gradleProperty("pluginSinceBuild") | ||
| untilBuild = providers.gradleProperty("pluginUntilBuild") |
There was a problem hiding this comment.
there is no such property. please use
untilBuild = provider { null }
| testImplementation { | ||
| isCanBeResolved = true | ||
|
|
||
| publishing { |
There was a problem hiding this comment.
please revert to:
publishPlugin {
token = environment("PUBLISH_TOKEN")
channels = properties("channel").map { listOf(it) }
}
| runtimeClasspath { | ||
| exclude(group = "org.slf4j", module = "slf4j-api") | ||
| signing { | ||
| certificateChain = providers.environmentVariable("CERTIFICATE_CHAIN") |
There was a problem hiding this comment.
please revert to:
signPlugin {
certificateChain = environment("CERTIFICATE_CHAIN")
privateKey = environment("PRIVATE_KEY")
password = environment("PRIVATE_KEY_PASSWORD")
}
| showFailedStandardStreams = true | ||
| logLevel = LogLevel.LIFECYCLE | ||
| pluginVerification { | ||
| ides { |
There was a problem hiding this comment.
please revert to:
failureLevel = listOf(INVALID_PLUGIN, COMPATIBILITY_PROBLEMS, MISSING_DEPENDENCIES )
verificationReportsFormats = listOf(MARKDOWN, HTML)
ides {
recommended()
}
also, the imports will need to be
import org.jetbrains.intellij.platform.gradle.tasks.VerifyPluginTask.FailureLevel.*
import org.jetbrains.intellij.platform.gradle.tasks.VerifyPluginTask.VerificationReportsFormats.*
| repositoryUrl = properties("pluginRepositoryUrl") | ||
| } | ||
|
|
||
| tasks.withType<Test> { |
There was a problem hiding this comment.
please revert this block ( because of the system property to disable telemetry )
| runPluginVerifier { | ||
| failureLevel = listOf(INVALID_PLUGIN, COMPATIBILITY_PROBLEMS, MISSING_DEPENDENCIES ) | ||
| verificationReportsFormats = listOf(MARKDOWN, HTML) | ||
| gradleVersion = providers.gradleProperty("gradleVersion").get() |
There was a problem hiding this comment.
please revert to previous same call
| untilBuild = properties("pluginUntilBuild") | ||
| //TODO inject changelog into plugin.xml change-notes | ||
| publishPlugin { | ||
| dependsOn(patchChangelog) |
There was a problem hiding this comment.
see above for generating changelog automatically.
| # IntelliJ Platform Properties -> https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html#configuration-intellij-extension | ||
| platformType=IC | ||
| platformVersion=2023.2 | ||
|
|
There was a problem hiding this comment.
for now, please still use the same version, then we'ell update to some new version later on.
| # Gradle Releases -> https://github.com/gradle/gradle/releases | ||
| gradleVersion=8.5 | ||
| gradleVersion=8.10.2 |
There was a problem hiding this comment.
same as above, please still use gradle 8.5 ( supported with intellij gradle plugin 2.X )
| kover = { id = "org.jetbrains.kotlinx.kover", version.ref = "kover" } | ||
| qodana = { id = "org.jetbrains.qodana", version.ref = "qodana" } |
There was a problem hiding this comment.
please fix acordingly with gradle.kts file.
| } | ||
|
|
||
| rootProject.name = "lsp4ij" | ||
| rootProject.name = "LSP4IJ" |
There was a problem hiding this comment.
no need to capitalize. please revert
| @@ -176,7 +176,7 @@ L | |||
| <depends optional="true">com.redhat.devtools.intellij.telemetry</depends> | |||
| <depends optional="true" config-file="plugin-json.xml">com.intellij.modules.json</depends> | |||
| <!-- please see http://www.jetbrains.org/intellij/sdk/docs/basics/getting_started/build_number_ranges.html for description --> | |||
| <idea-version since-build="232.*"/> | |||
| <idea-version since-build="233.*"/> | |||
There was a problem hiding this comment.
is there a need to update this version ?
|
@SCWells72 nice work ! sorry i did lots of comments but we will try at first to build with the same gradle and target versions, to check that the produced contents are identical. also, for the remaning workflows, there is some changes that need to be accomplished: |
Hi, @sbouchet. I tried to shoot you a note much earlier in this review but it doesn't look like you saw it: As stated there, I'm not planning to take this PR forward myself. Apologies for any confusion around that. I should have opened this as a draft PR and made that more clear. But of course the review feedback is mostly stuff that needs to happen to get this ready anyway. I say "mostly" because there are a few things that you commented on that warrant some additional explanation:
I think those were the main things. Hopefully that helps, and please let me know if you have any other questions or concerns. Otherwise I'll turn this over to you or whoever else will be doing this actual switch. |
|
Thanks for the explanations, i'll work on this early next week, probably by forking your PR. |
|
Superseded by #783. |
This updates the build to be based on the Gradle 2.x plugin SDK. The
buildPlugin,test, andrunIdetasks all run perfectly, but the custom release process -- and perhaps some other functionality -- will need to restored for this to have full parity with the current Gradle 1.x-based build.This was accomplished by creating a brand new plugin project from the template as a peer to the existing one and migrating from the current project into the new one until the aforementioned Gradle tasks ran successfully, then updating the original project with those changes resulting in the branch in this PR.
@angelozerr and @sbouchet, this should be a viable foundation upon which to restore the missing functionality and get LSP4IJ moved into a current plugin build model.