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

Generate POM files with non-wildcard excludes #21234

Merged
merged 2 commits into from Nov 4, 2016

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Nov 1, 2016

Dependencies are currently marked as non-transitive in generated POM files by adding a wildcard (*) exclusion. This breaks compatibility with the dependency manager Apache Ivy as it incorrectly translates POMs with * excludes to Ivy XML with * excludes which results in the main artifact being excluded as well (see https://issues.apache.org/jira/browse/IVY-1531). To stay compatible with the current release of Ivy this commit uses explicit excludes for each transitive artifact instead to ensure that the main artifact is not excluded. This should be revisited when we upgrade Gradle to a higher version as the current one (2.13) as Gradle automatically translates non-transitive dependencies to * excludes in 2.14+.

Relates to #21170

I've tested this patch as follows:

  1. applied patch to 5.0 branch and published artifacts to local maven repository by running gradle publishToMavenLocal
  2. checked various dependency management systems how they compare between the officially published 5.0.0 and the generated 5.0.1-SNAPSHOT version with the applied patch.

Dependency management systems:

Gradle

Build file:

apply plugin: 'java'

repositories {
	mavenLocal()
	mavenCentral()
}

dependencies {
	compile 'org.elasticsearch:elasticsearch:5.0.0'
}

Run gradle dependencies --configuration compile which yields

\--- org.elasticsearch:elasticsearch:5.0.0
     +--- org.apache.lucene:lucene-core:6.2.0
     +--- org.apache.lucene:lucene-analyzers-common:6.2.0
     +--- org.apache.lucene:lucene-backward-codecs:6.2.0
     +--- org.apache.lucene:lucene-grouping:6.2.0
     +--- org.apache.lucene:lucene-highlighter:6.2.0
     +--- org.apache.lucene:lucene-join:6.2.0
     +--- org.apache.lucene:lucene-memory:6.2.0
     +--- org.apache.lucene:lucene-misc:6.2.0
     +--- org.apache.lucene:lucene-queries:6.2.0
     +--- org.apache.lucene:lucene-queryparser:6.2.0
     +--- org.apache.lucene:lucene-sandbox:6.2.0
     +--- org.apache.lucene:lucene-spatial:6.2.0
     +--- org.apache.lucene:lucene-spatial-extras:6.2.0
     +--- org.apache.lucene:lucene-spatial3d:6.2.0
     +--- org.apache.lucene:lucene-suggest:6.2.0
     +--- org.elasticsearch:securesm:1.1
     +--- net.sf.jopt-simple:jopt-simple:5.0.2
     +--- com.carrotsearch:hppc:0.7.1
     +--- joda-time:joda-time:2.9.4
     +--- org.joda:joda-convert:1.2
     +--- org.yaml:snakeyaml:1.15
     +--- com.fasterxml.jackson.core:jackson-core:2.8.1
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-smile:2.8.1
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.8.1
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.8.1
     +--- com.tdunning:t-digest:3.0
     +--- org.hdrhistogram:HdrHistogram:2.1.6
     \--- net.java.dev.jna:jna:4.2.2

Replace the 5.0.0 dependency by 5.0.1-SNAPSHOT in the build file and run same command again:

\--- org.elasticsearch:elasticsearch:5.0.1-SNAPSHOT
     +--- org.apache.lucene:lucene-core:6.2.1
     +--- org.apache.lucene:lucene-analyzers-common:6.2.1
     +--- org.apache.lucene:lucene-backward-codecs:6.2.1
     +--- org.apache.lucene:lucene-grouping:6.2.1
     +--- org.apache.lucene:lucene-highlighter:6.2.1
     +--- org.apache.lucene:lucene-join:6.2.1
     +--- org.apache.lucene:lucene-memory:6.2.1
     +--- org.apache.lucene:lucene-misc:6.2.1
     +--- org.apache.lucene:lucene-queries:6.2.1
     +--- org.apache.lucene:lucene-queryparser:6.2.1
     +--- org.apache.lucene:lucene-sandbox:6.2.1
     +--- org.apache.lucene:lucene-spatial:6.2.1
     +--- org.apache.lucene:lucene-spatial-extras:6.2.1
     +--- org.apache.lucene:lucene-spatial3d:6.2.1
     +--- org.apache.lucene:lucene-suggest:6.2.1
     +--- org.elasticsearch:securesm:1.1
     +--- net.sf.jopt-simple:jopt-simple:5.0.2
     +--- com.carrotsearch:hppc:0.7.1
     +--- joda-time:joda-time:2.9.4
     +--- org.joda:joda-convert:1.2
     +--- org.yaml:snakeyaml:1.15
     +--- com.fasterxml.jackson.core:jackson-core:2.8.1
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-smile:2.8.1
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.8.1
     +--- com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.8.1
     +--- com.tdunning:t-digest:3.0
     +--- org.hdrhistogram:HdrHistogram:2.1.6
     \--- net.java.dev.jna:jna:4.2.2

Conclusion:

Gradle was handling the * excludes fine before and is still working as expected afterwards.

Ivy

Create ivysettings.xml file with following contents:

<ivysettings>
    <settings defaultResolver="default"/>
    <resolvers>
        <chain name="default">
            <filesystem name="local-maven2" m2compatible="true" checkmodified="true" changingPattern=".*SNAPSHOT">      
              <ivy pattern="${user.home}/.m2/repository/[organisation]/[module]/[revision]/[module]-[revision].pom" />
              <artifact pattern="${user.home}/.m2/repository/[organisation]/[module]/[revision]/[artifact]-[revision](-[classifier]).[ext]" />
            </filesystem>
            <ibiblio name="central" m2compatible="true"/>
        </chain>
    </resolvers>
</ivysettings>

Then run
ivy -settings ivysettings.xml -confs default -dependency org.elasticsearch elasticsearch 5.0.0 -retrieve "jars/[module]-[artifact](-[revision]).[ext]"

which yields

	---------------------------------------------------------------------
	|                  |            modules            ||   artifacts   |
	|       conf       | number| search|dwnlded|evicted|| number|dwnlded|
	---------------------------------------------------------------------
	|      default     |   29  |   29  |   29  |   0   ||   12  |   12  |
	---------------------------------------------------------------------

Only 12 artifacts instead of the 29 that we expect.

$ ls jars
HdrHistogram-HdrHistogram-2.1.6.jar   jackson-core-jackson-core-2.8.1.jar   joda-time-joda-time-2.9.4.jar         securesm-securesm-1.1.jar
elasticsearch-elasticsearch-5.0.0.jar jna-jna-4.2.2.jar                     jopt-simple-jopt-simple-5.0.2.jar     snakeyaml-snakeyaml-1.15.jar
hppc-hppc-0.7.1.jar                   joda-convert-joda-convert-1.2.jar     lucene-core-lucene-core-6.2.0.jar     t-digest-t-digest-3.0.jar

Let's repeat the same for 5.0.1-SNAPSHOT (but first rm jars/*):

ivy -settings ivysettings.xml -confs default -dependency org.elasticsearch elasticsearch 5.0.1-SNAPSHOT -retrieve "jars/[module]-[artifact](-[revision]).[ext]"

which yields:

	---------------------------------------------------------------------
	|                  |            modules            ||   artifacts   |
	|       conf       | number| search|dwnlded|evicted|| number|dwnlded|
	---------------------------------------------------------------------
	|      default     |   29  |   16  |   16  |   0   ||   29  |   19  |
	---------------------------------------------------------------------

All 29 artifacts are accounted for.

$ ls jars
HdrHistogram-HdrHistogram-2.1.6.jar                         lucene-highlighter-lucene-highlighter-6.2.1.jar
elasticsearch-elasticsearch-5.0.1-SNAPSHOT.jar              lucene-join-lucene-join-6.2.1.jar
hppc-hppc-0.7.1.jar                                         lucene-memory-lucene-memory-6.2.1.jar
jackson-core-jackson-core-2.8.1.jar                         lucene-misc-lucene-misc-6.2.1.jar
jackson-dataformat-cbor-jackson-dataformat-cbor-2.8.1.jar   lucene-queries-lucene-queries-6.2.1.jar
jackson-dataformat-smile-jackson-dataformat-smile-2.8.1.jar lucene-queryparser-lucene-queryparser-6.2.1.jar
jackson-dataformat-yaml-jackson-dataformat-yaml-2.8.1.jar   lucene-sandbox-lucene-sandbox-6.2.1.jar
jna-jna-4.2.2.jar                                           lucene-spatial-extras-lucene-spatial-extras-6.2.1.jar
joda-convert-joda-convert-1.2.jar                           lucene-spatial-lucene-spatial-6.2.1.jar
joda-time-joda-time-2.9.4.jar                               lucene-spatial3d-lucene-spatial3d-6.2.1.jar
jopt-simple-jopt-simple-5.0.2.jar                           lucene-suggest-lucene-suggest-6.2.1.jar
lucene-analyzers-common-lucene-analyzers-common-6.2.1.jar   securesm-securesm-1.1.jar
lucene-backward-codecs-lucene-backward-codecs-6.2.1.jar     snakeyaml-snakeyaml-1.15.jar
lucene-core-lucene-core-6.2.1.jar                           t-digest-t-digest-3.0.jar
lucene-grouping-lucene-grouping-6.2.1.jar

SBT (version 0.13.12)

Let's create a simple build file (build.sbt):

name := "test"

resolvers += Resolver.mavenLocal

libraryDependencies ++= Seq(
   "org.elasticsearch" % "elasticsearch" % "5.0.0"
)

and run sbt 'show runtime:fullClasspath'

List(Attributed(/Users/ywelsch/dev/sbt-example/target/scala-2.10/classes), Attributed(/Users/ywelsch/.sbt/boot/scala-2.10.6/lib/scala-library.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.elasticsearch/elasticsearch/jars/elasticsearch-5.0.0.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-core/jars/lucene-core-6.2.0.jar), Attributed(/Users/ywelsch/.m2/repository/org/elasticsearch/securesm/1.1/securesm-1.1.jar), Attributed(/Users/ywelsch/.m2/repository/net/sf/jopt-simple/jopt-simple/5.0.2/jopt-simple-5.0.2.jar), Attributed(/Users/ywelsch/.m2/repository/com/carrotsearch/hppc/0.7.1/hppc-0.7.1.jar), Attributed(/Users/ywelsch/.m2/repository/joda-time/joda-time/2.9.4/joda-time-2.9.4.jar), Attributed(/Users/ywelsch/.m2/repository/org/joda/joda-convert/1.2/joda-convert-1.2.jar), Attributed(/Users/ywelsch/.m2/repository/org/yaml/snakeyaml/1.15/snakeyaml-1.15.jar), Attributed(/Users/ywelsch/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.8.1/jackson-core-2.8.1.jar), Attributed(/Users/ywelsch/.m2/repository/com/tdunning/t-digest/3.0/t-digest-3.0.jar), Attributed(/Users/ywelsch/.m2/repository/org/hdrhistogram/HdrHistogram/2.1.6/HdrHistogram-2.1.6.jar), Attributed(/Users/ywelsch/.m2/repository/net/java/dev/jna/jna/4.2.2/jna-4.2.2.jar))

Same as for Ivy, only 12 artifacts instead of the 29 that we expect.

Replace the 5.0.0 dependency by 5.0.1-SNAPSHOT in the build file and run same command again:

List(Attributed(/Users/ywelsch/dev/sbt-example/target/scala-2.10/classes), Attributed(/Users/ywelsch/.sbt/boot/scala-2.10.6/lib/scala-library.jar), Attributed(/Users/ywelsch/.m2/repository/org/elasticsearch/elasticsearch/5.0.1-SNAPSHOT/elasticsearch-5.0.1-SNAPSHOT.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-core/jars/lucene-core-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-analyzers-common/jars/lucene-analyzers-common-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-backward-codecs/jars/lucene-backward-codecs-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-grouping/jars/lucene-grouping-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-highlighter/jars/lucene-highlighter-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-join/jars/lucene-join-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-memory/jars/lucene-memory-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-misc/jars/lucene-misc-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-queries/jars/lucene-queries-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-queryparser/jars/lucene-queryparser-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-sandbox/jars/lucene-sandbox-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-spatial/jars/lucene-spatial-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-spatial-extras/jars/lucene-spatial-extras-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-spatial3d/jars/lucene-spatial3d-6.2.1.jar), Attributed(/Users/ywelsch/.ivy2/cache/org.apache.lucene/lucene-suggest/jars/lucene-suggest-6.2.1.jar), Attributed(/Users/ywelsch/.m2/repository/org/elasticsearch/securesm/1.1/securesm-1.1.jar), Attributed(/Users/ywelsch/.m2/repository/net/sf/jopt-simple/jopt-simple/5.0.2/jopt-simple-5.0.2.jar), Attributed(/Users/ywelsch/.m2/repository/com/carrotsearch/hppc/0.7.1/hppc-0.7.1.jar), Attributed(/Users/ywelsch/.m2/repository/joda-time/joda-time/2.9.4/joda-time-2.9.4.jar), Attributed(/Users/ywelsch/.m2/repository/org/joda/joda-convert/1.2/joda-convert-1.2.jar), Attributed(/Users/ywelsch/.m2/repository/org/yaml/snakeyaml/1.15/snakeyaml-1.15.jar), Attributed(/Users/ywelsch/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.8.1/jackson-core-2.8.1.jar), Attributed(/Users/ywelsch/.m2/repository/com/fasterxml/jackson/dataformat/jackson-dataformat-smile/2.8.1/jackson-dataformat-smile-2.8.1.jar), Attributed(/Users/ywelsch/.m2/repository/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.8.1/jackson-dataformat-yaml-2.8.1.jar), Attributed(/Users/ywelsch/.m2/repository/com/fasterxml/jackson/dataformat/jackson-dataformat-cbor/2.8.1/jackson-dataformat-cbor-2.8.1.jar), Attributed(/Users/ywelsch/.m2/repository/com/tdunning/t-digest/3.0/t-digest-3.0.jar), Attributed(/Users/ywelsch/.m2/repository/org/hdrhistogram/HdrHistogram/2.1.6/HdrHistogram-2.1.6.jar), Attributed(/Users/ywelsch/.m2/repository/net/java/dev/jna/jna/4.2.2/jna-4.2.2.jar))

All 29 artifacts are there.

Dependencies are currently marked as non-transitive in generated POM files by adding a wildcard (*) exclusion.
This does not play with the Apache Ivy dependency manager as it interprets these wildcards to exclude the main artifact of the dependency as well.
This commit uses explicit excludes for each transitive artifact instead to ensure that the main artifact is not excluded.
@nik9000
Copy link
Member

nik9000 commented Nov 1, 2016

I ❤️ all the tests!

Also, sbt spits out some long lines....

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

👍 :shipit:

@rjernst
Copy link
Member

rjernst commented Nov 1, 2016

I do not see this as an elasticsearch bug, and do not think we should make this change. It is a bug in ivy. Wildcard exclusions are legitimate configuration for maven, and I added them to our generated poms to match what we will get when we are finally able to upgrade gradle to 3.x.

@nik9000
Copy link
Member

nik9000 commented Nov 1, 2016

We can ask contributors to run a specific version of gradle but I'm not sure we can ask users to run a particular version of ivy. Ivy should totally fix this but we should work around their bug in the mean time.

@ywelsch
Copy link
Contributor Author

ywelsch commented Nov 3, 2016

@rjernst The SBT team has only fixed this in their latest release (0.13.3 - released 6 days ago) by patching their custom Ivy fork whereas other build / dependency management systems (e.g. Grape) use the official Ivy release which doesn't have such a fix. Fixing the incompatibility on our end for now is an easy thing to do (as I've shown with this PR) and will enable users of all build systems to properly consume our dependencies. Unless you come up with a better plan, I will merge this tonight.

