-
Notifications
You must be signed in to change notification settings - Fork 156
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
Add IDEA provisioning #529
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
plugins { | ||
id("profiler.embedded-library") | ||
} | ||
|
||
java { | ||
toolchain { | ||
languageVersion.set(JavaLanguageVersion.of(17)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ Note: if we use Java 17 we won't be able to use it in gradle-profiler. Will we keep Java 17? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, it seems like The public API of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asodja How to do it correctly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to put some more thoughts in to that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asodja I think for now we can check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asodja Another "possible" option is fork There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checked out assemble of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not fork 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). |
||
} | ||
} | ||
|
||
repositories { | ||
maven { url = uri("https://www.jetbrains.com/intellij-repository/releases") } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These repos are subject to cleanup, I hope not all of them are required. It's just copy-paste from JetBrains example repo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓Can we clean them now? |
||
maven { url = uri("https://cache-redirector.jetbrains.com/intellij-dependencies") } | ||
} | ||
|
||
dependencies { | ||
implementation(libs.ideStarter) { | ||
exclude(group = "io.ktor") | ||
exclude(group = "com.jetbrains.infra") | ||
exclude(group = "com.jetbrains.intellij.remoteDev") | ||
} | ||
testImplementation(libs.bundles.testDependencies) | ||
testImplementation(libs.commonIo) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package org.gradle.profiler.ide; | ||
|
||
import kotlin.io.FilesKt; | ||
import org.gradle.profiler.ide.idea.IDEA; | ||
import org.gradle.profiler.ide.idea.IDEAProvider; | ||
|
||
import java.io.File; | ||
import java.nio.file.Path; | ||
|
||
public class DefaultIdeProvider implements IdeProvider<Ide> { | ||
|
||
private final IDEAProvider ideaProvider; | ||
|
||
public DefaultIdeProvider(IDEAProvider ideaProvider) { | ||
this.ideaProvider = ideaProvider; | ||
} | ||
|
||
@Override | ||
public File provideIde(Ide ide, Path homeDir, Path downloadsDir) { | ||
File result; | ||
if (ide instanceof IDEA) { | ||
result = ideaProvider.provideIde((IDEA) ide, homeDir, downloadsDir); | ||
} else { | ||
throw new IllegalArgumentException("Unknown IDE to provide"); | ||
} | ||
|
||
cleanup(homeDir, downloadsDir); | ||
return result; | ||
} | ||
|
||
private void cleanup(Path homeDir, Path downloadsDir) { | ||
FilesKt.deleteRecursively(downloadsDir.toFile()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package org.gradle.profiler.ide; | ||
|
||
import org.jetbrains.annotations.NotNull; | ||
|
||
public interface Ide { | ||
@NotNull | ||
String getVersion(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm doubting what data to use for providing. From one perspective There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can always change that later, if we have a need to. So I think version is fine for now |
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package org.gradle.profiler.ide; | ||
|
||
import java.io.File; | ||
import java.nio.file.Path; | ||
|
||
public interface IdeProvider<T extends Ide> { | ||
|
||
File provideIde(T ide, Path homeDir, Path downloadsDir); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package org.gradle.profiler.ide.idea; | ||
|
||
import org.gradle.profiler.ide.Ide; | ||
import org.jetbrains.annotations.NotNull; | ||
|
||
public class IDEA implements Ide { | ||
public static IDEA LATEST = new IDEA(""); | ||
private final String version; | ||
public IDEA(String version) { | ||
this.version = version; | ||
} | ||
|
||
@NotNull | ||
@Override | ||
public String getVersion() { | ||
return version; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package org.gradle.profiler.ide.idea; | ||
|
||
import com.intellij.ide.starter.community.PublicIdeDownloader; | ||
import com.intellij.ide.starter.community.model.BuildType; | ||
import com.intellij.ide.starter.ide.IdeArchiveExtractor; | ||
import com.intellij.ide.starter.ide.IdeDownloader; | ||
import com.intellij.ide.starter.ide.installer.IdeInstallerFile; | ||
import com.intellij.ide.starter.models.IdeInfo; | ||
import org.gradle.profiler.ide.IdeProvider; | ||
import org.jetbrains.annotations.Nullable; | ||
|
||
import java.io.File; | ||
import java.nio.file.Path; | ||
import java.util.Collections; | ||
|
||
public class IDEAProvider implements IdeProvider<IDEA> { | ||
private final IdeDownloader ideDownloader; | ||
private final IdeArchiveExtractor ideArchiveExtractor; | ||
|
||
public IDEAProvider() { | ||
this.ideDownloader = PublicIdeDownloader.INSTANCE; | ||
this.ideArchiveExtractor = IdeArchiveExtractor.INSTANCE; | ||
} | ||
|
||
@Override | ||
public File provideIde(IDEA ide, Path homeDir, Path downloadsDir) { | ||
IdeInfo ideInfo = new IdeInfo( | ||
"IC", | ||
"Idea", | ||
"idea", | ||
BuildType.EAP.getType(), | ||
Collections.emptyList(), | ||
"", // latest | ||
ide.getVersion(), | ||
null, | ||
null, | ||
"IDEA Community", | ||
info -> null | ||
); | ||
|
||
String version = ideInfo.getVersion().isEmpty() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and below own cache logic was implemented. Why: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't idea starter do any caching? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does. But it's relying on presence of installer(like a |
||
? "latest" | ||
: ideInfo.getVersion(); | ||
|
||
File unpackDir = homeDir | ||
.resolve(ideInfo.getInstallerFilePrefix()) | ||
.resolve(version) | ||
.toFile(); | ||
|
||
File unpackedIde = getUnpackedIde(unpackDir); | ||
|
||
if (unpackedIde != null) { | ||
System.out.println("Downloading is skipped, get " + ideInfo.getFullName() + " from cache"); | ||
return unpackedIde; | ||
} | ||
|
||
IdeInstallerFile installerFile = ideDownloader.downloadIdeInstaller(ideInfo, downloadsDir); | ||
ideArchiveExtractor.unpackIdeIfNeeded(installerFile.getInstallerFile().toFile(), unpackDir); | ||
|
||
return unpackDir.listFiles()[0]; | ||
} | ||
|
||
@Nullable | ||
private static File getUnpackedIde(File unpackDir) { | ||
File[] unpackedFiles = unpackDir.listFiles(); | ||
if (unpackedFiles == null || unpackedFiles.length == 0) { | ||
return null; | ||
} | ||
if (unpackedFiles.length == 1) { | ||
return unpackedFiles[0]; | ||
} | ||
throw new IllegalStateException("Unexpected content in " + unpackDir); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package org.gradle.profiler.ide | ||
|
||
import org.apache.commons.io.output.TeeOutputStream | ||
import org.junit.Rule | ||
import org.junit.rules.TemporaryFolder | ||
import spock.lang.Specification | ||
|
||
class AbstractIdeProvisioningTest extends Specification { | ||
|
||
private ByteArrayOutputStream outputBuffer | ||
|
||
@Rule | ||
protected TemporaryFolder tmpDir = new TemporaryFolder(new File("build/tmp/")) | ||
|
||
def setup() { | ||
outputBuffer = new ByteArrayOutputStream() | ||
System.out = new PrintStream(new TeeOutputStream(System.out, outputBuffer)) | ||
} | ||
|
||
protected void outputContains(String content) { | ||
assert getOutput().contains(content.trim()) | ||
} | ||
|
||
protected String getOutput() { | ||
System.out.flush() | ||
return new String(outputBuffer.toByteArray()) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package org.gradle.profiler.ide | ||
|
||
|
||
import org.gradle.profiler.ide.idea.IDEA | ||
import org.gradle.profiler.ide.idea.IDEAProvider | ||
|
||
class IdeProviderTest extends AbstractIdeProvisioningTest { | ||
|
||
def "can provide IDEA"() { | ||
given: | ||
def workDir = tmpDir.newFolder().toPath().toAbsolutePath() | ||
def downloadsDir = workDir.resolve("downloads") | ||
def ideHomeDir = workDir.resolve("ide") | ||
def ideProvider = new DefaultIdeProvider(new IDEAProvider()) | ||
|
||
when: | ||
def ide = ideProvider.provideIde(IDEA.LATEST, ideHomeDir, downloadsDir) | ||
|
||
then: | ||
outputContains("Downloading https://") | ||
ide.exists() | ||
|
||
when: | ||
def ide2 = ideProvider.provideIde(IDEA.LATEST, ideHomeDir, downloadsDir) | ||
|
||
then: | ||
outputContains("Downloading is skipped, get IDEA Community from cache") | ||
ide == ide2 | ||
|
||
and: | ||
!downloadsDir.toFile().exists() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gradle/gradle
directly.gradle/gradle
will invoke profiler with dedicated options for auto-provisioning.