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

Add option to use IntelliJ compiler for Pants projects #447

Merged
merged 47 commits into from
Dec 12, 2019

Conversation

wisechengyi
Copy link
Collaborator

@wisechengyi wisechengyi commented Nov 27, 2019

Problem

Often times shelling off to Pants has non trivial latency. This PR introduces using IntelliJ compiler for an imported Pants project to achieve faster turnaround time.

Visible changes includes:

  1. Add a checkbox at import time for IntelliJ Compiler
    Screen Shot 2019-12-12 at 10 28 38 AM

Once selected, all the test configurations generated will invoke the default Build action as below
Screen Shot 2019-12-05 at 9 29 06 AM

instead of PantsCompile
Screen Shot 2019-12-05 at 9 28 58 AM

  1. No longer hide the default Build Project button.
    Screen Shot 2019-12-05 at 9 40 08 AM

  2. Module content roots are further trimmed to the top level roots, so intellij won't double count the sources as it gathers all the sources for each content root.

Before

In this case, WelSpecXYZ will be passed to the compiler twice, causing errors.
Screen Shot 2019-12-05 at 9 50 30 AM

After

Screen Shot 2019-12-05 at 9 51 26 AM

@wisechengyi wisechengyi changed the title Intellij compiler Add option to use IntelliJ compiler for Pants project Dec 5, 2019
@wisechengyi wisechengyi changed the title Add option to use IntelliJ compiler for Pants project Add option to use IntelliJ compiler for Pants projects Dec 5, 2019
@wisechengyi
Copy link
Collaborator Author

TODO: add and fix some tests.

@wisechengyi wisechengyi marked this pull request as ready for review December 10, 2019 23:16
Copy link
Contributor

@blorente blorente left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! This is looking great!

@@ -112,20 +106,24 @@ else if (runConfiguration instanceof CommonProgramRunConfigurationParameters) {
((CommonProgramRunConfigurationParameters) runConfiguration).setWorkingDirectory(buildRoot.get().getPath());
}
}
}

public static void replaceDefaultMakeWithPantsMake(@NotNull RunConfiguration runConfiguration) {
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover comment line

@@ -77,6 +79,12 @@ protected void fillExtraControls(@NotNull PaintAwarePanel content, int indentLev
myUseIdeaProjectJdkCheckBox.setSelected(mySettings.useIdeaProjectJdk);
myImportSourceDepsAsJarsCheckBox.setSelected(mySettings.importSourceDepsAsJars);
myUseIntellijCompilerCheckBox.setSelected(mySettings.useIntellijCompiler);
LinkLabel<?> intellijCompilerHelpMessage = LinkLabel.create(PantsBundle.message("pants.settings.text.use.intellij.compiler.help.messasge"), new Runnable() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't quite see this message in the UI, it doesn't show up. Where should I be looking?

Copy link
Collaborator Author

@wisechengyi wisechengyi Dec 12, 2019

Choose a reason for hiding this comment

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

Updated the description under 1). It now shows up at import time.

@@ -76,6 +76,10 @@ public boolean isEnableIncrementalImport() {
return getLinkedProjectsSettings().stream().anyMatch(PantsProjectSettings::isEnableIncrementalImport);
}

public boolean isUseIntellijCompiler() {
return getLinkedProjectsSettings().stream().anyMatch(s -> s.useIntellijCompiler);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that if any of my imported projects use the intellij compiler, all of them have to? I'm not sure what getLinkedProjectSettings does.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One IntelliJ window can contain multiple projects actually. So yes, that's what this means.

This mostly just follows the pattern there. and the tricky part is that if a source file can belong to multiple projects, then which compiler should we use? I think this should be an okay heuristics.

@wisechengyi wisechengyi merged commit 7b7c13f into pantsbuild:master Dec 12, 2019
@wisechengyi wisechengyi deleted the intellij_compiler branch December 12, 2019 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants