Solr
  1. Solr
  2. SOLR-5950

In SolrJ's Maven Dependency Graph, slf4j-api is optional, which breaks plain solrj users

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.7, 4.7.1
    • Fix Version/s: 4.7.2, 4.8, 6.0
    • Component/s: clients - java
    • Labels:
      None

      Description

      To run as a Solr Client, you need slf4j actually configured in your classpath, because HttpSolrServer hardly depends on it, also the used libs like Commons Httpclient (and others)

      In SOLR-3706 all of the slf4j JARs were made optional, but because we did not exclude the dependencies of httpclient and others explicitely, the dependency was still included by maven automatically for a user that used SolrJ - so effectively the "optional" was never working.

      In 4.7, Steve Rowe now explicitely excluded all dependencies of all deps included via IVY. By that the implicit dependency by httpclient was killed.

      When I updated a project from SolrJ 4.6 to SolrJ 4.7, it suddenly failed to run, because it failed with a classnotfound ex directly after starting. Not even the default logging to console was enabled (which is provided by slf4j). This is bad for users of SolrJ, because they have to explicitely add a dependency for something that really required to use SolrJ.

      The reason for excluding slf4j-api was that we don't want to have it in the WAR file, so it was made optional. But that is wrong:

      • In trunk we no longer have a WAR file
      • The primary user of SolrJ via Maven is not somebody who wants to install Solr in his appserver, its the user needing the client.

      So we should restore the state from Solr 4.6, where the dep was implicitely included, by making it non-optional in Maven.

      When building the WAR file in 4.x we should exclude it somehow via Maven Magic(tm).

      1. SOLR-5950.branch_4x.patch
        7 kB
        Steve Rowe
      2. SOLR-5950.patch
        2 kB
        Steve Rowe
      3. SOLR-5950.trunk.patch
        6 kB
        Steve Rowe

        Issue Links

          Activity

          Hide
          Steve Rowe added a comment -

          Trunk patch that makes the org.slf4j:slf4j-api dependency not optional for solrj. This dependency remains optional for all other Solr modules.

          Show
          Steve Rowe added a comment - Trunk patch that makes the org.slf4j:slf4j-api dependency not optional for solrj. This dependency remains optional for all other Solr modules.
          Hide
          Mark Miller added a comment -

          In trunk we no longer have a WAR file

          The problem there is that we do still have a WAR file and we do still want logging jars to be easily swapped in trunk.

          Show
          Mark Miller added a comment - In trunk we no longer have a WAR file The problem there is that we do still have a WAR file and we do still want logging jars to be easily swapped in trunk.
          Hide
          Uwe Schindler added a comment -

          The WAR file is no longer deployed to Maven.

          Show
          Uwe Schindler added a comment - The WAR file is no longer deployed to Maven.
          Hide
          Steve Rowe added a comment -

          The WAR file is no longer deployed to Maven.

          Uwe, the WAR file is still deployed to Maven, and will be for all 4.X releases, e.g. 4.7.1: http://central.maven.org/maven2/org/apache/solr/solr/4.7.1/solr-4.7.1.war

          Show
          Steve Rowe added a comment - The WAR file is no longer deployed to Maven. Uwe, the WAR file is still deployed to Maven, and will be for all 4.X releases, e.g. 4.7.1: http://central.maven.org/maven2/org/apache/solr/solr/4.7.1/solr-4.7.1.war
          Hide
          Mark Miller added a comment -

          He was just talking about 5x Got pulled out of context a bit.

          Show
          Mark Miller added a comment - He was just talking about 5x Got pulled out of context a bit.
          Hide
          Steve Rowe added a comment -

          He was just talking about 5x Got pulled out of context a bit.

          I thought maybe that's what he meant, but I wanted to make a definitive statement to make sure there are no misunderstandings.

          When building the WAR file in 4.x we should exclude it somehow via Maven Magic(tm).

          The war deployed to Maven Central is built using Ant, and the Ant build already excludes all the SLF4J jars - the patch doesn't change this.

          For building the war using the Maven build, no Maven Magic required - I applied the patch to branch_4x and built the war, and the SLF4J jars are not included. The war's POM lists all SLF4J dependencies as optional. And the Solrj POM does not list org.slf4j:slf4j-api as optional.

          I'll commit to trunk and backport to branch_4x and the lucene_solr_4_7 branch.

          Show
          Steve Rowe added a comment - He was just talking about 5x Got pulled out of context a bit. I thought maybe that's what he meant, but I wanted to make a definitive statement to make sure there are no misunderstandings. When building the WAR file in 4.x we should exclude it somehow via Maven Magic(tm). The war deployed to Maven Central is built using Ant, and the Ant build already excludes all the SLF4J jars - the patch doesn't change this. For building the war using the Maven build, no Maven Magic required - I applied the patch to branch_4x and built the war, and the SLF4J jars are not included. The war's POM lists all SLF4J dependencies as optional. And the Solrj POM does not list org.slf4j:slf4j-api as optional. I'll commit to trunk and backport to branch_4x and the lucene_solr_4_7 branch.
          Hide
          Uwe Schindler added a comment -

          I thought maybe that's what he meant, but I wanted to make a definitive statement to make sure there are no misunderstandings.

          I was referring to trunk. There we no longer create and store the WAR in Maven.

          But the whole issue here is not really related to the WAR file: This is about making slf4j-log API non-optional for the solrj-Module. For using solrj with your project, you definitely need slf4j-api, otherwise you will get ClassNotFoundExceptions when starting the code or run tests (because solrj hardly depends on the log API).

          Show
          Uwe Schindler added a comment - I thought maybe that's what he meant, but I wanted to make a definitive statement to make sure there are no misunderstandings. I was referring to trunk. There we no longer create and store the WAR in Maven. But the whole issue here is not really related to the WAR file: This is about making slf4j-log API non-optional for the solrj-Module. For using solrj with your project, you definitely need slf4j-api, otherwise you will get ClassNotFoundExceptions when starting the code or run tests (because solrj hardly depends on the log API).
          Hide
          Uwe Schindler added a comment -

          For building the war using the Maven build, no Maven Magic required - I applied the patch to branch_4x and built the war, and the SLF4J jars are not included. The war's POM lists all SLF4J dependencies as optional. And the Solrj POM does not list org.slf4j:slf4j-api as optional.

          I will check this in a minute!

          Show
          Uwe Schindler added a comment - For building the war using the Maven build, no Maven Magic required - I applied the patch to branch_4x and built the war, and the SLF4J jars are not included. The war's POM lists all SLF4J dependencies as optional. And the Solrj POM does not list org.slf4j:slf4j-api as optional. I will check this in a minute!
          Hide
          Uwe Schindler added a comment -

          Hi Steve,
          I applied the patch to 4.7 and fixed the diamonds. After that I ran it and checked the POM output. It looks fine to me, solrj has the required dependency.

          But I think the whole magic is not needed at all. Because solrj is required by almost all projects (contrib, solr-core,..) the declaration of "optional" there is not really needed? I think only the WAR file should have it optional, all others get it required, because they depend on solrj (so when you compile or use solr-core, it should also fetch solrj, if I understand it correctly?

          Please correct me if this is not true.

          In my opinion, all of solr should depend on slf4j-api (because its used as a hard dependency, it is not optional!). Only the WAR file should exclude it by making it optional.

          Show
          Uwe Schindler added a comment - Hi Steve, I applied the patch to 4.7 and fixed the diamonds. After that I ran it and checked the POM output. It looks fine to me, solrj has the required dependency. But I think the whole magic is not needed at all. Because solrj is required by almost all projects (contrib, solr-core,..) the declaration of "optional" there is not really needed? I think only the WAR file should have it optional, all others get it required, because they depend on solrj (so when you compile or use solr-core, it should also fetch solrj, if I understand it correctly? Please correct me if this is not true. In my opinion, all of solr should depend on slf4j-api (because its used as a hard dependency, it is not optional!). Only the WAR file should exclude it by making it optional.
          Hide
          Steve Rowe added a comment -

          In my opinion, all of solr should depend on slf4j-api (because its used as a hard dependency, it is not optional!). Only the WAR file should exclude it by making it optional.

          I agree. I'll fix.

          Show
          Steve Rowe added a comment - In my opinion, all of solr should depend on slf4j-api (because its used as a hard dependency, it is not optional!). Only the WAR file should exclude it by making it optional. I agree. I'll fix.
          Hide
          Uwe Schindler added a comment -

          In my opinion, all of solr should depend on slf4j-api (because its used as a hard dependency, it is not optional!). Only the WAR file should exclude it by making it optional.

          This is, by the way, how it looked like in Solr 4.6.

          Show
          Uwe Schindler added a comment - In my opinion, all of solr should depend on slf4j-api (because its used as a hard dependency, it is not optional!). Only the WAR file should exclude it by making it optional. This is, by the way, how it looked like in Solr 4.6.
          Hide
          Steve Rowe added a comment -

          Separate trunk and branch_4x patches, since trunk doesn't have a war POM.

          On trunk, org.slf4j:slf4j-api is a transitive dependency (not optional) in all modules in which it is a dependency.

          On branch_4x, org.slf4j:slf4j-api is a transitive dependency (not optional) in all modules in which it is a dependency, except for the war POM, where it is optional. When I build the war using Maven after applying the branch_4x patch, it does not contain the slf4j-api jar.

          Committing shortly to trunk and branch_4x, then backporting from branch_4x to the lucene_solr_4_7 branch.

          Show
          Steve Rowe added a comment - Separate trunk and branch_4x patches, since trunk doesn't have a war POM. On trunk, org.slf4j:slf4j-api is a transitive dependency (not optional) in all modules in which it is a dependency. On branch_4x, org.slf4j:slf4j-api is a transitive dependency (not optional) in all modules in which it is a dependency, except for the war POM, where it is optional. When I build the war using Maven after applying the branch_4x patch, it does not contain the slf4j-api jar. Committing shortly to trunk and branch_4x, then backporting from branch_4x to the lucene_solr_4_7 branch.
          Hide
          ASF subversion and git services added a comment -

          Commit 1584473 from sarowe@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1584473 ]

          SOLR-5950: Maven config: make the org.slf4j:slf4j-api dependency transitive (i.e., not optional) in all modules in which it's a dependency, including solrj, except for the WAR, where it will remain optional.

          Show
          ASF subversion and git services added a comment - Commit 1584473 from sarowe@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1584473 ] SOLR-5950 : Maven config: make the org.slf4j:slf4j-api dependency transitive (i.e., not optional) in all modules in which it's a dependency, including solrj, except for the WAR, where it will remain optional.
          Hide
          ASF subversion and git services added a comment -

          Commit 1584474 from sarowe@apache.org in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1584474 ]

          SOLR-5950: Maven config: make the org.slf4j:slf4j-api dependency transitive (i.e., not optional) in all modules in which it's a dependency, including solrj, except for the WAR, where it will remain optional. (merged trunk r1584473)

          Show
          ASF subversion and git services added a comment - Commit 1584474 from sarowe@apache.org in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1584474 ] SOLR-5950 : Maven config: make the org.slf4j:slf4j-api dependency transitive (i.e., not optional) in all modules in which it's a dependency, including solrj, except for the WAR, where it will remain optional. (merged trunk r1584473)
          Hide
          ASF subversion and git services added a comment -

          Commit 1584480 from sarowe@apache.org in branch 'dev/branches/lucene_solr_4_7'
          [ https://svn.apache.org/r1584480 ]

          SOLR-5950: Maven config: make the org.slf4j:slf4j-api dependency transitive (i.e., not optional) in all modules in which it's a dependency, including solrj, except for the WAR, where it will remain optional. (merged branch_4x r1584474)

          Show
          ASF subversion and git services added a comment - Commit 1584480 from sarowe@apache.org in branch 'dev/branches/lucene_solr_4_7' [ https://svn.apache.org/r1584480 ] SOLR-5950 : Maven config: make the org.slf4j:slf4j-api dependency transitive (i.e., not optional) in all modules in which it's a dependency, including solrj, except for the WAR, where it will remain optional. (merged branch_4x r1584474)
          Hide
          Steve Rowe added a comment -

          Committed to trunk, branch_4x and the lucene_solr_4_7 branch.

          Thanks Uwe!

          Show
          Steve Rowe added a comment - Committed to trunk, branch_4x and the lucene_solr_4_7 branch. Thanks Uwe!
          Hide
          Uwe Schindler added a comment -

          Thanks Steve!

          Show
          Uwe Schindler added a comment - Thanks Steve!
          Hide
          Uwe Schindler added a comment -

          Close issue after release of 4.8.0

          Show
          Uwe Schindler added a comment - Close issue after release of 4.8.0

            People

            • Assignee:
              Steve Rowe
              Reporter:
              Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development