Incorrect source compatibility breakage reported when interface moved into another abstract class

[vmassol]

Here's a difference of a breakage that we didn't have with CLIRR and that is breaking our build with japicmp.

**** MODIFIED CLASS: PUBLIC org.xwiki.platform.flavor.script.FlavorManagerScriptService  (not serializable)
        ---* REMOVED INTERFACE: org.xwiki.script.service.ScriptService

More precisely here's what we had before: `public class FlavorManagerScriptService implements ScriptService

And here's what we changed it to: public class FlavorManagerScriptService extends AbstractExtensionScriptService and we have public abstract class AbstractExtensionScriptService implements ScriptService.

Thus I believe it's not correct to say that source compatibility was broken. Is that correct?

Thanks

[siom79]

Fixed this on the develop branch.

If you like, you can try it:

git clone https:/siom79/japicmp.git
cd japicmp
git checkout develop
mvn install

[vmassol]

Thanks a lot for all the quick fixes Martin! Tomorrow I'll be in meeting most of the day. I'll try to test the changes in the evening though.

[vmassol]

Ok I've been able to test it and I'm still getting errors that shouldn't be there:

+++* NEW CLASS: PUBLIC(+) org.xwiki.platform.flavor.job.FlavorSearchRequest  (compatible)
        +++  NEW INTERFACE: org.xwiki.job.Request
        +++  NEW INTERFACE: org.xwiki.extension.job.ExtensionRequest
        +++  NEW INTERFACE: java.io.Serializable
        +++* NEW SUPERCLASS: org.xwiki.extension.job.InstallRequest
        +++  NEW CONSTRUCTOR: PUBLIC(+) FlavorSearchRequest()
        +++  NEW CONSTRUCTOR: PUBLIC(+) FlavorSearchRequest(org.xwiki.job.Request)
        +++  NEW ANNOTATION: org.xwiki.stability.Unstable
***  MODIFIED CLASS: PUBLIC org.xwiki.platform.flavor.script.FlavorManagerScriptService  (not serializable)
        ***  MODIFIED SUPERCLASS: org.xwiki.extension.script.AbstractExtensionScriptService (<- java.lang.Object)
        +++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String ROLEHINT
        +++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String SEARCH_ID
        ===  UNCHANGED METHOD: PUBLIC org.xwiki.extension.repository.result.IterableResult getFlavors(org.xwiki.platform.flavor.FlavorQuery)
                +++  NEW ANNOTATION: java.lang.Deprecated
        ---  REMOVED METHOD: PUBLIC(-) java.lang.Exception getLastError()
        +++  NEW METHOD: PUBLIC(+) org.xwiki.platform.flavor.internal.job.FlavorSearchStatus getSearchValidFlavorsStatus(java.lang.String)
        +++  NEW METHOD: PUBLIC(+) org.xwiki.platform.flavor.internal.job.FlavorSearchStatus getSearchValidFlavorsStatus()
        +++  NEW METHOD: PUBLIC(+) java.lang.String getSearchValidFlavorsStatusURL(java.lang.String)
        +++  NEW METHOD: PUBLIC(+) java.lang.String getSearchValidFlavorsStatusURL()
        +++  NEW METHOD: PUBLIC(+) java.util.List getValidExtensions()
        +++  NEW METHOD: PUBLIC(+) org.xwiki.extension.repository.result.IterableResult searchFlavors(org.xwiki.platform.flavor.FlavorQuery)
        +++  NEW METHOD: PUBLIC(+) org.xwiki.job.Job searchValidFlavors(java.lang.String)
        +++  NEW METHOD: PUBLIC(+) org.xwiki.job.Job searchValidFlavors()

I got:

[ERROR] Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.7.2-SNAPSHOT:cmp (default) on project xwiki-platform-flavor-api: Breaking the build because there is at least one source incompatible class: org.xwiki.platform.flavor.job.FlavorSearchRequest -> [Help 1]

The are no backward compat issues with those classes normally (CLIRR doesn't fail for example).

[siom79]

The output indicates that the superclass org.xwiki.extension.job.InstallRequest has changed source incompatible. If this is the case it may affect the new class org.xwiki.platform.flavor.job.FlavorSearchRequest as you could cast FlavorSearchRequest to InstallRequest and provoke the incompatibility.

On which module do you get this output and how do I activate the japicmp plugin?

Btw: The HTML report as well as the XML report do output more information than the simple diff format.

[vmassol]

On which module do you get this output and how do I activate the japicmp plugin?

If you wish to reproduce, you can checkout:

• xwiki-commons (https:/xwiki/xwiki-commons), branch XCOMMONS-891 and build the top level pom (mvn clean install -N)
• xwiki-platform (https:/xwiki/xwiki-platform), branch XCOMMONS-891 and go to xwiki-platform-core/xwiki-platform-legacy/xwiki-platform-legacy-oldcore and run mvn clean verify -Dxwiki.clirr.skip=true

Thanks

[siom79]

Thank you for providing the detailed information.

Running mvn clean verify -Dxwiki.clirr.skip=true on module xwiki-platform-legacy-oldcore (with version 0.7.1 of japicmp) did not break the build.

The class FlavorSearchRequest resides within xwiki-platform-core/xwiki-platform-flavor/xwiki-platform-flavor-api. Running on this module japicmp did break the build.

The point was that the interface ScriptService was moved to the abstract class AbstractExtensionScriptService. I have fixed this on the develop branch:

git clone https:/siom79/japicmp.git
cd japicmp
git checkout develop
mvn install

With this version (0.7.2-SNAPSHOT) japicmp runs through on xwiki-platform-flavor-api.

[vmassol]

Thanks Martin. Glad that I could help by flushing out all those builds. Thanks to your efforts japicmp should be a lot stronger now! :) I'm a bit surprised nobody found them before.