@clintongormley clintongormley added the :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts label Nov 3, 2016
@nik9000 nik9000 added the blocker label Nov 3, 2016
@nik9000
Copy link
Member

nik9000 commented Nov 3, 2016

Unless you come up with a better plan, I will merge this tonight.

As much as I'd love to get this merged I don't think we should merge this over @rjernst's objections. It is our place as supporters of the patch to argue for it and @rjernst's place to either be convinced and withdraw his objections or not. I really do want us to generate the poms this way so ivy users can use it but I don't want to set a precedent that it is ok to override objections.

Don't get me wrong, it is perfectly healthy to not do all of the thing requested in a PR but it is the requester of the change's place to withdraw the request not our place to merge anyway. We do this all the time, deciding that things should be done in a followup or not at all.

With that said, I'll make my case again:
I think it is reasonable for us to support the released versions of ivy/gradle/grape. We can and likely should propose a fix to ivy because we very badly want the wildcard excludes to our POMs. And we should add them back (reverting this PR) once the fix is released and good and saturated.

@rjernst
Copy link
Member

rjernst commented Nov 3, 2016

Thank you Nik.

We can and likely should propose a fix to ivy because we very badly want the wildcard excludes to our POMs.

If we are going to merge this, then I think we should first file an issue with Ivy, which can be referenced in comments in this commit. And a comment should still exist that explains this will need revisiting when we upgrade to gradle 3.x.

@ywelsch
Copy link
Contributor Author

ywelsch commented Nov 3, 2016

There is an open issue for it on the IVY issue tracker (didn't find it the first time):

IVY-1531: Translation of POM to Ivy XML with * exclusion is removing main artifact
https://issues.apache.org/jira/browse/IVY-1531

@rjernst
Copy link
Member

rjernst commented Nov 3, 2016

@ywelsch Can you please add a link to that issue in your commit, along with a note that Gradle does this in 2.14+ automatically and we will need to revisit when upgrading?

@ywelsch
Copy link
Contributor Author

ywelsch commented Nov 3, 2016

@rjernst I've added the link and few more words in f3bbb76. I will also add this to the commit message when the PR is squash-merged. Please let me know if you're ok with merging this.

@ywelsch ywelsch merged commit d53e1d2 into elastic:master Nov 4, 2016
@ywelsch
Copy link
Contributor Author

ywelsch commented Nov 4, 2016

Thanks @nik9000 @rjernst

ywelsch added a commit that referenced this pull request Nov 4, 2016
Dependencies are currently marked as non-transitive in generated POM files by adding a wildcard (*) exclusion. This breaks compatibility with the dependency manager Apache Ivy as it incorrectly translates POMs with * excludes to Ivy XML with * excludes which results in the main artifact being excluded as well (see https://issues.apache.org/jira/browse/IVY-1531). To stay compatible with the current release of Ivy this commit uses explicit excludes for each transitive artifact instead to ensure that the main artifact is not excluded. This should be revisited when we upgrade Gradle to a higher version as the current one (2.13) as Gradle automatically translates non-transitive dependencies to * excludes in 2.14+.
ywelsch added a commit that referenced this pull request Nov 4, 2016
Dependencies are currently marked as non-transitive in generated POM files by adding a wildcard (*) exclusion. This breaks compatibility with the dependency manager Apache Ivy as it incorrectly translates POMs with * excludes to Ivy XML with * excludes which results in the main artifact being excluded as well (see https://issues.apache.org/jira/browse/IVY-1531). To stay compatible with the current release of Ivy this commit uses explicit excludes for each transitive artifact instead to ensure that the main artifact is not excluded. This should be revisited when we upgrade Gradle to a higher version as the current one (2.13) as Gradle automatically translates non-transitive dependencies to * excludes in 2.14+.
@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
blocker >bug :Delivery/Build Build or test infrastructure :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team v5.0.1 v5.1.1 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants