API / binary compatibility checking tool via revapi gradle plugin #4638

kbendick · 2022-04-26T23:57:03Z

Purpose

As part of our journey to the 1.0 release, we'd like to have stability / binary compatibility guarantees for the api package.

Added

Adds a gradle plugin, that's currently still actively maintained, for the Revapi tool, for checking API and ABI compatibility and ensuring that any breaks are caught during gradlew check (or gradlew revapi).

It also allows for allowing overrides and setting binary compatibility checks against different versions / tags.

Info can be found here: https://github.com/palantir/gradle-revapi

Using

A generated override file is added that allows us to accept breaking changes and optionally denote why they're allowed.

This plugin requires that we point to some sort of base (to check binary / API compatibility from). I pointed it to 0.13.1 for this PR.

With this patch, if ./gradlew check is run, it fails because it can't determine the correct upstream version to compare against.

So I ran the following to generate the config file in ./palantir/revapi.yml.

./gradlew iceberg-api:revapiVersionOverride --replacement-version 0.13.1

Please feel free to take a look.

Why this plugin

MiMa doesn't support Gradle (at least not through any very official seeming plugins)
This plugin is actively maintained (commits seemingly daily) and we already use several palantir projects in our build / linter infrastructure.
The simplicity of the generated configuration file and the ability to run checks between different versions via gradle tasks.
The fact that this adds additional gradle tasks, but also runs during the check phase so that little to none of our CI needs to change.

Anybody who is interested, please take a look, play around and see how you like it.

cc @rdblue @danielcweeks @nastra @jackye1995 @snazy

kbendick · 2022-04-26T23:59:59Z

Another tool to consider is me.champeau.gradle.japicmp, which seems to be more for checking diffs than anything else, but was adopted by the OpenTelemetry project in the following PR and associated issue:

open-telemetry/opentelemetry-java#2692

open-telemetry/opentelemetry-java#3183

The rev-api plugin used in this PR is very actively maintained and relatively simple, but anybody who is interested might use that for comparison.

kbendick · 2022-04-27T00:01:45Z

The available tasks:

$ ./gradlew tasks --all | grep rev
iceberg-api:revapi
iceberg-api:revapiAcceptAllBreaks
iceberg-api:revapiAcceptBreak
iceberg-api:revapiAnalyze
iceberg-api:revapiVersionOverride

kbendick · 2022-04-27T00:02:23Z

