asfimport commented 11 years ago

6313 introduced centralized versioning of 3rd party dependencies and converted all ivy.xml files across Lucene/Solr to use this scheme. But there is nothing preventing people from ignoring this setup and (intentionally or not) introducing non-centralized dependency versions.

SOLR-3664 discusses the problem of out-of-sync 3rd party dependencies between Lucene/Solr modules. Centralized versioning makes synchronization problems less likely but not impossible.

One fairly simple way to ensure that all modules use the same version of 3rd party deps would be to require that all deps in ivy.xml would have to use the rev="$\{/org/name}" syntax, via a validation script.

The problem remains that there may eventually be a requirement to use different 3rd party libs in different modules. Any form of lockdown here should allow for this possibility. Hoss's suggestion from a conversation on #lucene IRC earlier today:

  <hoss> perhaps exceptions could be by naming convetion
<sarowe> can you give an example?
  <hoss> ie: variables must match either ${group}/${artifact} or they must match
         /VERSION_MISTMATCH_EXCEPTION/${group}/${artifact}
<sarowe> nice idea
         no external config required
  <hoss> right
         and it has to be real obvious when you are bucking convention
  <hoss> or better yet: ${group}/${artifact}/VERSION_MISTMATCH_EXCEPTION
         ... and there is another check that the version file is in ascii order
         so you are garuntted that it has to be right there in the versions file
         one line after ${group}/${artifact}
<sarowe> i like it
  <hoss> no change someone updating ${group}/${artifact} won't notice it
         i suppose really it should be
         ${group}/${artifact}/VERSION_MISTMATCH_EXCEPTION/${reason}
         ... since you might have more then one exception per ${group}/${artifact}
         but now i'm just making things up w/o evn really understanding
         the conventions you've alreay put in place
<sarowe> :)
  <hoss> you get the idea
<sarowe> yes

Migrated from LUCENE-5257 by Steven Rowe (@sarowe), resolved Oct 04 2013 Attachments: LUCENE-5257.patch (versions: 3) Linked issues:

SOLR-3664

asfimport commented 11 years ago

Robert Muir (@rmuir) (migrated from JIRA)

The problem remains that there may eventually be a requirement to use different 3rd party libs in different modules. Any form of lockdown here should allow for this possibility.

Not a chance. -1 to jar hell.

asfimport commented 11 years ago

Steven Rowe (@sarowe) (migrated from JIRA)

Not a chance. -1 to jar hell.

Cool, makes this issue easier.

asfimport commented 11 years ago

Chris M. Hostetter (@hossman) (migrated from JIRA)

FWIW: the idea that we might down the road want to allow moduleA to depends on third-party-2.0.jar while unrelated moduleB depends on third-party-3.0.jar came from a mental exercise of "if we lock this down with a build failure now, how will we deal with it if we decide we want to allow it in some special case later?"

but i agree: even if moduleA and moduleB have no dependencies on eachother, and no other moduleC exists that depends on both moduleA and moduleB it's still a bad idea to let moduleA and moduleB depend on diff versions of third-party.jar since some downstream client of lucene/solr might want to use both moduleA and moduleB.

more importantly: we can lock this down with a build failure now, and if we do ever change out mind, we have an idea here of how to tweak the rules to allow special exceptions in a way that's non-sneaky and really obvious ... but unless someone is clamoring for it, i say we help our users avoid classpath hell just as much as we want to.

asfimport commented 11 years ago

Robert Muir (@rmuir) (migrated from JIRA)

Let me clarify what i mean by jar hell:

if we want to add exceptions because someone jacked up their maven coordinates or some other reason, when in fact its not "jar hell", to me thats different than declaring we will allow exceptions for jar hell.

For example: some stuff depends on jetty6 (org.mortbay.jetty) and other stuff depends on jetty8 (org.eclipse.jetty). This isn't jar hell, because the packages are totally different in the jar files so they don't conflict. They also renamed the coordinates to org.eclipse.jetty to match.

But i think we shouldnt allow true jar hell (where the stuff inside the jars conflict). People should be able to pick and choose what stuff they want to use and use it, and we shouldnt ship a release with jar hell.

