Skip to content

Conversation

@pgier
Copy link

@pgier pgier commented Apr 12, 2017

This is a bug fix upgrade and removes support for very old JDK versions (<1.2).
More information here: https://commons.apache.org/proper/commons-logging/changes-report.html

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

The Hadoop dependency definitely can not change, and I think that there is investigation needed before we can safely make the larger change.

compile 'com.google.guava:guava:11.0.2'
compile 'com.google.protobuf:protobuf-java:2.5.0'
compile 'commons-logging:commons-logging:1.1.3'
compile 'commons-logging:commons-logging:1.2'
Copy link
Member

Choose a reason for hiding this comment

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

No we can't do this one, the version of Hadoop here depends on 1.1.3.

# When updating httpasyncclient, please also update core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy
httpasyncclient = 4.1.2
commonslogging = 1.1.3
commonslogging = 1.2
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to look at what modules have a dependency on commonslogging and see if we can change it across the board like this. This also impacts downstream projects like plugins that rely on the version properties here in core to determine the version that they use.

Copy link
Member

Choose a reason for hiding this comment

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

I look at all the uses of commonslogging, and I think that going to 1.2 is safe here with the exception of the Hadoop change which as I mentioned must stay on 1.1.3.

@jasontedor
Copy link
Member

Can you please revert the repository-hdfs change and run gradle updateSHAs and push the resulting sha changes?

@jasontedor
Copy link
Member

test this please

@pgier
Copy link
Author

pgier commented Apr 12, 2017

Thanks for the quick feedback! I have pushed the requested changes.

@jasontedor
Copy link
Member

jasontedor commented Apr 12, 2017

Thanks for the quick feedback! I have pushed the requested changes.

You're welcome. Thanks for the PR and the quick iteration. I have asked our CI to kick off a build.

@jasontedor
Copy link
Member

I think that the transitive commons-logging dependency in repository-hdfs needs to be excluded.

@rjernst
Copy link
Member

rjernst commented Apr 18, 2017

@jasontedor I'm not sure that will work. When we moved repository-hdfs into ES, we did so adding each dependency (ie starting with excluding all transitive deps, and adding them one by one) until it worked. So I don't think simply dropping commons-logging will work.

@jasontedor
Copy link
Member

I meant the conflicting transitive dependency only but keeping the explicitly defined one.

@rjernst
Copy link
Member

rjernst commented Jun 13, 2017

I'm going to close this in favor of #25208. Once that issue is addressed, we can simply remove the shared version, rather than going through the pain of figuring out a safe version which all users of commons libs could upgrade to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants