Fix projects that failed to build within Intellij #62258

jakelandis · 2020-09-10T21:30:17Z

This commit address some build failures from the perspective of Intellij.
These changes include:

changing an order of a dependency definition that seems to can cause Intellij build to fail.
introduction of an abstract class out of the test source set (seems to be an issue sharing
classes cross projects with non-standard source sets.
a couple of missing dependency definitions (not sure how the command line worked prior to this)

This has been tested to work with
-> a clean checkout (clean -xdf) -> restart and invalidate cache -> rebuild project and no errors.

jakelandis · 2020-09-10T21:49:27Z

x-pack/plugin/src/test/java/org/elasticsearch/xpack/test/rest/AbstractXPackRestTest.java

+/** Runs rest tests against external cluster */
+// TODO: Remove this timeout increase once this test suite is broken up
+@TimeoutSuite(millis = 60 * TimeUnits.MINUTE)
+public class AbstractXPackRestTest extends ESClientYamlSuiteTestCase {


There are not any actual changes to this file...just renamed and moved back to the test source set.

I think this change this makes sense since independent of any other reason since there are a few runners that extend this... however the real reason i am introducing this is that I could not figure out how to get Intellij to share classes x-project and x-non-standard source set...it works fine from the test sourceset, but when shared from the custom source set (yamlRestTest) Intellij seems to have issues (the command line works fine).

I believe that's due to setting up the source sets correctly. This should work, and we do it in other places.

I believe that's due to setting up the source sets correctly. This should work, and we do it in other places.

I'm not sure I follow the comment... I tried a variety of ways to get Intellij to honor the configuration, it always works when defined in the test sourceset , but fails with identical config with a different sourceset name. I am sure I am missing something (or there is a bug in intellij), but not sure what. I don't believe it is related to the GradleUtils comment below since it is a different failure that addresses. Is there something specific you would like me to try ? I think this changes as-is actually a net positive change, with the benefit that it does not show red squiggly lines in Intellij.

jakelandis · 2020-09-10T21:51:22Z

x-pack/plugin/build.gradle

  from sourceSets.test.output
-  from sourceSets.yamlRestTest.output
-  from sourceSets.javaRestTest.output


I am pretty sure the only reason we expose this is for the yaml tests and the custom parent runner, both of which are (now) in the test source set...so only expose that in the jar and testArtifact config.

jakelandis · 2020-09-10T21:52:50Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

@@ -155,7 +155,7 @@ public static void setupIdeForTestSourceSet(Project project, SourceSet testSourc
        project.getPluginManager().withPlugin("idea", p -> {
            IdeaModel idea = project.getExtensions().getByType(IdeaModel.class);
            idea.getModule().setTestSourceDirs(testSourceSet.getJava().getSrcDirs());
-            idea.getModule().getScopes().put("TEST", Map.of("plus", List.of(runtimeClasspathConfiguration)));
+            idea.getModule().getScopes().put(testSourceSet.getName(), Map.of("plus", List.of(runtimeClasspathConfiguration)));


this doesn't seem to have any functional impact, just noticed I missed this before and I think it is a bit more correct.

jakelandis · 2020-09-10T21:56:10Z

x-pack/plugin/ml/qa/ml-with-security/build.gradle

@@ -1,8 +1,8 @@
 apply plugin: 'elasticsearch.yaml-rest-test'

 dependencies {
-  yamlRestTestImplementation project(path: xpackModule('core'), configuration: 'testArtifacts')
  yamlRestTestImplementation project(path: xpackModule('core'))


wierd dependency ordering thing...likely a subtle in Intellij

jakelandis · 2020-09-10T23:07:06Z

cc: @javanna @williamrandolph

mark-vieira · 2020-09-10T23:46:50Z

x-pack/plugin/ilm/qa/multi-node/build.gradle

@@ -2,6 +2,8 @@ apply plugin: 'elasticsearch.java-rest-test'

 dependencies {
  javaRestTestImplementation project(path: xpackProject('plugin').path, configuration: 'testArtifacts')
+  // let the javaRestTest see the classpath of main
+  javaRestTestImplementation project.sourceSets.main.runtimeClasspath


This might be why intellij gets confused. Instead we should add sourcets.main.runtimeClasspath to sourcesets.javaRestTest.runtimeClasspath rather than wire this up as a filecollection dependency. Basically, we should use GradleUtils.extendSourceSet() here if we need javaRestTest to "extend from" main.

relevant prior comment: https://github.com/elastic/elasticsearch/pull/59444/files/c89b5de16f777a00a7be76280565debd6014728f#diff-3f6ea6fd97c3537cc14d8d9f622aac7b

Are there any concerns with using GradleUtils here (and about 5 other places) ? Concerns with example project different from rest of the projects?

Yes, since example projects are meant for external plugin authors to use as a baseline for their own projects. Internal use is not so much an issue.

done 87c2493

jakelandis · 2020-09-15T01:15:55Z

@mark-vieira - ready for another pass. I had to add a method to GradleUtils that will re-load the classpath for a test task since these RestIntegTestTask's are eagerly created (as opposed to lazily like the internalClusterTests). d2aceef

mark-vieira

👍

jakelandis · 2020-09-15T17:51:39Z

Thanks @mark-vieira !

elasticmachine · 2020-09-15T17:52:49Z

Pinging @elastic/es-core-infra (:Core/Infra/Build)

This commit address some build failures from the perspective of Intellij. These changes include: * changing an order of a dependency definition that seems to can cause Intellij build to fail. * introduction of an abstract class out of the test source set (seems to be an issue sharing classes cross projects with non-standard source sets. * a couple of missing dependency definitions (not sure how the command line worked prior to this) # Conflicts: # x-pack/qa/security-example-spi-extension/build.gradle

williamrandolph · 2020-09-15T19:06:27Z

Thanks for this! I had been getting the build error, but this fixes the problem for me.

) This commit address some build failures from the perspective of Intellij. These changes include: * changing an order of a dependency definition that seems to can cause Intellij build to fail. * introduction of an abstract class out of the test source set (seems to be an issue sharing classes cross projects with non-standard source sets. * a couple of missing dependency definitions (not sure how the command line worked prior to this)

jakelandis added 8 commits September 10, 2020 13:25

iniitial fix

eabcb42

xperamint

933852c

move xpack rest test back to test

b0b5b36

minor typo fix

e71f397

fix x-pack rest testss

a273b94

fix over reaching intellij refactor

0c952a0

fix name

6760bfa

attempt to preserve git history and fix jar hell

e6b3090

jakelandis commented Sep 10, 2020

View reviewed changes

jakelandis changed the title ~~Fix intellij build~~ Fix projects that failed to build within Intellij Sep 10, 2020

jakelandis marked this pull request as ready for review September 10, 2020 23:05

jakelandis requested a review from mark-vieira September 10, 2020 23:05

mark-vieira requested changes Sep 10, 2020

View reviewed changes

jakelandis added 4 commits September 14, 2020 15:59

use GradleUtils.extendSourceSet

87c2493

add support to extend eager test task classpaths

d2aceef

undo example

61f288d

Merge remote-tracking branch 'upstream/master' into fix_intellij_build

54b3373

mark-vieira approved these changes Sep 15, 2020

View reviewed changes

jakelandis merged commit 86660b0 into elastic:master Sep 15, 2020

jakelandis deleted the fix_intellij_build branch September 15, 2020 17:52

jakelandis added :Delivery/Build Build or test infrastructure >non-issue v7.10.0 v8.0.0 labels Sep 15, 2020

elasticmachine added the Team:Core/Infra Meta label for core/infra team label Sep 15, 2020

jakelandis added the backport pending label Sep 15, 2020

jakelandis mentioned this pull request Sep 15, 2020

[7.x] Fix projects that failed to build within Intellij (#62258) #62408

Merged

mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020

jakelandis removed the backport pending label Mar 2, 2021

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix projects that failed to build within Intellij #62258

Fix projects that failed to build within Intellij #62258

jakelandis commented Sep 10, 2020 •

edited

Loading

jakelandis Sep 10, 2020

mark-vieira Sep 10, 2020

jakelandis Sep 14, 2020

jakelandis Sep 10, 2020

jakelandis Sep 10, 2020

jakelandis Sep 10, 2020

jakelandis commented Sep 10, 2020

mark-vieira Sep 10, 2020

jakelandis Sep 14, 2020

mark-vieira Sep 14, 2020

jakelandis Sep 14, 2020

jakelandis commented Sep 15, 2020

mark-vieira left a comment

jakelandis commented Sep 15, 2020

elasticmachine commented Sep 15, 2020

williamrandolph commented Sep 15, 2020

Fix projects that failed to build within Intellij #62258

Fix projects that failed to build within Intellij #62258

Conversation

jakelandis commented Sep 10, 2020 • edited Loading

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

jakelandis commented Sep 10, 2020

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

jakelandis commented Sep 15, 2020

mark-vieira left a comment

Choose a reason for hiding this comment

jakelandis commented Sep 15, 2020

elasticmachine commented Sep 15, 2020

williamrandolph commented Sep 15, 2020

jakelandis commented Sep 10, 2020 •

edited

Loading