Skip to content

Conversation

@hub-cap
Copy link
Contributor

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

The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle.

Closes #25884
Relates #25208

Copy link
Member

Choose a reason for hiding this comment

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

Why not just check the version of gradle that is running to determine which configuration to manipulate? See GradleVersion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could do that too. This also works w/o any conditional logic tho for all the 3.x and 4.0. Those comments are more notes mentioning why we are removing each one. Im fine either way with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill go ahead and do that to prevent the extra configuration from being created in 3.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 752f2db

The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle. It also cleans up issues caused by shadowJar and
shadeDeps causing no dependencies to be put on the classpath.

Relates elastic#25208
configurations.runtime.artifacts.removeAll { it.archiveTask.is jar }

// removes the runtime configuration inheritance from compile
// runtimeElements does not exist in 3.3
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably use the GradleVersion for a conditional here as Ryan suggested.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh sorry. didn't see you already said you're on it (as the previous collapsed convo reflects)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 752f2db

@talevy
Copy link
Contributor

talevy commented Jul 25, 2017

Since elasticsearch-ci would not catch this for us. I'll give my test status report here...

$ gradle modules:reindex:integTest
...
=======================================
Elasticsearch Build Hamster says Hello!
=======================================
  Gradle Version        : 3.5
  OS Info               : Mac OS X 10.12.4 (x86_64)
  JDK Version           : Azul Systems, Inc. 1.8.0_131 [OpenJDK 64-Bit Server VM 25.131-b11]
  JAVA_HOME             : /Users/tal/.sdkman/candidates/java/8u131-zulu
  Random Testing Seed   : F9BFC07CD9E8FFD6
The executable property on ForkOptions has been deprecated and is scheduled to be removed in Gradle 5.0. Please use javaHome instead.
...
==> Test Info: seed=90419E107D08AA73; jvm=1; suite=1
==> Test Summary: 1 suite, 119 tests
:modules:reindex:integTestCluster#stop
:modules:reindex:integTest

BUILD SUCCESSFUL

🍏 📗 💚 🥗

Intellij & Eclipse not reflected here

@bleskes
Copy link
Contributor

bleskes commented Jul 26, 2017

I can't comment to the approach taken but this works for me on both 3.5 and 4.0.1. It also fixed my intellij 2017.2 that decided to break this morning.

@s1monw
Copy link
Contributor

s1monw commented Jul 26, 2017

++ lets merge this

@bleskes bleskes merged commit 9d10dbe into elastic:master Jul 26, 2017
@bleskes
Copy link
Contributor

bleskes commented Jul 26, 2017

@rjernst @hub-cap I merged this as it seems to work and allows people on the european time zone to work on master with less hassle. We can always change it if it's not perfect yet. Thanks for fixing.

@olcbean
Copy link
Contributor

olcbean commented Jul 26, 2017

@hub-cap the fix worked on 3.4.1, but now I am experiencing issues with :qa:full-cluster-restart

@olcbean
Copy link
Contributor

olcbean commented Jul 26, 2017

@hub-cap I just opened #25907 for the failure in :qa:full-cluster-restart

hub-cap added a commit to hub-cap/elasticsearch that referenced this pull request Jul 26, 2017
The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle.

Closes elastic#25884
Relates elastic#25208
hub-cap added a commit that referenced this pull request Jul 26, 2017
The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle.

Closes #25884
Relates #25208
hub-cap added a commit that referenced this pull request Jul 26, 2017
The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle.

Closes #25884
Relates #25208
hub-cap added a commit that referenced this pull request Jul 26, 2017
The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle.

Closes #25884
Relates #25208
hub-cap added a commit that referenced this pull request Jul 26, 2017
The configuration removed from the runtime configuration did not
properly remove the deps jar from gradle versions > 3.3. The rest client
now removes both the 3.3 and 3.3+ configurations so this works on both
versions of gradle.

Closes #25884
Relates #25208
@olcbean
Copy link
Contributor

olcbean commented Jul 26, 2017

@hub-cap @bleskes
I just noticed that the title is referring to gradle 3.5+. The code change and the description are both for gradle 3.3+. Maybe a typo?

@hub-cap
Copy link
Contributor Author

hub-cap commented Jul 27, 2017

Facepalm. Let's pretend we didn't just commit a misleading msg to git forever :D

@olcbean
Copy link
Contributor

olcbean commented Jul 27, 2017

Oh, I did not know that force push is not allowed...

@colings86 colings86 added :Delivery/Build Build or test infrastructure v5.6.0 and removed v6.0.0 >bug v5.6.1 labels Jul 31, 2017
@hub-cap hub-cap deleted the issues/25208-fix_gradle branch August 28, 2017 19:18
@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 v5.6.0 v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TEST] failures in integ-test-zip

8 participants