Add IDEA provisioning #529

6hundreds · 2024-01-15T09:05:12Z

This PR integrates ide-starter to Gradle profiler and provide some wrappers around it. In particular, some abstraction for IDE provisioning is introduced alongside with an implementation of IDEA provisioning.

Provisioning does include downloading of an IDE of desired version to a desired directory.

6hundreds · 2024-01-15T09:06:11Z

subprojects/ide-provisioning/build.gradle.kts

+}
+
+repositories {
+    maven { url = uri("https://www.jetbrains.com/intellij-repository/releases") }


These repos are subject to cleanup, I hope not all of them are required. It's just copy-paste from JetBrains example repo

❓Can we clean them now?

6hundreds · 2024-01-15T09:14:32Z

subprojects/ide-provisioning/src/main/java/org/gradle/profiler/ide/idea/IDEAProvider.java

+            info -> null
+        );
+
+        String version = ideInfo.getVersion().isEmpty()


Here and below own cache logic was implemented. Why:
IdeDownloader is able to skip downloading, but it's considering a presence of installer(like a .dmg) file.
However, I'm deleting an installer after unpacking of it, because it's quite big (1+ Gb).
At the same time, I don't want to download IDE each time. So simple heuristics was introduced - create new dir like ideaIC/latest/ and assuming that only one file (ide installation, in fact, like a .app) is there. If it there - skip downloading.

Doesn't idea starter do any caching?

It does. But it's relying on presence of installer(like a .dmg file). If we want to delete installers after unpacking(I guess we want), then we can't rely on it

6hundreds · 2024-01-15T09:17:44Z

subprojects/ide-provisioning/src/main/java/org/gradle/profiler/ide/Ide.java

+
+public interface Ide {
+    @NotNull
+    String getVersion();


I'm doubting what data to use for providing. From one perspective version is a good and simple declaration of which release of IDE we want to get. However, there is a buildNumber as well, and it's possible to have a IDE with same version with different buildNumbers(I thinks it's the case for nightlies)

We can always change that later, if we have a need to. So I think version is fine for now

6hundreds · 2024-01-15T09:19:15Z

subprojects/ide-provisioning/src/test/groovy/org/gradle/profiler/TeeOutputStream.java

+import java.util.Arrays;
+import java.util.List;
+
+public class TeeOutputStream extends OutputStream {


This was copy/pasted from gradle-profiler, but I think we should create some project with test fixtures or so. WDYT?

Yes, we could have some text-fixtures if we share the code

TeeOutputStream is a part of common-io, that was already included to gradle-profiler. So decided to remove own implementations and use common one

asodja · 2024-01-15T09:29:20Z

subprojects/ide-provisioning/build.gradle.kts

+
+java {
+    toolchain {
+        languageVersion.set(JavaLanguageVersion.of(17))


❓ Note: if we use Java 17 we won't be able to use it in gradle-profiler. Will we keep Java 17?

Huh, it seems like ide-starter was compiled using Java 17:
class file has wrong version 61.0, should be 52.0

The public API of ide-provisioning subproject is not contains any classes from ide-starter, so I think it should be possible somehow to configure toolchains in such a way, to use ide-starter with Java 17 internally, but compile ide-provisioning subproject using Java 8. Do you know how to achieve that?

The problem is not just compiling, but also running classes compiled with Java17 on JDK < 17. So then we would need gradle-profiler to require to run only on JDK >= 17 or at least require JDK >= 17 when doing sync profiling.

@asodja How to do it correctly?

Good question. I believe we still want to profile with gradle-profiler on JDK8 for most scenarions, but maybe we could soften that for sync profiling.

I guess we would need to:

Add a check when user tries to profile sync and they use JDK < 17 we fail

Add some interface to call IDE provisioning

Use multirelease jar or ServiceLoader to implement Java17 implementation

I think we need to put some more thoughts in to that

@asodja I think for now we can check JDK < 17 only if user enabled IDE auto-provisioning for sync. In future, when profiler will use ide-starter for sync triggering as well, we could think about it extend scope of that check, yes

@asodja Another "possible" option is fork ide-starter and build it from source using 8, right? But supporting a fork can be painful tho.

Checked out assemble of ide-starter with 8 - the main issue is using java.lang.ProcessHandle, which is was introduced in 9

I would not fork ide-starter, since that means extra work to maintain it as you mentioned. And I am sure we don't have resources for that.

Theoretically we could also just call that part of code with JDK >= 17 (so user would somehow provide a path to some JDK17 compatbile JDK), but I think having a requirement to use JDK17 for sync is ok.

I would maybe just ask on Slack if anyone has any concern regarding that (or possible if they see any other option).

asodja · 2024-01-15T09:30:19Z

subprojects/ide-provisioning/build.gradle.kts

+}
+
+repositories {
+    maven { url = uri("https://www.jetbrains.com/intellij-repository/releases") }


❓Can we clean them now?

asodja · 2024-01-15T09:30:54Z

subprojects/ide-provisioning/build.gradle.kts

+}
+
+dependencies {
+    implementation("com.jetbrains.intellij.tools:ide-starter-squashed:233.13135.103") {


💅 Maybe we can use version catalog for this one too

asodja · 2024-01-15T09:36:26Z

subprojects/ide-provisioning/src/test/groovy/org/gradle/profiler/TeeOutputStream.java

+import java.util.Arrays;
+import java.util.List;
+
+public class TeeOutputStream extends OutputStream {


Yes, we could have some text-fixtures if we share the code

asodja · 2024-01-15T09:37:32Z

subprojects/ide-provisioning/src/test/groovy/org/gradle/profiler/ide/IdeProviderTest.groovy

+
+        then:
+        outputContains("Downloading https://")
+        assert ide.exists()


❓ Do you need assert? If you setup Spock correctly then assert should not be needed here

asodja · 2024-01-15T09:46:10Z

subprojects/ide-provisioning/build.gradle.kts

+plugins {
+    id("profiler.embedded-library")
+}
+


❓How will we use and bundle it in the gradle-profiler? Also how do you plan to publish it in reuse in gradle/gradle.

I think this two things will require some additional work.

I thought about regular subproject usage as a dependency.

In fact, we don't need to reuse it in gradle/gradle directly. gradle/gradle will invoke profiler with dedicated options for auto-provisioning.

Signed-off-by: Sergey Opivalov <sopivalov@gradle.com>

6hundreds requested a review from asodja January 15, 2024 09:05

6hundreds self-assigned this Jan 15, 2024

6hundreds commented Jan 15, 2024

View reviewed changes

6hundreds added the enhancement label Jan 15, 2024

asodja reviewed Jan 15, 2024

View reviewed changes

6hundreds added 3 commits January 15, 2024 15:15

Add IDEA provisioning

e9cef14

Signed-off-by: Sergey Opivalov <sopivalov@gradle.com>

Remove explicit asserts in test

cd27166

Signed-off-by: Sergey Opivalov <sopivalov@gradle.com>

Use TeeOutputStream from common-io

cf92c32

Signed-off-by: Sergey Opivalov <sopivalov@gradle.com>

6hundreds force-pushed the IdeProvisioning branch from 30cd3c5 to cf92c32 Compare January 15, 2024 12:15

6hundreds mentioned this pull request Jan 26, 2024

IDEA sync smoke tests gradle/gradle#27838

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IDEA provisioning #529

Add IDEA provisioning #529

6hundreds commented Jan 15, 2024

6hundreds Jan 15, 2024

asodja Jan 15, 2024

6hundreds Jan 15, 2024

asodja Jan 15, 2024

6hundreds Jan 15, 2024

6hundreds Jan 15, 2024

asodja Jan 15, 2024 •

edited

6hundreds Jan 15, 2024

asodja Jan 15, 2024

6hundreds Jan 15, 2024

asodja Jan 15, 2024

6hundreds Jan 15, 2024

asodja Jan 15, 2024 •

edited

6hundreds Jan 15, 2024

asodja Jan 15, 2024

asodja Jan 15, 2024

6hundreds Jan 16, 2024

6hundreds Jan 16, 2024

6hundreds Jan 16, 2024

asodja Jan 16, 2024

asodja Jan 15, 2024

asodja Jan 15, 2024

asodja Jan 15, 2024

asodja Jan 15, 2024

asodja Jan 15, 2024

6hundreds Jan 15, 2024

Add IDEA provisioning #529

Are you sure you want to change the base?

Add IDEA provisioning #529

Conversation

6hundreds commented Jan 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asodja Jan 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asodja Jan 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asodja Jan 15, 2024 •

edited

asodja Jan 15, 2024 •

edited