Skip to content

Fix eclipse issues related to rest client shading#25874

Merged
hub-cap merged 2 commits intoelastic:masterfrom
hub-cap:issues/25208-fix_eclipse
Jul 27, 2017
Merged

Fix eclipse issues related to rest client shading#25874
hub-cap merged 2 commits intoelastic:masterfrom
hub-cap:issues/25208-fix_eclipse

Conversation

@hub-cap
Copy link
Copy Markdown
Contributor

@hub-cap hub-cap commented Jul 25, 2017

  • A cycle was detected in eclipse, and was fixed in the same fashion as
    core and core-tests.
  • The rest client deps jar was not properly exported in the generated
    eclipse classpath file for rest client.
  • Moved the plugin dependency due to eclipse-build 'apply from' not
    liking the plugin declaration in the rest client build.

Relates #25208

* A cycle was detected in eclipse, and was fixed in the same fashion as
  core and core-tests.
* The rest client deps jar was not properly exported in the generated
  eclipse classpath file for rest client.
* Moved the plugin dependency due to eclipse-build 'apply from' not
  liking the plugin declaration in the rest client build.

Relates elastic#25208
@hub-cap
Copy link
Copy Markdown
Contributor Author

hub-cap commented Jul 25, 2017

@nik9000 just an fyi, there is still a single problem in the eclipse build, but its totally unrelated to (or at least I think) the changes in the rest client.

Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM, just a question on why we need the shadow plugin in buildSrc

compile 'com.perforce:p4java:2012.3.551082' // THIS IS SUPPOSED TO BE OPTIONAL IN THE FUTURE....
compile 'de.thetaphi:forbiddenapis:2.3'
compile 'org.apache.rat:apache-rat:0.11'
compile 'com.github.jengelman.gradle.plugins:shadow:2.0.1'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this needed? Nothing in buildSrc uses it right?

* `org.elasticsearch.client`. This jar is the only actual output artifact of this job.
*/
plugins {
id "com.github.johnrengelman.shadow" version "2.0.1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are you moving this to the "apply plugin" form?

@colings86
Copy link
Copy Markdown
Contributor

colings86 commented Jul 25, 2017

Just for information, I have tested this on "Eclipse Neon.3 Release (4.6.3)" and can confirm it fixes the dependency cycle leaving no issues in Eclipse. This test was done on both master and 6.x branches

@colings86
Copy link
Copy Markdown
Contributor

I also tested master on "Eclipse Oxygen Release (4.7.0)" and the dependency cycle issue is fixed but I get the following error:

Description	Resource	Path	Location	Type
Cannot infer type argument(s) for <T> checkEqualsAndHashCode(T, EqualsHashCodeTestUtils.CopyFunction<T>, EqualsHashCodeTestUtils.MutateFunction<T>)	SuggestBuilderTests.java	/:core-tests/java/org/elasticsearch/search/suggest	line 90

I assume this is the error that @hub-cap is referring to above. These kinds of generics issues are common in eclipse unfortunately and seems unrelated to the issue this PR is fixing

@nik9000
Copy link
Copy Markdown
Member

nik9000 commented Jul 25, 2017

These kinds of generics issues are common in eclipse unfortunately and seems unrelated to the issue this PR is fixing

We tend to push Eclipse's compiler quite a bit, yeah. I've been filing issues with them as we encounter the issues and working around them and leaving a comment linking to the issue.

sourceSets {
if (project.path == ":client:rest") {
main.java.srcDirs = ['java']
//main.resources.srcDirs = ['resources']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd either uncomment or remove this line entirely.

@nik9000
Copy link
Copy Markdown
Member

nik9000 commented Jul 25, 2017

I tested locally and this fixes my issue.

@hub-cap
Copy link
Copy Markdown
Contributor Author

hub-cap commented Jul 26, 2017

I thought I was crazy for a moment, but this indeed (with that reverted second commit) blows up when doing anything eclipse related. I will attempt to use the buildscript style.

https://docs.gradle.org/3.3/userguide/plugins.html#plugins_dsl_limitations

The plugins {} block can currently only be used in a project's build script. It cannot be used in script plugins, the settings.gradle file or init scripts.

Future versions of Gradle will remove this restriction.

If the restrictions of the new syntax are prohibitive, the recommended approach is to apply plugins using the buildscript {} block.

@hub-cap
Copy link
Copy Markdown
Contributor Author

hub-cap commented Jul 27, 2017

2c271f0 fixes the last comment I made.

@hub-cap hub-cap merged commit 3a20922 into elastic:master Jul 27, 2017
hub-cap added a commit that referenced this pull request Jul 27, 2017
* A cycle was detected in eclipse, and was fixed in the same fashion as
  core and core-tests.
* The rest client deps jar was not properly exported in the generated
  eclipse classpath file for rest client.

Relates #25208
hub-cap added a commit that referenced this pull request Jul 27, 2017
* A cycle was detected in eclipse, and was fixed in the same fashion as
  core and core-tests.
* The rest client deps jar was not properly exported in the generated
  eclipse classpath file for rest client.

Relates #25208
hub-cap added a commit that referenced this pull request Jul 27, 2017
* A cycle was detected in eclipse, and was fixed in the same fashion as
  core and core-tests.
* The rest client deps jar was not properly exported in the generated
  eclipse classpath file for rest client.

Relates #25208
@colings86 colings86 added v6.0.0-beta1 :Delivery/Build Build or test infrastructure and removed v6.0.0 >bug labels Jul 31, 2017
@hub-cap hub-cap deleted the issues/25208-fix_eclipse branch August 28, 2017 19:18
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants