Add revapi binary and source backwards compatibility check to build.

diegov commented 3 years ago

Description of the change

This adds a revapi (https://github.com/revapi/revapi) binary and compile level compatibility check to the gradle build.

Revapi seems to be the most straight forward and maintained Java binary compatibility checker available. Based on my tests it has very good (if not complete) coverage of the Java spec's "Binary Compatiblity" section (https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html)

The gradle plugin will automatically check the build against the most recent git tag, which works very well in our case since all published versions have tags. It will skip checks when a published artifact does not exist for a given project, so it won't break when new projects are added, but will automatically start checking them as soon as we have published an initial version.

Breaking changes can be added to an exclusion list, so it won't prevent us from making changes intentionally, but hopefully it will catch accidental compatibility breakages.

Type of change

[ ] Bug fix (non-breaking change that fixes an issue)
[x] New feature (non-breaking change that adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Checklists

Development

[x] Lint rules pass locally
[ ] The code changed/added as part of this pull request has been covered with tests
[x] All tests related to the changed code pass in development

Code review

[ ] This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
[x] "Ready for review" label attached to the PR and reviewers mentioned in a comment
[ ] Changes have been reviewed by at least one other engineer
[x] Issue from task tracker has a link to this pull request

diegov commented 3 years ago

Related to [ch83040]

shortcut-integration[bot] commented 3 years ago

This pull request has been linked to Clubhouse Story #83040: DX: rollbar-java - check binary and source compatibility against latest published version in GH action.

diegov commented 3 years ago

Nice @diegov ! One question, will this task be executed in every PR? What would happen if a PR breaks the compatibility? Is there any plan for handling those cases?

It will run for each PR. If a PR breaks compatibility, and we're OK with it, there is a gradle command which is part of the plugin, to accept all breaking changes:

./gradlew :project:revapiAcceptAllBreaks

Or alternatively, each change can be accepted individually:

./gradlew revapiAcceptBreak --justification "{why this is ok}" \
        --code "{revapi check code}" \
        --old "{optional revapi description of old element}" \
        --new "{optional revapi description of new element}"

These commands create or update the project's ignore file, which must be added to the commit, and makes the check pass.

I think this is a pretty decent workflow overall.

diegov commented 3 years ago

The commands I pasted above are written to the build log (with the specific details of the error, not as templates) as part of the error if the build fails, so it's clear what to do when it fails.

$ ./gradlew rollbar-api:revapi
> Task :rollbar-api:revapi FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':rollbar-api:revapi'.
> There were Java public API/ABI breaks reported by revapi:

  java.method.numberOfParametersChanged: The number of parameters of the method have changed.

  old: method java.lang.String com.rollbar.api.payload.data.Server::getHost()
  new: method java.lang.String com.rollbar.api.payload.data.Server::getHost(int)

  SOURCE: BREAKING, BINARY: BREAKING

  From old archive: rollbar-api-1.7.6.jar
  From new archive: rollbar-api-1.7.7-SNAPSHOT.jar

  If this is an acceptable break that will not harm your users, you can ignore it in future runs like so for:

    * Just this break:
        ./gradlew :rollbar-api:revapiAcceptBreak --justification "{why this break is ok}" \
          --code "java.method.numberOfParametersChanged" \
          --old "method java.lang.String com.rollbar.api.payload.data.Server::getHost()" \
          --new "method java.lang.String com.rollbar.api.payload.data.Server::getHost(int)"
    * All breaks in this project:
        ./gradlew :rollbar-api:revapiAcceptAllBreaks --justification "{why this break is ok}"
    * All breaks in all projects:
        ./gradlew revapiAcceptAllBreaks --justification "{why this break is ok}"
  ----------------------------------------------------------------------------------------------------

rollbar / rollbar-java