Sample output from running `./gradlew check.

  ----------------------------------------------------------------------------------------------------
  java.class.defaultSerializationChanged: The default serialization ID for the class has changed. This means that the new version of the class is not deserializable from the byte stream of a serialized old class.

  old: class org.apache.iceberg.SortOrder
  new: class org.apache.iceberg.SortOrder

  SOURCE: EQUIVALENT, BINARY: EQUIVALENT, SEMANTIC: BREAKING

  From old archive: iceberg-api-0.13.1.jar
  From new archive: iceberg-api-0.14.0-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 :iceberg-api:revapiAcceptBreak --justification "{why this break is ok}" \
          --code "java.class.defaultSerializationChanged" \
          --old "class org.apache.iceberg.SortOrder" \
          --new "class org.apache.iceberg.SortOrder"
    * All breaks in this project:
        ./gradlew :iceberg-api:revapiAcceptAllBreaks --justification "{why this break is ok}"
    * All breaks in all projects:
        ./gradlew revapiAcceptAllBreaks --justification "{why this break is ok}"
  ----------------------------------------------------------------------------------------------------

danielcweeks · 2022-04-27T18:12:49Z

I really like this approach due to the simplicity and how well it integrates into the current build process.

+1 (will wait on other feedback)

rdblue · 2022-04-29T01:52:55Z

.palantir/revapi.yml

@@ -0,0 +1,2 @@
+versionOverrides:
+  org.apache.iceberg:iceberg-api:release-base-0.13.0: "0.13.1"


What is this doing?

The plug-in works by comparing the previous version with the current code, by fetching the previous version from maven.

The previous version is inferred based on the most recent tag, which for the master branch is release-base-0.xx normally.

However, that’s not an artifact available on maven.

So this sets the “old version” to compare for breaking changes to apache-iceberg-0.13.1 from maven.

See here: https://github.com/palantir/gradle-revapi#version-overrides

Also the Configuration section has a succinct description of how it works: https://github.com/palantir/gradle-revapi#configuration

“””
gradle-revapi should work out of the box for most uses cases once applied.
By default it compares against the previous version of the jar from the project it is applied in by finding the last tag using git describe.
However, if you need to need to override the artifact to compare against, you can do so.
“””

Here's the documentation on the actual underlying revapi tool: https://revapi.org/revapi-site/main/index.html

Is it possible to move this to a different directory? I isn't very obvious what to fix if this is in .palantir.

Looking through their source code, the .palantir directory is hardcoded: https://github.com/kbendick/gradle-revapi/blob/09039d215efbd41e25b6743409a12ceb22ca5d75/src/main/java/com/palantir/gradle/revapi/RevapiPlugin.java#L176-L178

We could either add a comment somewhere in the Gradle build file or possibly try to open a PR upstream.

That said, nobody should have to update the config file manually. It's all done via gradlew commands which autogenerates the file and updates it.

dev/.rat-excludes

kbendick · 2022-04-29T05:48:24Z

Another thought: This runs as part of gradlew check, so we don’t technically need to update CI.

However, we might want to add this as its CI step eventually.

The logic being that this check is relatively inexpensive, and so it could be placed first alongside the PR labeler. Then, if API compatibility fails, we could not run all of the expensive tests.

I wouldn’t add it as a standalone check just yet because it will get run as part of gradle check, but something to think about.

rdblue · 2022-05-03T01:59:40Z

.palantir/revapi.yml

@@ -0,0 +1,2 @@
+versionOverrides:


For the upstream version, I agree it should be 0.13.0, which is the tag you're using. But should this mark it as 0.13.1? What happens if we make the branch 0.13.0 instead?

This is marking it as 0.13.0 because of the output of git describe. I believe if we pushed a tag for release-base-0.13.1, it would use that instead.

However, it seems we can use an override in the build.gradle file instead: https://github.com/palantir/gradle-revapi#configuration

Let me try that.

Setting it to 0.13.0 then compares the output of current master (or the generated code from this branch) to the 0.13.0 artifact.

I set it by running ./gradlew iceberg-api:revapiVersionOverride --replacement-version 0.13.0

If we tagged a release-base-0.13.1, we'd also be able to compare that from within the same file.

I left the full output comparison in a gist. https://gist.github.com/kbendick/b9543ac8498bb06815a8c4e993f5c969

I think we want to compare against the last minor release right now. Later, we'll want to compare to the last major release.

kbendick · 2022-05-04T19:46:10Z

build.gradle

+  revapi {
+    oldGroup = 'org.apache.iceberg'
+    oldName = 'iceberg-api'
+    oldVersion = '0.13.0'
+  }


Using this seems to be not the best idea, as it would need to be updated manually across versions.

Using the gradle tasks doesn't update this, but we might consider using this for when we cut a release tag (not the release base off of master but the actual tag).

kbendick · 2022-05-04T20:05:40Z

Here's a gist comparing the output of 0.13.0 API module to the current 0.14.0-SNAPSHOT: https://gist.github.com/kbendick/b9543ac8498bb06815a8c4e993f5c969

This was generated by running ./gradlew revapi from this branch.

…pare against for changes

…iew output

…ce for now

rdblue · 2022-05-14T18:34:26Z

Thanks, @kbendick! This is great to have. I merged this. Do we expect builds to start failing now if there are binary breaking changes?

kbendick · 2022-05-14T18:48:27Z

Thanks, @kbendick! This is great to have. I merged this. Do we expect builds to start failing now if there are binary breaking changes?

Yes. In fact, builds might start failing now (the changes to gradle check were prossibly not run in this PR’s status checks).

If you run ./gradlew check it will run the api binary compatibility and then we can add justifications. The whole thing will tell us how.

I’ll hop on my computer and open a PR or if you make one then I’ll review it. Then from there it will be required. @rdblue

kbendick · 2022-05-14T18:50:07Z

But TLDR - We don’t have to add anything new to CI, this runs during gradle check.

KarlManong · 2022-06-02T07:31:14Z

Can't run build on branch master 9ab94f8 ubuntu 20.04 LTS .

$ ./gradlew build -x test -x integrationTest
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 59821  100 59821    0     0  61926      0 --:--:-- --:--:-- --:--:-- 61862
Downloading https://services.gradle.org/distributions/gradle-7.4.2-bin.zip
...........10%...........20%...........30%...........40%...........50%...........60%...........70%...........80%...........90%...........100%

Welcome to Gradle 7.4.2!

Here are the highlights of this release:
 - Aggregated test and JaCoCo reports
 - Marking additional test source directories as tests in IntelliJ
 - Support for Adoptium JDKs in Java toolchains

For more details see https://docs.gradle.org/7.4.2/release-notes.html

Starting a Gradle Daemon (subsequent builds will be faster)

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':iceberg-api:revapiAnalyze'.
> Failed to query the value of task ':iceberg-api:revapiAnalyze' property 'oldApiJars'.
   > Failed to query the value of extension 'revapi' property 'oldVersions'.
      > Failed running command:
                Command:[]
                Exit code: 128
                Stdout:
                Stderr:fatal: 没有标签能描述 '7ff467106431cdd8715aeec9b5f58458dc5d2102'。
        尝试 --always，或者创建一些标签。


* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

$ git describe --tags --abbrev=0
release-base-0.13.0

What did I miss? @kbendick

KarlManong · 2022-06-05T01:42:33Z

I do think it caused by palantir/gradle-revapi#550

kbendick · 2022-06-05T03:29:54Z

Oh I’m sorry I missed this.

If you’re on master, can you fetch all tags from the upstream repository? git fetch upstream. I believe you’re missing release-base-0.13.x branch @KarlManong.

Commenting on closed PRs is likely to not get seen. Probably better to open an issue. But @chenjunjiedada had this issue recently and they were missing the most recent release-base branch.

kbendick · 2022-06-05T03:31:27Z

Here’s an image of the tags needed. It’s just a release-base-0.13.x that’s needed. @KarlManong

EDIT: Couldn’t upload a picture, but…

If you do git tag | grep release, you should have at least release-base-0.13.0. That’s sufficient for the version function in build.gradle to pick up the current version and increment it to get 0.14.0-SNAPSHOT.

kbendick · 2022-06-05T03:36:03Z

Oh sorry. I see you do have a release-base.

I think maybe your gradle / maven dependency revolution (possibly in a fork) isn’t able to fetch the dependencies from the old version. Maybe a Firewall issue or maven / gradle configuration difference? Like I said, open an issue as a closed old PR is not the best place to discuss this.

But try running the build with —debug before opening an issue. It should try to fetch apache-iceberg-api:0.13.0 from maven.

KarlManong · 2022-06-05T03:57:22Z

Sorry.
I create an issue #4959

github-actions bot added the build label Apr 26, 2022

kbendick mentioned this pull request Apr 27, 2022

Migrate back to use ReusableArrayData in Flink #4624

Closed

2 tasks

rdblue reviewed Apr 29, 2022

View reviewed changes

dev/.rat-excludes Show resolved Hide resolved

kbendick force-pushed the api-compatability-add-gradle-revapi-plugin-v2-api-only branch from 7789671 to 5a452c2 Compare April 29, 2022 02:41

kbendick changed the title ~~[WIP] API / binary compatibility checking tool via revapi gradle plugin~~ API / binary compatibility checking tool via revapi gradle plugin Apr 29, 2022

rdblue reviewed May 3, 2022

View reviewed changes

kbendick force-pushed the api-compatability-add-gradle-revapi-plugin-v2-api-only branch from 5a452c2 to 22254d3 Compare May 4, 2022 19:14

kbendick commented May 4, 2022

View reviewed changes

kbendick added 15 commits May 9, 2022 12:46

Build - Add binary compatability checker via Gradle RevAPI plugin

ac81c2d

Add generated config file setting 0.13.1 Iceberg as the source to com…

c1dbc9a

…pare against for changes

Add revapi file to rat-excludes, as it's autogenerated

37eaf9e

Remove override file and use configuration for comparison

0c2c320

Try setting replacement version as 0.13.0 temporarily

84ad5ba

Run ./gradlew revapiVersionOverride --replacement-version 0.13.1 to v…

9b2f683

…iew output

Update build file to 0.13.1 and then see how that affects revapi file

ce64b57

Make the branch 0.13.0 instead of 0.13.1

684a11d

Change palantir file permissions

397fea8

Remove manually created revapi file

6202360

Add in regenerated revapi yaml file

fb545e5

Comment out the revapi section of build.gradle - leave it for referen…

f6bce55

…ce for now

Dont use the gradle build file configuration at all

152013f

Remove the version override that's not specified by release-base

afd204e

Remove the revapi section from the build file

13f3c41

kbendick force-pushed the api-compatability-add-gradle-revapi-plugin-v2-api-only branch from d9e75a0 to 13f3c41 Compare May 9, 2022 19:46

rdblue merged commit 4a0a3a1 into apache:master May 14, 2022

kbendick mentioned this pull request May 14, 2022

Core: Accept all API warnings in master #4770

Merged

kbendick deleted the api-compatability-add-gradle-revapi-plugin-v2-api-only branch June 5, 2022 03:34

KarlManong mentioned this pull request Jun 5, 2022

revapi plugin will not work in an non-english locale computer #4959

Closed

sdavids mentioned this pull request Mar 26, 2024

Make config file location configurable palantir/gradle-revapi#649

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API / binary compatibility checking tool via revapi gradle plugin #4638

API / binary compatibility checking tool via revapi gradle plugin #4638

kbendick commented Apr 26, 2022 •

edited

Loading

kbendick commented Apr 26, 2022 •

edited

Loading

kbendick commented Apr 27, 2022

kbendick commented Apr 27, 2022

danielcweeks commented Apr 27, 2022

rdblue Apr 29, 2022

kbendick Apr 29, 2022

kbendick Apr 29, 2022

kbendick Apr 29, 2022

rdblue May 3, 2022

kbendick May 4, 2022

kbendick commented Apr 29, 2022

rdblue May 3, 2022

kbendick May 4, 2022

kbendick May 4, 2022

kbendick May 4, 2022

kbendick May 4, 2022

rdblue May 14, 2022

kbendick May 4, 2022 •

edited

Loading

kbendick commented May 4, 2022

rdblue commented May 14, 2022

kbendick commented May 14, 2022

kbendick commented May 14, 2022

KarlManong commented Jun 2, 2022 •

edited

Loading

KarlManong commented Jun 5, 2022

kbendick commented Jun 5, 2022

kbendick commented Jun 5, 2022 •

edited

Loading

kbendick commented Jun 5, 2022 •

edited

Loading

KarlManong commented Jun 5, 2022

		@@ -0,0 +1,2 @@
		versionOverrides:
		org.apache.iceberg:iceberg-api:release-base-0.13.0: "0.13.1"

API / binary compatibility checking tool via revapi gradle plugin #4638

API / binary compatibility checking tool via revapi gradle plugin #4638

Conversation

kbendick commented Apr 26, 2022 • edited Loading

Purpose

Added

Using

Why this plugin

kbendick commented Apr 26, 2022 • edited Loading

kbendick commented Apr 27, 2022

kbendick commented Apr 27, 2022

danielcweeks commented Apr 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick commented Apr 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbendick May 4, 2022 • edited Loading

Choose a reason for hiding this comment

kbendick commented May 4, 2022

rdblue commented May 14, 2022

kbendick commented May 14, 2022

kbendick commented May 14, 2022

KarlManong commented Jun 2, 2022 • edited Loading

KarlManong commented Jun 5, 2022

kbendick commented Jun 5, 2022

kbendick commented Jun 5, 2022 • edited Loading

kbendick commented Jun 5, 2022 • edited Loading

KarlManong commented Jun 5, 2022

kbendick commented Apr 26, 2022 •

edited

Loading

kbendick commented Apr 26, 2022 •

edited

Loading

kbendick May 4, 2022 •

edited

Loading

KarlManong commented Jun 2, 2022 •

edited

Loading

kbendick commented Jun 5, 2022 •

edited

Loading

kbendick commented Jun 5, 2022 •

edited

Loading