search

siom79 / japicmp

Comparison of two versions of a jar archive
https://siom79.github.io/japicmp
Apache License 2.0
701 stars 107 forks source link

New abstract methods on interface are ignored and not detected as a change #339

Closed idelpivnitskiy closed 1 year ago

idelpivnitskiy commented 1 year ago

Tried with japicmp 0.16.0 and 0.15.3. OpenJDK 11.0.11.

I don’t know how to provide a standalone reproducer, but will describe steps how we observed the issue in our repo.

We added 3 new abstract methods for H2ProtocolConfig interface (lives in servicetalk-http-netty module) in PR#2341.

We run our japicmp.sh script and for servicetalk-http-netty module the output was:

Comparing binary compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar
No changes.

This was unexpected result, new methods were not detected. We tried 0.15.3 originally and upgraded to 0.16.0, result was the same.

  1. We added default impl for these 3 methods in PR#2361. After that the output became expected:
Comparing binary compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar
***! MODIFIED INTERFACE: PUBLIC ABSTRACT io.servicetalk.http.netty.H2ProtocolConfig  (not serializable)
        ===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
        ===  UNCHANGED SUPERCLASS: java.lang.Object (<- java.lang.Object)
        +++! NEW METHOD: PUBLIC(+) int flowControlQuantum()
        +++! NEW METHOD: PUBLIC(+) int flowControlWindowIncrement()
        +++! NEW METHOD: PUBLIC(+) io.servicetalk.http.api.Http2Settings initialSettings()

To reproduce:

  1. git clone git@github.com:apple/servicetalk.git
  2. cd servicetalk
  3. git checkout 07534e9cf
  4. ./gradlew assemble --rerun-tasks
  5. ./scripts/japicmp.sh 0.42.16 local
  6. check logs for servicetalk-http-netty module, it says "No changes", while we expect it to detect 3 new methods on io.servicetalk.http.netty.H2ProtocolConfig.
siom79 commented 1 year ago

I don't know what exactly your script japicmp.sh does, but when I issue the following command I see the three methods you have mentioned:

$ java -jar japicmp-0.16.0-jar-with-dependencies.jar -o servicetalk-http-netty-0.42.16.jar -n servicetalk-http-netty-0.42.17.jar --ignore-missing-classes -m
Comparing source compatibility of /home/mmois/Downloads/servicetalk-http-netty-0.42.17.jar against /home/mmois/Downloads/servicetalk-http-netty-0.42.16.jar
WARNING: You are using the option '--ignore-missing-classes', i.e. superclasses and interfaces that could not be found on the classpath are ignored. Hence changes caused by these superclasses and interfaces are not reflected in the output.
***! MODIFIED INTERFACE: PUBLIC ABSTRACT io.servicetalk.http.netty.H2ProtocolConfig  (not serializable)
===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++! NEW METHOD: PUBLIC(+) int flowControlQuantum()
+++! NEW METHOD: PUBLIC(+) int flowControlWindowIncrement()
+++! NEW METHOD: PUBLIC(+) io.servicetalk.http.api.Http2Settings initialSettings()
***  MODIFIED CLASS: PUBLIC FINAL io.servicetalk.http.netty.H2ProtocolConfigBuilder  (not serializable)
===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++  NEW METHOD: PUBLIC(+) io.servicetalk.http.netty.H2ProtocolConfigBuilder flowControlQuantum(int)
+++  NEW METHOD: PUBLIC(+) io.servicetalk.http.netty.H2ProtocolConfigBuilder flowControlWindowIncrement(int)
+++  NEW METHOD: PUBLIC(+) io.servicetalk.http.netty.H2ProtocolConfigBuilder initialSettings(io.servicetalk.http.api.Http2Settings)
idelpivnitskiy commented 1 year ago

Hi Martin,

Thank you for having a look. Your command compares 0.42.17 release with 0.42.16. It shows 3 new methods because before releasing 0.42.17 we added default implementations for those methods.

If you will try the steps provided, you will be able to compare 0.42.16 with an intermediate "snapshot" that you will build locally. That revision has the same 3 methods without default implementations. See:

The script downloads japicmp-0.16.0-jar-with-dependencies.jar, then builds a list of published artifacts based on servicetalk-bom.pom and compares those with the local build using this command.

If you don't want to deal with the script, use only steps 1-4 then run your command specifying path to the locally built artifact: 

$ java -jar japicmp-0.16.0-jar-with-dependencies.jar -o servicetalk-http-netty-0.42.16.jar -n ./servicetalk/servicetalk-http-netty/build/libs/servicetalk-http-netty-0.42.17-SNAPSHOT.jar --ignore-missing-classes -m
siom79 commented 1 year ago

Hi @idelpivnitskiy ,

I have checked out revision 07534e9cf, compiled it and compared the SNAPSHOT with the mentioned version 0.42.16:

$ java -jar ~/Downloads/japicmp-0.16.0-jar-with-dependencies.jar -o ~/Downloads/servicetalk-http-netty-0.42.16.jar -n servicetalk-http-netty-0.42.17-SNAPSHOT.jar -m --ignore-missing-classes
Comparing source compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar
WARNING: You are using the option '--ignore-missing-classes', i.e. superclasses and interfaces that could not be found on the classpath are ignored. Hence changes caused by these superclasses and interfaces are not reflected in the output.
**** MODIFIED INTERFACE: PUBLIC ABSTRACT io.servicetalk.http.netty.H2ProtocolConfig  (not serializable)
    ===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
    +++* NEW METHOD: PUBLIC(+) ABSTRACT(+) int flowControlQuantum()
    +++* NEW METHOD: PUBLIC(+) ABSTRACT(+) int flowControlWindowIncrement()
    +++* NEW METHOD: PUBLIC(+) ABSTRACT(+) io.servicetalk.http.api.Http2Settings initialSettings()
***  MODIFIED CLASS: PUBLIC FINAL io.servicetalk.http.netty.H2ProtocolConfigBuilder  (not serializable)
    ===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
    +++  NEW METHOD: PUBLIC(+) io.servicetalk.http.netty.H2ProtocolConfigBuilder flowControlQuantum(int)
    +++  NEW METHOD: PUBLIC(+) io.servicetalk.http.netty.H2ProtocolConfigBuilder flowControlWindowIncrement(int)
    +++  NEW METHOD: PUBLIC(+) io.servicetalk.http.netty.H2ProtocolConfigBuilder initialSettings(io.servicetalk.http.api.Http2Settings)
siom79 commented 1 year ago

I don't know what exactly your script japicmp.sh does, but when I issue the following command I see the three methods you have mentioned:

$ java -jar japicmp-0.16.0-jar-with-dependencies.jar -o
servicetalk-http-netty-0.42.16.jar -n servicetalk-http-netty-0.42.17.jar
--ignore-missing-classes -m
Comparing source compatibility of
/home/mmois/Downloads/servicetalk-http-netty-0.42.17.jar against
/home/mmois/Downloads/servicetalk-http-netty-0.42.16.jar
WARNING: You are using the option '--ignore-missing-classes', i.e.
superclasses and interfaces that could not be found on the classpath are
ignored. Hence changes caused by these superclasses and interfaces are not
reflected in the output.
***! MODIFIED INTERFACE: PUBLIC ABSTRACT
io.servicetalk.http.netty.H2ProtocolConfig  (not serializable)
===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++! NEW METHOD: PUBLIC(+) int flowControlQuantum()
+++! NEW METHOD: PUBLIC(+) int flowControlWindowIncrement()
+++! NEW METHOD: PUBLIC(+) io.servicetalk.http.api.Http2Settings
initialSettings()
***  MODIFIED CLASS: PUBLIC FINAL
io.servicetalk.http.netty.H2ProtocolConfigBuilder  (not serializable)
===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++  NEW METHOD: PUBLIC(+)
io.servicetalk.http.netty.H2ProtocolConfigBuilder flowControlQuantum(int)
+++  NEW METHOD: PUBLIC(+)
io.servicetalk.http.netty.H2ProtocolConfigBuilder
flowControlWindowIncrement(int)
+++  NEW METHOD: PUBLIC(+)
io.servicetalk.http.netty.H2ProtocolConfigBuilder
initialSettings(io.servicetalk.http.api.Http2Settings)

Am Mo., 19. Sept. 2022 um 19:21 Uhr schrieb Idel Pivnitskiy < @.***>:

Tried with japicmp 0.16.0 and 0.15.3. OpenJDK 11.0.11.

I don’t know how to provide a standalone reproducer, but will describe steps how we observed the issue in our repo.

We added 3 new abstract methods for H2ProtocolConfig interface (lives in servicetalk-http-netty module) in PR#2341 https://github.com/apple/servicetalk/pull/2341/files#diff-03ddce5278dccb15ba1e3b9f1ac4e6056998fc82bd2c9dbf0fdfd03a7fb88918 .

We run our japicmp.sh https://github.com/apple/servicetalk/blob/07534e9cf0470d98a00f1eaa416a9bb5f8f4ebaa/scripts/japicmp.sh script and for servicetalk-http-netty module the output was:

Comparing binary compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar

No changes.

This was unexpected result, new methods were not detected. We tried 0.15.3 originally and upgraded to 0.16.0, result was the same.

  1. We added default impl for these 3 methods in PR#2361 https://github.com/apple/servicetalk/pull/2361. After that the output became expected:

Comparing binary compatibility of servicetalk-http-netty-0.42.17-SNAPSHOT.jar against servicetalk-http-netty-0.42.16.jar

***! MODIFIED INTERFACE: PUBLIC ABSTRACT io.servicetalk.http.netty.H2ProtocolConfig (not serializable)

    ===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0

    ===  UNCHANGED SUPERCLASS: java.lang.Object (<- java.lang.Object)

    +++! NEW METHOD: PUBLIC(+) int flowControlQuantum()

    +++! NEW METHOD: PUBLIC(+) int flowControlWindowIncrement()

    +++! NEW METHOD: PUBLIC(+) io.servicetalk.http.api.Http2Settings initialSettings()

To reproduce:

  1. git clone @.***:apple/servicetalk.git
  2. cd servicetalk
  3. git checkout 07534e9cf
  4. ./gradlew assemble --rerun-tasks
  5. ./scripts/japicmp.sh 0.42.16 local
  6. check logs for servicetalk-http-netty module, it says "No changes", while we expect it to detect 3 new methods on io.servicetalk.http.netty.H2ProtocolConfig.

— Reply to this email directly, view it on GitHub https://github.com/siom79/japicmp/issues/339, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4U7NEKSSUIPIDLK6IIWD3V7COI3ANCNFSM6AAAAAAQQJXSMY . You are receiving this because you are subscribed to this thread.Message ID: @.***>