Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.6
    • Component/s: general/build
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      After the forbidden API checker was released separately from Lucene as a Google Code project (forked and improved), including Maven support, the checks on Lucene should be changed to work per-module.

      The reason for this is: The improved checker is more picky about e.g. extending classes that are forbidden or overriding methods and calling super.method() if they are on the forbidden signatures list. For these checks, it is not enough to have the class files and the rt.jar, you need the whole classpath. The forbidden APIs 1.0 now by default complains if classes are missing from the classpath.

      It is very hard with the module architecture of Lucene/Solr, to make a uber-classpath, instead the checks should be done per module, so the default compile/test classpath of the module can be used and no crazy path statements with */.jar are needed. This needs some refactoring in the exclusion lists, but the Lucene checks could be done by a macro in common-build, that allows custom exclusion lists for specific modules.

      Currently, the "strict" checking is disabled for Solr, so the checker only complains about missing classes but does not fail the build:

      -check-forbidden-java-apis:
      [forbidden-apis] Reading bundled API signatures: jdk-unsafe-1.6
      [forbidden-apis] Reading bundled API signatures: jdk-deprecated-1.6
      [forbidden-apis] Reading bundled API signatures: commons-io-unsafe-2.1
      [forbidden-apis] Reading API signatures: C:\Users\Uwe Schindler\Projects\lucene\trunk-lusolr3\lucene\tools\forbiddenApis\executors.txt
      [forbidden-apis] Reading API signatures: C:\Users\Uwe Schindler\Projects\lucene\trunk-lusolr3\lucene\tools\forbiddenApis\servlet-api.txt
      [forbidden-apis] Loading classes to check...
      [forbidden-apis] Scanning for API signatures and dependencies...
      [forbidden-apis] WARNING: The referenced class 'org.apache.lucene.analysis.uima.ae.AEProviderFactory' cannot be loaded. Please fix the classpath!
      [forbidden-apis] WARNING: The referenced class 'org.apache.lucene.analysis.uima.ae.AEProviderFactory' cannot be loaded. Please fix the classpath!
      [forbidden-apis] WARNING: The referenced class 'org.apache.lucene.analysis.uima.ae.AEProvider' cannot be loaded. Please fix the classpath!
      [forbidden-apis] WARNING: The referenced class 'org.apache.lucene.collation.ICUCollationKeyAnalyzer' cannot be loaded. Please fix the classpath!
      [forbidden-apis] Scanned 2177 (and 1222 related) class file(s) for forbidden API invocations (in 1.80s), 0 error(s).
      

      I added almost all missing jars, but those do not seem to be in the solr part of the source tree (i think they are only copied when building artifacts). With making the whole thing per module, we can use the default classpath of the module which makes it much easier.

      1. LUCENE-4753.patch
        22 kB
        Uwe Schindler
      2. LUCENE-4753.patch
        21 kB
        Uwe Schindler
      3. LUCENE-4753.patch
        21 kB
        Uwe Schindler

        Activity

        Hide
        Uwe Schindler added a comment -

        Recently on MacOSX, with the default heap size, we get OOMs while running forbidden-checker. So we should really do this now.

        My proposal: Move the forbidden targets into commons-build.xml on Lucene and Solr. Inside commons-build, also define some properties to make some excludes, so we can define per module, which patterns/filesets should be checked.

        Show
        Uwe Schindler added a comment - Recently on MacOSX, with the default heap size, we get OOMs while running forbidden-checker. So we should really do this now. My proposal: Move the forbidden targets into commons-build.xml on Lucene and Solr. Inside commons-build, also define some properties to make some excludes, so we can define per module, which patterns/filesets should be checked.
        Hide
        Uwe Schindler added a comment -

        The Maven builds are already per module! So we should get the file patterns and targets also synchronized with the definitions in the maven POMs - I have to say: in this case, the maven build is better than our ANT build Thanks Steve Rowe!!!

        Show
        Uwe Schindler added a comment - The Maven builds are already per module! So we should get the file patterns and targets also synchronized with the definitions in the maven POMs - I have to say: in this case, the maven build is better than our ANT build Thanks Steve Rowe !!!
        Hide
        Uwe Schindler added a comment -

        Patch. I will commit this soon if nobody objects.

        There is still room for improvements (e.g. we can now enable servlet-api checks in some Lucene modules that use servlets, or enable commons-io checks for lucene modules that use commons-io).

        Show
        Uwe Schindler added a comment - Patch. I will commit this soon if nobody objects. There is still room for improvements (e.g. we can now enable servlet-api checks in some Lucene modules that use servlets, or enable commons-io checks for lucene modules that use commons-io).
        Hide
        Uwe Schindler added a comment -

        New patch, removed useless dependency.

        Show
        Uwe Schindler added a comment - New patch, removed useless dependency.
        Hide
        Uwe Schindler added a comment -

        Final patch. Will commit in a moment.

        Show
        Uwe Schindler added a comment - Final patch. Will commit in a moment.
        Hide
        ASF subversion and git services added a comment -

        Commit 1540573 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1540573 ]

        LUCENE-4753: Run forbidden-apis Ant task per module. This allows more improvements and prevents OOMs after the number of class files raised recently

        Show
        ASF subversion and git services added a comment - Commit 1540573 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1540573 ] LUCENE-4753 : Run forbidden-apis Ant task per module. This allows more improvements and prevents OOMs after the number of class files raised recently
        Hide
        ASF subversion and git services added a comment -

        Commit 1540575 from Uwe Schindler in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1540575 ]

        Merged revision(s) 1540573 from lucene/dev/trunk:
        LUCENE-4753: Run forbidden-apis Ant task per module. This allows more improvements and prevents OOMs after the number of class files raised recently

        Show
        ASF subversion and git services added a comment - Commit 1540575 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1540575 ] Merged revision(s) 1540573 from lucene/dev/trunk: LUCENE-4753 : Run forbidden-apis Ant task per module. This allows more improvements and prevents OOMs after the number of class files raised recently
        Hide
        Uwe Schindler added a comment -

        FYI: I opened https://code.google.com/p/forbidden-apis/issues/detail?id=20 to improve the memory usage of forbidden-apis.

        Show
        Uwe Schindler added a comment - FYI: I opened https://code.google.com/p/forbidden-apis/issues/detail?id=20 to improve the memory usage of forbidden-apis.
        Hide
        Markus Jelsma added a comment -

        Hi Uwe, i can't compile anymore since something was changed to build.xml

        BUILD FAILED
        Target "install-forbidden-apis" does not exist in the project "solr". It is used from target "check-forbidden-apis".
        
        Total time: 0 seconds
        
        Show
        Markus Jelsma added a comment - Hi Uwe, i can't compile anymore since something was changed to build.xml BUILD FAILED Target "install-forbidden-apis" does not exist in the project "solr" . It is used from target "check-forbidden-apis" . Total time: 0 seconds
        Hide
        Uwe Schindler added a comment -

        Hi,

        which build target did you call from where? This outdated target "install-forbidden-apis" no longer exists (it was renamed). I looks like you have a checkout with mixed svn revisions or you have changed some build.xml files yourself and they conflicted.

        Be sure to:

        1. revert all changes (make sure you save your changes in a diff before doing this)
        2. update your checkout from the root folder (where lucene, dev-tools, and solr subfolders are visible). Updating only solr or lucene subfolder leads to inconsistency as dependencies inside ANT no longer work
        3. if nothing helps, use a fresh checkout and try again. You can apply the patch from step 1 to restore your changes.

        Jenkins already verified that everything is fine. I cannot find any problems, too: I can call "ant compile/test/check-forbidden-apis/..." from everywhere and it works.

        Show
        Uwe Schindler added a comment - Hi, which build target did you call from where? This outdated target "install-forbidden-apis" no longer exists (it was renamed). I looks like you have a checkout with mixed svn revisions or you have changed some build.xml files yourself and they conflicted. Be sure to: revert all changes (make sure you save your changes in a diff before doing this) update your checkout from the root folder (where lucene, dev-tools, and solr subfolders are visible). Updating only solr or lucene subfolder leads to inconsistency as dependencies inside ANT no longer work if nothing helps, use a fresh checkout and try again. You can apply the patch from step 1 to restore your changes. Jenkins already verified that everything is fine. I cannot find any problems, too: I can call "ant compile/test/check-forbidden-apis/..." from everywhere and it works.
        Hide
        Markus Jelsma added a comment -

        ant example from /solr. I svn upped my checkout not long ago and got no updated build.xml. I upped again and i finally received your commit. Svn must be behind.
        Thanks

        Show
        Markus Jelsma added a comment - ant example from /solr. I svn upped my checkout not long ago and got no updated build.xml. I upped again and i finally received your commit. Svn must be behind. Thanks
        Hide
        Uwe Schindler added a comment -

        It is still strange, because the whole change was one single commit. So you would have either nothing or all of the changes. From your error message, it looks as you may have only updated the lucene folder and not solr. Because this old target was only existent in lucene/common-build.xml; if this file was updated, solr would no longer find it with the old name.

        Show
        Uwe Schindler added a comment - It is still strange, because the whole change was one single commit. So you would have either nothing or all of the changes. From your error message, it looks as you may have only updated the lucene folder and not solr. Because this old target was only existent in lucene/common-build.xml; if this file was updated, solr would no longer find it with the old name.
        Hide
        Markus Jelsma added a comment -

        I usually update both but perhaps i didn't this time or i didn't get all commits, the latter sometimes happens and then i have to update twice.
        It's fixed now.

        Show
        Markus Jelsma added a comment - I usually update both but perhaps i didn't this time or i didn't get all commits, the latter sometimes happens and then i have to update twice. It's fixed now.

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development