Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9746

Eclipse project broken due to duplicate package-info.java in LTR contrib

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      The eclipse project generated through ant eclipse seems to be broken, since there are errors complaining duplicate resources. The problem is that the following files have the same package and class names:

      ./solr/core/src/java/org/apache/solr/response/transform/package-info.java
      ./solr/contrib/ltr/src/java/org/apache/solr/response/transform/package-info.java
      ./solr/core/src/java/org/apache/solr/search/package-info.java
      ./solr/contrib/ltr/src/java/org/apache/solr/search/package-info.java
      

      Not sure if the idea project is affected.

        Issue Links

          Activity

          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 252c6e9385ba516887543eb1968c8654b35b2b81 in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=252c6e9 ]

          SOLR-8542, SOLR-9746: prefix solr/contrib/ltr's search and response.transform packages with ltr

          Show
          jira-bot ASF subversion and git services added a comment - Commit 252c6e9385ba516887543eb1968c8654b35b2b81 in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=252c6e9 ] SOLR-8542 , SOLR-9746 : prefix solr/contrib/ltr's search and response.transform packages with ltr
          Hide
          thetaphi Uwe Schindler added a comment - - edited

          Hi,

          as said before, I would not share packages between JAR files. This is a problem when we make the Lucene/Solr packages real Java 9 modules using module-info.java. Currently it is not a problem because the legacy JAR files use the legacy Java 9 classloader (all in one). But once they get modules every JAR files has its own classloader and the module system disallows to export same package (private packages are fine).

          So in short: We should avoid sharing packages and not add new shared ones! So the fix used here is fine.

          Show
          thetaphi Uwe Schindler added a comment - - edited Hi, as said before, I would not share packages between JAR files. This is a problem when we make the Lucene/Solr packages real Java 9 modules using module-info.java. Currently it is not a problem because the legacy JAR files use the legacy Java 9 classloader (all in one). But once they get modules every JAR files has its own classloader and the module system disallows to export same package (private packages are fine). So in short: We should avoid sharing packages and not add new shared ones! So the fix used here is fine.
          Hide
          ichattopadhyaya Ishan Chattopadhyaya added a comment - - edited

          Indeed, Alexandre. It seems that the difference between these cases and the LTR case was that the package-info.java file was duplicated in the LTR contrib. Perhaps just removing it from the contrib may have done the needful as well?

          Edit: Re-read Uwe's comment, and the committed fix seems the best long term.

          Show
          ichattopadhyaya Ishan Chattopadhyaya added a comment - - edited Indeed, Alexandre. It seems that the difference between these cases and the LTR case was that the package-info.java file was duplicated in the LTR contrib. Perhaps just removing it from the contrib may have done the needful as well? Edit: Re-read Uwe's comment, and the committed fix seems the best long term.
          Hide
          arafalov Alexandre Rafalovitch added a comment -

          If I understood the question correctly, then the following is the list of (non-test) java packages that appear in different directory roots. Package name first, root second. Alternatively, I could do a similar test against the shipped jars, if that's too much noise. I know I definitely have seen some duplication at that point as well.

          org/apache/lucene/analysis/icu - ./lucene/analysis/icu/src/java/
          org/apache/lucene/analysis/icu - ./lucene/analysis/icu/src/tools/java/
          org/apache/lucene/analysis/ja/util - ./lucene/analysis/kuromoji/src/java/
          org/apache/lucene/analysis/ja/util - ./lucene/analysis/kuromoji/src/tools/java/
          org/apache/lucene/analysis/standard - ./lucene/analysis/common/src/java/
          org/apache/lucene/analysis/standard - ./lucene/analysis/common/src/tools/java/
          org/apache/lucene/analysis/standard - ./lucene/core/src/java/
          org/apache/lucene/codecs - ./lucene/backward-codecs/src/java/
          org/apache/lucene/codecs - ./lucene/core/src/java/
          org/apache/lucene/codecs/lucene50 - ./lucene/backward-codecs/src/java/
          org/apache/lucene/codecs/lucene50 - ./lucene/core/src/java/
          org/apache/lucene/codecs/lucene60 - ./lucene/backward-codecs/src/java/
          org/apache/lucene/codecs/lucene60 - ./lucene/core/src/java/
          org/apache/lucene/codecs/lucene62 - ./lucene/backward-codecs/src/java/
          org/apache/lucene/codecs/lucene62 - ./lucene/core/src/java/
          org/apache/lucene/collation - ./lucene/analysis/common/src/java/
          org/apache/lucene/collation - ./lucene/analysis/icu/src/java/
          org/apache/lucene/collation/tokenattributes - ./lucene/analysis/common/src/java/
          org/apache/lucene/collation/tokenattributes - ./lucene/analysis/icu/src/java/
          org/apache/lucene/document - ./lucene/core/src/java/
          org/apache/lucene/document - ./lucene/misc/src/java/
          org/apache/lucene/document - ./lucene/sandbox/src/java/
          org/apache/lucene/index - ./lucene/core/src/java/
          org/apache/lucene/index - ./lucene/misc/src/java/
          org/apache/lucene/search - ./lucene/core/src/java/
          org/apache/lucene/search - ./lucene/misc/src/java/
          org/apache/lucene/search - ./lucene/sandbox/src/java/
          org/apache/lucene/spatial - ./lucene/spatial-extras/src/java/
          org/apache/lucene/spatial - ./lucene/spatial/src/java/
          org/apache/lucene/spatial/util - ./lucene/spatial-extras/src/java/
          org/apache/lucene/spatial/util - ./lucene/spatial/src/java/
          org/apache/lucene/store - ./lucene/core/src/java/
          org/apache/lucene/store - ./lucene/misc/src/java/
          org/apache/lucene/util/fst - ./lucene/core/src/java/
          org/apache/lucene/util/fst - ./lucene/misc/src/java/
          org/apache/solr/handler/component - ./solr/contrib/analytics/src/java/
          org/apache/solr/handler/component - ./solr/core/src/java/
          org/apache/solr/handler/dataimport - ./solr/contrib/dataimporthandler-extras/src/java/
          org/apache/solr/handler/dataimport - ./solr/contrib/dataimporthandler/src/java/
          org/apache/solr/response - ./solr/contrib/velocity/src/java/
          org/apache/solr/response - ./solr/core/src/java/
          org/apache/solr/schema - ./solr/contrib/analysis-extras/src/java/
          org/apache/solr/schema - ./solr/core/src/java/
          org/apache/solr/update/processor - ./solr/contrib/langid/src/java/
          org/apache/solr/update/processor - ./solr/core/src/java/

          Show
          arafalov Alexandre Rafalovitch added a comment - If I understood the question correctly, then the following is the list of (non-test) java packages that appear in different directory roots. Package name first, root second. Alternatively, I could do a similar test against the shipped jars, if that's too much noise. I know I definitely have seen some duplication at that point as well. org/apache/lucene/analysis/icu - ./lucene/analysis/icu/src/java/ org/apache/lucene/analysis/icu - ./lucene/analysis/icu/src/tools/java/ org/apache/lucene/analysis/ja/util - ./lucene/analysis/kuromoji/src/java/ org/apache/lucene/analysis/ja/util - ./lucene/analysis/kuromoji/src/tools/java/ org/apache/lucene/analysis/standard - ./lucene/analysis/common/src/java/ org/apache/lucene/analysis/standard - ./lucene/analysis/common/src/tools/java/ org/apache/lucene/analysis/standard - ./lucene/core/src/java/ org/apache/lucene/codecs - ./lucene/backward-codecs/src/java/ org/apache/lucene/codecs - ./lucene/core/src/java/ org/apache/lucene/codecs/lucene50 - ./lucene/backward-codecs/src/java/ org/apache/lucene/codecs/lucene50 - ./lucene/core/src/java/ org/apache/lucene/codecs/lucene60 - ./lucene/backward-codecs/src/java/ org/apache/lucene/codecs/lucene60 - ./lucene/core/src/java/ org/apache/lucene/codecs/lucene62 - ./lucene/backward-codecs/src/java/ org/apache/lucene/codecs/lucene62 - ./lucene/core/src/java/ org/apache/lucene/collation - ./lucene/analysis/common/src/java/ org/apache/lucene/collation - ./lucene/analysis/icu/src/java/ org/apache/lucene/collation/tokenattributes - ./lucene/analysis/common/src/java/ org/apache/lucene/collation/tokenattributes - ./lucene/analysis/icu/src/java/ org/apache/lucene/document - ./lucene/core/src/java/ org/apache/lucene/document - ./lucene/misc/src/java/ org/apache/lucene/document - ./lucene/sandbox/src/java/ org/apache/lucene/index - ./lucene/core/src/java/ org/apache/lucene/index - ./lucene/misc/src/java/ org/apache/lucene/search - ./lucene/core/src/java/ org/apache/lucene/search - ./lucene/misc/src/java/ org/apache/lucene/search - ./lucene/sandbox/src/java/ org/apache/lucene/spatial - ./lucene/spatial-extras/src/java/ org/apache/lucene/spatial - ./lucene/spatial/src/java/ org/apache/lucene/spatial/util - ./lucene/spatial-extras/src/java/ org/apache/lucene/spatial/util - ./lucene/spatial/src/java/ org/apache/lucene/store - ./lucene/core/src/java/ org/apache/lucene/store - ./lucene/misc/src/java/ org/apache/lucene/util/fst - ./lucene/core/src/java/ org/apache/lucene/util/fst - ./lucene/misc/src/java/ org/apache/solr/handler/component - ./solr/contrib/analytics/src/java/ org/apache/solr/handler/component - ./solr/core/src/java/ org/apache/solr/handler/dataimport - ./solr/contrib/dataimporthandler-extras/src/java/ org/apache/solr/handler/dataimport - ./solr/contrib/dataimporthandler/src/java/ org/apache/solr/response - ./solr/contrib/velocity/src/java/ org/apache/solr/response - ./solr/core/src/java/ org/apache/solr/schema - ./solr/contrib/analysis-extras/src/java/ org/apache/solr/schema - ./solr/core/src/java/ org/apache/solr/update/processor - ./solr/contrib/langid/src/java/ org/apache/solr/update/processor - ./solr/core/src/java/
          Hide
          cpoerschke Christine Poerschke added a comment -

          Thanks Ishan Chattopadhyaya for reporting this.

          Show
          cpoerschke Christine Poerschke added a comment - Thanks Ishan Chattopadhyaya for reporting this.
          Hide
          cpoerschke Christine Poerschke added a comment -

          ... I'd suggest to rename the packages in ltr to have "ltr" in its name. ...

          >> Done.

          ... If the package names are the same to work around package-protected access ...

          >> The package names were made the same only with the intention that then no solrconfig.xml changes would be required if ltr graduates from solr/contrib to solr/core. However, the same effect should similarly so be achievable and more clearly so via something like this as part of graduation

          # solr/contrib/.../LTRQParserPlugin.java
            package org.apache.solr.ltr.search;
          - public class LTRQParserPlugin extends QParserPlugin implements ... {
          - ...
          - }
          + @Deprecated // use {@link org.apache.solr.search.LTRQParserPlugin} instead
          + public class LTRQParserPlugin extends org.apache.solr.search.LTRQParserPlugin {
          + }
          
          + #solr/core/.../LTRQParserPlugin.java
          + package org.apache.solr.search;
          + public class LTRQParserPlugin extends QParserPlugin implements ... {
          + ...
          + }
          

          ... I know we have some modules in Lucene that also share packages, but we should work on fixing them. ...

          >> Interesting. I had a quick look around but couldn't find obvious examples, assuming .../abc/util and .../xyz/util would be considered to not share? If it doesn't exist already, shall we create a list of modules that would need attention then?

          Show
          cpoerschke Christine Poerschke added a comment - ... I'd suggest to rename the packages in ltr to have "ltr" in its name. ... >> Done. ... If the package names are the same to work around package-protected access ... >> The package names were made the same only with the intention that then no solrconfig.xml changes would be required if ltr graduates from solr/contrib to solr/core. However, the same effect should similarly so be achievable and more clearly so via something like this as part of graduation # solr/contrib/.../LTRQParserPlugin.java package org.apache.solr.ltr.search; - public class LTRQParserPlugin extends QParserPlugin implements ... { - ... - } + @Deprecated // use {@link org.apache.solr.search.LTRQParserPlugin} instead + public class LTRQParserPlugin extends org.apache.solr.search.LTRQParserPlugin { + } + #solr/core/.../LTRQParserPlugin.java + package org.apache.solr.search; + public class LTRQParserPlugin extends QParserPlugin implements ... { + ... + } ... I know we have some modules in Lucene that also share packages, but we should work on fixing them. ... >> Interesting. I had a quick look around but couldn't find obvious examples, assuming .../abc/util and .../xyz/util would be considered to not share? If it doesn't exist already, shall we create a list of modules that would need attention then?
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 86a515789f6e4626d71480c7fdf38c33b71ded93 in lucene-solr's branch refs/heads/master from Christine Poerschke
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=86a5157 ]

          SOLR-8542, SOLR-9746: prefix solr/contrib/ltr's search and response.transform packages with ltr

          Show
          jira-bot ASF subversion and git services added a comment - Commit 86a515789f6e4626d71480c7fdf38c33b71ded93 in lucene-solr's branch refs/heads/master from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=86a5157 ] SOLR-8542 , SOLR-9746 : prefix solr/contrib/ltr's search and response.transform packages with ltr
          Hide
          thetaphi Uwe Schindler added a comment -

          This is a clear bug in the package structure! This will lead to problems with Java 9's module system (once we migrate to it), too. It should generally be avoided to have same Java package in different JAR files / modules. I'd suggest to rename the packages in ltr to have "ltr" in its name. If the package names are the same to work around package-protected access, this is likely to break with Java 9, too where each modular JAR file gets its own classloader.

          I know we have some modules in Lucene that also share packages, but we should work on fixing them. This also causes problems with generating Javadocs, because it causes broken links.

          Show
          thetaphi Uwe Schindler added a comment - This is a clear bug in the package structure! This will lead to problems with Java 9's module system (once we migrate to it), too. It should generally be avoided to have same Java package in different JAR files / modules. I'd suggest to rename the packages in ltr to have "ltr" in its name. If the package names are the same to work around package-protected access, this is likely to break with Java 9, too where each modular JAR file gets its own classloader. I know we have some modules in Lucene that also share packages, but we should work on fixing them. This also causes problems with generating Javadocs, because it causes broken links.

            People

            • Assignee:
              cpoerschke Christine Poerschke
              Reporter:
              ichattopadhyaya Ishan Chattopadhyaya
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development