I would also like to explore adding a "jar hell detector" in the future to our smoketester, that unzips all jar files and fails if e.g. any .class (maybe even resources outside of META-INF) conflict. I think we should try to improve our build with checks like this, but if we allow for "jar hell exceptions" it makes such a check extremely difficult or impossible.

asfimport commented 11 years ago

Steven Rowe (@sarowe) (migrated from JIRA)

Patch implementing the idea as a custom Ant task: LibVersionCheckTask.

I had originally planned a scripted implementation, but I figured that could fail for valid ivy-version.properties syntax, or valid ivy.xml syntax, since a script would likely make assumptions about the format, so I went with full parsing for both, though I had to write the Properties file parser, since java.util.Properties doesn't provide file-order access to the parsed contents.

The new task checks the /org/name keys in the centralized versions file for lexical sort order and for duplicates, then verifies that the rev attributes for all <dependency>-s in all ivy.xml files in the current directory (lucene/ and solr/) and below are formatted in $\{/org/name} format, where org matches the value of the "org" attribute, and name matches the value of the "name" attribute.

I modeled it on (and stole a lot of it from) the custom LicenseCheckTask.

The patch includes a fix to the ivy-versions.properties file that the new task found: there are currently duplicate property keys for two of the httpcomponents jars.

Like LicenseCheckTask, I made it run with the validate target, so it'll be included with ant precommit.

Run alone, it took less than a tenth of second in each of lucene/ and solr/ on my machine.

I think it's ready to go.

asfimport commented 11 years ago

Steven Rowe (@sarowe) (migrated from JIRA)

New patch with a few changes:

Added orphan checking for /org/name coordinate keys in ivy-versions.properties file that are not referred to in any ivy.xml file.
In order to add orphan checking, I had to make the task run only at the top level, so that it sees all the ivy.xml files across the whole project. The simplest way to do that with the current build was to place the check-lib-versions target in lucene/build.xml, and then invoke it from the top-level validate target, recursively checking the ivy.xml files in the parent of the lucene/ directory. (This is a little clunky - really it should live at the top level - but I can live with it.)
Cleaned up the ivy-versions.properties file, so that versions shared by two or more libs are represented as a property named for their org/groupId (and sometimes part of their name/artifactId if the org/groupId didn't seem to be sufficiently differentiating).

I'll commit later today if there are no objections.

asfimport commented 11 years ago

Steven Rowe (@sarowe) (migrated from JIRA)

Patch with cosmetic cleanups and a CHANGES.txt entry.

asfimport commented 11 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1529243 from @sarowe in branch 'dev/trunk' https://svn.apache.org/r1529243

LUCENE-5257: Lock down centralized versioning of ivy dependencies

asfimport commented 11 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1529246 from @sarowe in branch 'dev/trunk' https://svn.apache.org/r1529246

LUCENE-5257: Fix top-level validate target subant invocation syntax

asfimport commented 11 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1529248 from @sarowe in branch 'dev/branches/branch_4x' https://svn.apache.org/r1529248

LUCENE-5257: Lock down centralized versioning of ivy dependencies (merged trunk r1529243 and r1529246)

asfimport commented 11 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1529249 from @sarowe in branch 'dev/trunk' https://svn.apache.org/r1529249

LUCENE-5257: merge CHANGES.txt entry with LUCENE-5249's entry

asfimport commented 11 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1529250 from @sarowe in branch 'dev/branches/branch_4x' https://svn.apache.org/r1529250

LUCENE-5257: merge CHANGES.txt entry with LUCENE-5249's entry (merged trunk r1529249)

asfimport commented 11 years ago

Steven Rowe (@sarowe) (migrated from JIRA)

Committed to trunk and branch_4x.

apache / lucene

Lock down centralized versioning of ivy dependencies [LUCENE-5257] #6321

6313 introduced centralized versioning of 3rd party dependencies and converted all ivy.xml files across Lucene/Solr to use this scheme. But there is nothing preventing people from ignoring this setup and (intentionally or not) introducing non-centralized dependency versions.