Now I have to be honest with you: on the xwiki side, we're evaluating a replacement for CLIRR and we've tested both japicmp and revapi (it's nice to have 2 capable frameworks for doing this! When I started searching I didn't think I could find a replacement to CLIRR so that a good surprise :).

ATM I have revapi fully working on the xwiki code base (they also had to fix a couple of bugs too ;)).

I'm keeping a comparisons between the 2 here if you're interested (and in case you wish to improve japicmp against revapi ;)): http://jira.xwiki.org/browse/XCOMMONS-891?focusedCommentId=90493&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-90493

It's hard to choose but at the moment I'm leaning a bit more towards revapi. A big plus is the error reporting and the greater maturity.

On both sides, it's great to see that you guys are doing an amazing support. If you ever need help with XWiki I'll be therer to help out!

Thanks

[vmassol]

Waiting for the 0.7.2 release to try it out. Thanks

[siom79]

Ok, I am disappointed to hear that you tend to use revapi instead of japicmp. But thank you for providing feedback and making japicmp a bit more stable than it was before.

At this point I have to mention that support for source compatibility checks were introduced with 0.7.0, just mid of February. So it was very fresh anyhow and you were mabe the first that tried this new feature on a mature code base with with lots of changes. Thanks for the reportings anyhow.

The error messages in case of detected source incompatibilites can be improved.

Btw: I just run mvn verify -Dxwiki.clirr.skip=true -DskipTests -Dxwiki.revapi.skip=true -Dxwiki.japicmp.skip=false on xwiki-platform-core with BUILD SUCCESS using the latest 0.7.2-SNAPSHOT.

[vmassol]

Ok, I am disappointed to hear that you tend to use revapi instead of japicmp

I know and I feel bad about it, especially after all the great support you gave me. It's not 100% decided yet but close.

If you check xwiki's master branch you'll see that ATM I have committed support for both revapi and japicmp and I'm waiting for the 0.7.2 release to commit that change too (right now japicmp is turned off because it fails the build).

You guys should consider merging maybe :)

Btw: I just run mvn verify -Dxwiki.clirr.skip=true -DskipTests -Dxwiki.revapi.skip=true -Dxwiki.japicmp.skip=false on xwiki-platform-core with BUILD SUCCESS using the latest 0.7.2-SNAPSHOT.

Great job!

[siom79]

I just tried japicmp 0.7.2-SNAPSHOT on the master branch. The plugin did find the following incompatibtilites and therefore broke the build. My investigations turned out that the reported incompatibilties are correct (please note the improved error reporting):

org.xwiki.platform.flavor.FlavorManager.searchFlavors(org.xwiki.platform.flavor.FlavorQuery):METHOD_ADDED_TO_INTERFACE
org.xwiki.chart.ChartGenerator.COLORS_PARAM:FIELD_REMOVED

Beyond that the following two classes have been removed (which also seems to be correct):

com.xpn.xwiki.plugin.diff.DiffPlugin
com.xpn.xwiki.plugin.diff.DiffPluginApi

On the following modules I had to turn off the plugin because you have configured the packing type to maven-plugin but delacred in the base pom.xml that all artifact files end with .${project.packaging}.

xwiki-platform-tool-packager-plugin
xwiki-platform-tool-xmldoc-update-plugin

Therefore the file ${project.build.directory}/${project.artifactId}-${project.version}.${project.packaging} could not be found (also correct because the artifact did end with jar).

The packaging modules produce .deb files which can also be exclude from japicmp analysis. Hence I configured to skip the following modules for japicmp too:

xwiki-platform-distribution-debian

With the following configuration japicmp did run through on the master branch:

              <excludes>
                <exclude>*.internal.*</exclude>
                <exclude>*.test.*</exclude>
        <exclude>com.xpn.xwiki.plugin.diff.DiffPlugin</exclude>
        <exclude>com.xpn.xwiki.plugin.diff.DiffPluginApi</exclude>
        <exclude>org.xwiki.chart.ChartGenerator</exclude>
        <exclude>org.xwiki.platform.flavor.FlavorManager</exclude>
              </excludes>

[vmassol]

Actually, on master, the previous revision compared to is now 8.0 (was 7.4.2 before) so there shouldn't be any violations reported (your build was probably using the commons top level pom you built from the XCOMMONS-891 branch, that's why japicmp was comparing to 7.4.2).

Regarding the required override, yes it's a pain, this is what I meant by: "no way to specify def of artifact compared centrally for all cases (issue with packaging type = maven-archetype)" on http://jira.xwiki.org/browse/XCOMMONS-891?focusedCommentId=90493&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-90493

It would be great if japicmp was offering a way to configure the mapping in the top level pom (i.e. centrally) or if it could auto-discover artifact extensions.

Thanks!