Solr
  1. Solr
  2. SOLR-2756

SolrJ maven dependencies are faulty; needless dependency on lucene-core

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: clients - java
    • Labels:
      None

      Description

      I included a SolrJ 3.3 dependency into a new project and I noticed needless dependencies transitive show up.

      Here is a subset of the output from mvn dependency:tree:

      [INFO] +- org.apache.solr:solr-solrj:jar:3.3.0:compile
      [INFO] |  +- org.apache.lucene:lucene-core:jar:3.3.0:compile
      [INFO] |  +- commons-httpclient:commons-httpclient:jar:3.1:compile
      [INFO] |  |  \- commons-codec:commons-codec:jar:1.2:compile
      [INFO] |  +- org.apache.geronimo.specs:geronimo-stax-api_1.0_spec:jar:1.0.1:compile
      [INFO] |  +- org.apache.zookeeper:zookeeper:jar:3.3.1:compile
      [INFO] |  |  +- log4j:log4j:jar:1.2.15:compile
      [INFO] |  |  |  \- javax.mail:mail:jar:1.4:compile
      [INFO] |  |  |     \- javax.activation:activation:jar:1.1:compile
      [INFO] |  |  \- jline:jline:jar:0.9.94:compile
      [INFO] |  \- org.codehaus.woodstox:wstx-asl:jar:3.2.7:compile
      [INFO] |     \- stax:stax-api:jar:1.0.1:compile
      

      Clearly there is an inconsistency with solr/dist/solrj-lib and this list.

      • lucene-core dependency should be removed
      • AFAIK, geronimo-stax-api and wstx-asl are only needed for Java 1.5. Right? These can be put in a maven profile activated by jdk1.5.
      • zookeeper dependency should be removed. Is this used in Solr 4? Even if it is, it strikes me as an optional dependency.

        Issue Links

          Activity

          Hide
          David Smiley added a comment -

          I'm willing to add a patch once we get consensus.

          Show
          David Smiley added a comment - I'm willing to add a patch once we get consensus.
          Hide
          Martijn van Groningen added a comment -

          +1 The lucene-core dependency should be removed. Solr is Java 1.6, so geronimo-stax-api and wstx-asl can also be removed.
          In the trunk there is SolrCloud code in the commons.cloud package. I'm not sure why this is put in the solrj source tree.
          In the 3x the zookeeper can be removed.

          Show
          Martijn van Groningen added a comment - +1 The lucene-core dependency should be removed. Solr is Java 1.6, so geronimo-stax-api and wstx-asl can also be removed. In the trunk there is SolrCloud code in the commons.cloud package. I'm not sure why this is put in the solrj source tree. In the 3x the zookeeper can be removed.
          Hide
          Uwe Schindler added a comment -

          Only Solr trunk is Java 1.6. 3.x is still Java 5.

          Show
          Uwe Schindler added a comment - Only Solr trunk is Java 1.6. 3.x is still Java 5.
          Hide
          Steve Rowe added a comment -

          Solr is Java 1.6, so geronimo-stax-api and wstx-asl can also be removed.

          About geronimo-stax-api, that dependency could be placed in a Java 1.5-activated profile.

          About wstx-asl, see SOLR-2054; I don't think this issue should decide that.

          Show
          Steve Rowe added a comment - Solr is Java 1.6, so geronimo-stax-api and wstx-asl can also be removed. About geronimo-stax-api, that dependency could be placed in a Java 1.5-activated profile. About wstx-asl, see SOLR-2054 ; I don't think this issue should decide that.
          Hide
          Martijn van Groningen added a comment -

          Oops.. in that case the 3x branch should then have the java5 profile

          Show
          Martijn van Groningen added a comment - Oops.. in that case the 3x branch should then have the java5 profile
          Hide
          Steve Rowe added a comment -

          lucene-core dependency should be removed

          I don't think it can be. When I tried, compilation fails:

          [ERROR] BUILD FAILURE
          [INFO] ------------------------------------------------------------------------
          [INFO] Compilation failure
          
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[19,29] package org.apache.lucene.util does not exist
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[353,38] cannot find symbol
          symbol  : class PriorityQueue
          location: class org.apache.solr.common.util.ConcurrentLRUCache<K,V>
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[320,24] cannot find symbol
          symbol  : method size()
          location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[320,58] cannot find symbol
          symbol  : method size()
          location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[321,56] cannot find symbol
          symbol  : method pop()
          location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[358,6] non-static variable super cannot be referenced from a static context
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[358,11] cannot find symbol
          symbol  : method initialize(int)
          location: class java.lang.Object
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[359,13] cannot find symbol
          symbol  : method getHeapArray()
          location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[365,4] method does not override or implement a method from a supertype
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[373,10] non-static method size() cannot be referenced from a static context
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[374,8] cannot find symbol
          symbol  : method add(java.lang.Object)
          location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[376,17] non-static method size() cannot be referenced from a static context
          \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[379,8] cannot find symbol
          symbol  : method updateTop()
          location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          
          Show
          Steve Rowe added a comment - lucene-core dependency should be removed I don't think it can be. When I tried, compilation fails: [ERROR] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Compilation failure \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[19,29] package org.apache.lucene.util does not exist \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[353,38] cannot find symbol symbol : class PriorityQueue location: class org.apache.solr.common.util.ConcurrentLRUCache<K,V> \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[320,24] cannot find symbol symbol : method size() location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[320,58] cannot find symbol symbol : method size() location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[321,56] cannot find symbol symbol : method pop() location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[358,6] non-static variable super cannot be referenced from a static context \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[358,11] cannot find symbol symbol : method initialize(int) location: class java.lang.Object \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[359,13] cannot find symbol symbol : method getHeapArray() location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[365,4] method does not override or implement a method from a supertype \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[373,10] non-static method size() cannot be referenced from a static context \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[374,8] cannot find symbol symbol : method add(java.lang.Object) location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[376,17] non-static method size() cannot be referenced from a static context \svn\lucene\dev\branches\branch_3x\solr\solrj\src\java\org\apache\solr\common\util\ConcurrentLRUCache.java:[379,8] cannot find symbol symbol : method updateTop() location: class org.apache.solr.common.util.ConcurrentLRUCache.PQueue
          Hide
          David Smiley added a comment -

          Ugh, well that sucks. Ideas:

          1. Perhaps the ConcurrentLRUCache can be omitted from the solrj jar? Off-hand, I'm not sure how to do this on the maven compile stage
          2. Put the dependency as <optional>true</optional> so that it is not transitively included. This is the simplest solution, for sure. It still means that solrj includes classes that aren't actually used by solrj but requires Lucene. But that's what ya get when you have a /util/ package so I suppose it's understandable.
          Show
          David Smiley added a comment - Ugh, well that sucks. Ideas: Perhaps the ConcurrentLRUCache can be omitted from the solrj jar? Off-hand, I'm not sure how to do this on the maven compile stage Put the dependency as <optional>true</optional> so that it is not transitively included. This is the simplest solution, for sure. It still means that solrj includes classes that aren't actually used by solrj but requires Lucene. But that's what ya get when you have a /util/ package so I suppose it's understandable.
          Hide
          Steve Rowe added a comment -

          zookeeper dependency should be removed. Is this used in Solr 4? Even if it is, it strikes me as an optional dependency.

          +1 - the zookeeper jar is included and used in Solr 4 (for SolrCloud), but isn't included in branch_3x.

          Show
          Steve Rowe added a comment - zookeeper dependency should be removed. Is this used in Solr 4? Even if it is, it strikes me as an optional dependency. +1 - the zookeeper jar is included and used in Solr 4 (for SolrCloud), but isn't included in branch_3x.
          Hide
          Steve Rowe added a comment -

          1. Perhaps the ConcurrentLRUCache can be omitted from the solrj jar? Off-hand, I'm not sure how to do this on the maven compile stage

          Bad idea, unless you mean ConcurrentLRUCache should be moved to solr/core/; Solr's FastLRUCache uses ConcurrentLRUCache.

          2. Put the dependency as <optional>true</optional> so that it is not transitively included. This is the simplest solution, for sure. It still means that solrj includes classes that aren't actually used by solrj but requires Lucene. But that's what ya get when you have a /util/ package so I suppose it's understandable.

          Downstream users of ConcurrentLRUCache - a public class - would succeed at compilation, but fail at runtime. Smells bad to me.

          Show
          Steve Rowe added a comment - 1. Perhaps the ConcurrentLRUCache can be omitted from the solrj jar? Off-hand, I'm not sure how to do this on the maven compile stage Bad idea, unless you mean ConcurrentLRUCache should be moved to solr/core/ ; Solr's FastLRUCache uses ConcurrentLRUCache . 2. Put the dependency as <optional>true</optional> so that it is not transitively included. This is the simplest solution, for sure. It still means that solrj includes classes that aren't actually used by solrj but requires Lucene. But that's what ya get when you have a /util/ package so I suppose it's understandable. Downstream users of ConcurrentLRUCache - a public class - would succeed at compilation, but fail at runtime. Smells bad to me.
          Hide
          Steve Rowe added a comment -

          This patch removes the zookeeper dependency, and makes geronimo-stax-api a dependency only under Java 1.5.

          Compiles for me under both Java 1.5 and 1.6.

          Show
          Steve Rowe added a comment - This patch removes the zookeeper dependency, and makes geronimo-stax-api a dependency only under Java 1.5. Compiles for me under both Java 1.5 and 1.6.
          Hide
          David Smiley added a comment -

          I didn't mean to move ConcurrentLRUCache to solr/core, but I like that idea. How is it pertinent that FastLRUCache uses ConcurrentLRUCache?

          If the choice is between doing nothing (keeping lucene-core dependency) vs making it optional, then I EMPHATICALLY insist making it optional is the right approach. If someone wants to use ConcurrentLRUCache, then they can simply depend on lucene-core (assuming they weren't already depending on it for some other reason). I would rather err on the side of this than have excessive dependencies. SolrJ's core function is to be a client to Solr, remember. Lets not trigger dependencies not needed for it's core function.

          But maybe there is another option to remove the dependency?

          Show
          David Smiley added a comment - I didn't mean to move ConcurrentLRUCache to solr/core, but I like that idea. How is it pertinent that FastLRUCache uses ConcurrentLRUCache? If the choice is between doing nothing (keeping lucene-core dependency) vs making it optional, then I EMPHATICALLY insist making it optional is the right approach. If someone wants to use ConcurrentLRUCache, then they can simply depend on lucene-core (assuming they weren't already depending on it for some other reason). I would rather err on the side of this than have excessive dependencies. SolrJ's core function is to be a client to Solr, remember. Lets not trigger dependencies not needed for it's core function. But maybe there is another option to remove the dependency?
          Hide
          David Smiley added a comment -

          Steve, in your patch, you forgot to put the woodstox jar into the jdk15 profile, and ideally with runtime scope. I am aware that Solr needs this on the server for some XSLT functions but that is not pertinent in the client. also, there appears to be dependency on stax:stax-api:jar:1.0.1 that is questionably if we already depend on geronimo's stax API – which I assume is the same Stax API.

          (thanks for doing the patch; I offered to do it)

          Show
          David Smiley added a comment - Steve, in your patch, you forgot to put the woodstox jar into the jdk15 profile, and ideally with runtime scope. I am aware that Solr needs this on the server for some XSLT functions but that is not pertinent in the client. also , there appears to be dependency on stax:stax-api:jar:1.0.1 that is questionably if we already depend on geronimo's stax API – which I assume is the same Stax API. (thanks for doing the patch; I offered to do it)
          Hide
          Steve Rowe added a comment -

          I didn't mean to move ConcurrentLRUCache to solr/core, but I like that idea. How is it pertinent that FastLRUCache uses ConcurrentLRUCache?

          Solr uses solrj for distributed search, and so depends on it (that and also because o.a.s.common.* is housed under solrj). AFAICT, FastLRUCache is the only consumer in Lucene/Solr-land of ConcurrentLRUCache.

          Does that answer your question?

          SolrJ's core function is to be a client to Solr, remember. Lets not trigger dependencies not needed for it's core function.

          Solrj has the additional core function of enabling distributed search.

          Show
          Steve Rowe added a comment - I didn't mean to move ConcurrentLRUCache to solr/core, but I like that idea. How is it pertinent that FastLRUCache uses ConcurrentLRUCache? Solr uses solrj for distributed search, and so depends on it (that and also because o.a.s.common.* is housed under solrj). AFAICT, FastLRUCache is the only consumer in Lucene/Solr-land of ConcurrentLRUCache . Does that answer your question? SolrJ's core function is to be a client to Solr, remember. Lets not trigger dependencies not needed for it's core function. Solrj has the additional core function of enabling distributed search.
          Hide
          Steve Rowe added a comment -

          Steve, in your patch, you forgot to put the woodstox jar into the jdk15 profile, and ideally with runtime scope.

          I didn't forget. See my above comment about SOLR-2054.

          also, there appears to be dependency on stax:stax-api:jar:1.0.1 that is questionably if we already depend on geronimo's stax API - which I assume is the same Stax API.

          I agree - this transitive dependency should be excluded.

          Show
          Steve Rowe added a comment - Steve, in your patch, you forgot to put the woodstox jar into the jdk15 profile, and ideally with runtime scope. I didn't forget. See my above comment about SOLR-2054 . also , there appears to be dependency on stax:stax-api:jar:1.0.1 that is questionably if we already depend on geronimo's stax API - which I assume is the same Stax API. I agree - this transitive dependency should be excluded.
          Hide
          Steve Rowe added a comment -

          On #lucene-dev IRC, David pointed out that Solr's use of Solrj for distributed search was irrelevant to the issue of whether Solrj's dependency on lucene-core should be made optional; I agreed.

          However, because ConcurrentLRUCache is the only class in Solrj that requires the lucene-core dependency, and solr-core's FastLRUCache is the only Lucene/Solr use of ConcurrentLRUCache, I think ConcurrentLRUCache should be moved from Solrj to solr-core.

          Show
          Steve Rowe added a comment - On #lucene-dev IRC , David pointed out that Solr's use of Solrj for distributed search was irrelevant to the issue of whether Solrj's dependency on lucene-core should be made optional; I agreed. However, because ConcurrentLRUCache is the only class in Solrj that requires the lucene-core dependency, and solr-core's FastLRUCache is the only Lucene/Solr use of ConcurrentLRUCache , I think ConcurrentLRUCache should be moved from Solrj to solr-core.
          Hide
          Steve Rowe added a comment -

          I think ConcurrentLRUCache should be moved from Solrj to solr-core.

          Done: SOLR-2758

          Show
          Steve Rowe added a comment - I think ConcurrentLRUCache should be moved from Solrj to solr-core. Done: SOLR-2758
          Hide
          Steve Rowe added a comment -

          there appears to be dependency on stax:stax-api:jar:1.0.1 that is questionably if we already depend on geronimo's stax API - which I assume is the same Stax API.

          This version of the patch excludes the stax:stax-api transitive dependency.

          I also added a CHANGES.txt entry.

          I plan on committing later today to branch_3x, then applying the same stax:stax-api transitive dependency exclusion to trunk (the other changes to branch_3x are not applicable to trunk).

          Show
          Steve Rowe added a comment - there appears to be dependency on stax:stax-api:jar:1.0.1 that is questionably if we already depend on geronimo's stax API - which I assume is the same Stax API. This version of the patch excludes the stax:stax-api transitive dependency. I also added a CHANGES.txt entry. I plan on committing later today to branch_3x, then applying the same stax:stax-api transitive dependency exclusion to trunk (the other changes to branch_3x are not applicable to trunk).
          Hide
          Ryan McKinley added a comment -

          However, because ConcurrentLRUCache is the only class in Solrj that requires the lucene-core dependency, and solr-core's FastLRUCache is the only Lucene/Solr use of ConcurrentLRUCache, I think ConcurrentLRUCache should be moved from Solrj to solr-core.

          +1

          solrj is the client it should not have any dependency on the server.

          Show
          Ryan McKinley added a comment - However, because ConcurrentLRUCache is the only class in Solrj that requires the lucene-core dependency, and solr-core's FastLRUCache is the only Lucene/Solr use of ConcurrentLRUCache, I think ConcurrentLRUCache should be moved from Solrj to solr-core. +1 solrj is the client it should not have any dependency on the server.
          Hide
          Steve Rowe added a comment -

          With the patch applied, the output under solr/solrj/ from mvn dependency:tree under Java 1.6 is now:

          [INFO] org.apache.solr:solr-solrj:jar:3.5-SNAPSHOT
          [INFO] +- commons-httpclient:commons-httpclient:jar:3.1:compile
          [INFO] |  +- commons-logging:commons-logging:jar:1.1.1:compile (version managed from 1.0.4)
          [INFO] |  \- commons-codec:commons-codec:jar:1.4:compile (version managed from 1.2)
          [INFO] +- commons-io:commons-io:jar:1.4:compile
          [INFO] +- org.codehaus.woodstox:wstx-asl:jar:3.2.7:compile
          [INFO] \- org.slf4j:slf4j-api:jar:1.6.1:compile
          
          Show
          Steve Rowe added a comment - With the patch applied, the output under solr/solrj/ from mvn dependency:tree under Java 1.6 is now: [INFO] org.apache.solr:solr-solrj:jar:3.5-SNAPSHOT [INFO] +- commons-httpclient:commons-httpclient:jar:3.1:compile [INFO] | +- commons-logging:commons-logging:jar:1.1.1:compile (version managed from 1.0.4) [INFO] | \- commons-codec:commons-codec:jar:1.4:compile (version managed from 1.2) [INFO] +- commons-io:commons-io:jar:1.4:compile [INFO] +- org.codehaus.woodstox:wstx-asl:jar:3.2.7:compile [INFO] \- org.slf4j:slf4j-api:jar:1.6.1:compile
          Hide
          Steve Rowe added a comment -

          Committed to branch_3x and (partially) to trunk.

          Show
          Steve Rowe added a comment - Committed to branch_3x and (partially) to trunk.
          Hide
          Uwe Schindler added a comment -

          Bulk close after 3.5 is released

          Show
          Uwe Schindler added a comment - Bulk close after 3.5 is released

            People

            • Assignee:
              Steve Rowe
              Reporter:
              David Smiley
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development