Reconsider the breaking changes check policy to detect breaking chang…

…es against released versions (#13292) * Reconsider the breaking changes check policy to detect breaking changes against released versions Signed-off-by: Andriy Redko <[email protected]> * Add an ability to provide the target version to compare with for japicmp tasks Signed-off-by: Andriy Redko <[email protected]> * Add documentation for japicmp tasks Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]>
opensearch-project · Apr 19, 2024 · b4692c8 · b4692c8
1 parent 0282e64
commit b4692c8
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Improve built-in secure transports support ([#12907](https://github.com/opensearch-project/OpenSearch/pull/12907))
 - Update links to documentation in rest-api-spec ([#13043](https://github.com/opensearch-project/OpenSearch/pull/13043))
 - Refactoring globMatch using simpleMatchWithNormalizedStrings from Regex ([#13104](https://github.com/opensearch-project/OpenSearch/pull/13104))
+- [BWC and API enforcement] Reconsider the breaking changes check policy to detect breaking changes against released versions ([#13292](https://github.com/opensearch-project/OpenSearch/pull/13292))
 
 ### Deprecated
 

diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md
@@ -57,6 +57,7 @@
       - [Developer API](#developer-api)
       - [User API](#user-api)
       - [Experimental Development](#experimental-development)
+      - [API Compatibility Checks](#api-compatibility-checks)
     - [Backports](#backports)
     - [LineLint](#linelint)
     - [Lucene Snapshots](#lucene-snapshots)
@@ -607,6 +608,20 @@ a LTS feature but with additional guard rails and communication mechanisms to si
 release, or be removed altogether. Any Developer or User APIs implemented along with the experimental feature should be marked with `@ExperimentalApi` (or documented as
 `@opensearch.experimental`) annotation to signal the implementation is not subject to LTS and does not follow backwards compatibility guidelines.
 
+#### API Compatibility Checks
+
+The compatibility checks for public APIs are performed using [japicmp](https://siom79.github.io/japicmp/) and are available as separate Gradle tasks (those are run on demand at the moment):
+
+```
+./gradlew japicmp
+```
+
+By default, the API compatibility checks are run against the latest released version of the OpenSearch, however the target version to compare to could be provided using system property during the build, fe.:
+
+```
+./gradlew japicmp  -Djapicmp.compare.version=2.14.0-SNAPSHOT
+```
+
 ### Backports
 
 The Github workflow in [`backport.yml`](.github/workflows/backport.yml) creates backport PRs automatically when the original PR with an appropriate label `backport <backport-branch-name>` is merged to main with the backport workflow run successfully on the PR. For example, if a PR on main needs to be backported to `1.x` branch, add a label `backport 1.x` to the PR and make sure the backport workflow runs on the PR along with other checks. Once this PR is merged to main, the workflow will create a backport PR to the `1.x` branch.

diff --git a/server/build.gradle b/server/build.gradle
@@ -173,6 +173,22 @@ tasks.named("testingConventions").configure {
     }
 }
 
+// Set to current version by default
+def japicmpCompareTarget = System.getProperty("japicmp.compare.version")
+if (japicmpCompareTarget == null) { /* use latest released version */
+    // Read the list from maven central.
+    // Fetch the metadata and parse the xml into Version instances, pick the latest one
+    japicmpCompareTarget = new URL('https://repo1.maven.org/maven2/org/opensearch/opensearch/maven-metadata.xml').openStream().withStream { s ->
+        new XmlParser().parse(s)
+           .versioning.versions.version
+           .collect { it.text() }.findAll { it ==~ /\d+\.\d+\.\d+/ }
+           .collect { org.opensearch.gradle.Version.fromString(it) }
+           .toSorted()
+           .last()
+           .toString()
+        }
+}
+
 def generateModulesList = tasks.register("generateModulesList") {
     List<String> modules = project(':modules').subprojects.collect { it.name }
     File modulesFile = new File(buildDir, 'generated-resources/modules.txt')
@@ -380,60 +396,59 @@ tasks.named("sourcesJar").configure {
   }
 }
 
-/** Compares the current build against a snapshot build */
+/** Compares the current build against a laltest released version or the version supplied through 'japicmp.compare.version' system property */
 tasks.register("japicmp", me.champeau.gradle.japicmp.JapicmpTask) {
-    oldClasspath.from(files("${buildDir}/snapshot/opensearch-${version}.jar"))
+    logger.info("Comparing public APIs from ${version} to ${japicmpCompareTarget}")
+    oldClasspath.from(files("${buildDir}/japicmp-target/opensearch-${japicmpCompareTarget}.jar"))
     newClasspath.from(tasks.named('jar'))
     onlyModified = true
     failOnModification = true
     ignoreMissingClasses = true
     annotationIncludes = ['@org.opensearch.common.annotation.PublicApi', '@org.opensearch.common.annotation.DeprecatedApi']
     txtOutputFile = layout.buildDirectory.file("reports/java-compatibility/report.txt")
     htmlOutputFile = layout.buildDirectory.file("reports/java-compatibility/report.html")
-    dependsOn downloadSnapshot
+    dependsOn downloadJapicmpCompareTarget
 }
 
 /** If the Java API Comparison task failed, print a hint if the change should be merged from its target branch */
 gradle.taskGraph.afterTask { Task task, TaskState state ->
     if (task.name == 'japicmp' && state.failure != null) {
-        def sha = getGitShaFromJar("${buildDir}/snapshot/opensearch-${version}.jar")
-        logger.info("Incompatiable java api from snapshot jar built off of commit ${sha}")
-
-        if (!inHistory(sha)) {
-          logger.warn('\u001B[33mPlease merge from the target branch and run this task again.\u001B[0m')
-        }
+        logger.info("Public APIs changes incompatiable with ${japicmpCompareTarget} target have been detected")
     }
 }
 
-/** Downloads latest snapshot from maven repository */
-tasks.register("downloadSnapshot", Copy) {
+/** Downloads latest released version from maven repository */
+tasks.register("downloadJapicmpCompareTarget", Copy) {
     def mavenSnapshotRepoUrl = "https://aws.oss.sonatype.org/content/repositories/snapshots/"
     def groupId = "org.opensearch"
     def artifactId = "opensearch"
 
-    def repos = project.getRepositories();
-    MavenArtifactRepository opensearchRepo = repos.maven(repo -> {
-        repo.setName("opensearch-snapshots");
-        repo.setUrl(mavenSnapshotRepoUrl);
-    });
-
-    repos.exclusiveContent(exclusiveRepo -> {
-        exclusiveRepo.filter(descriptor -> descriptor.includeGroup(groupId));
-        exclusiveRepo.forRepositories(opensearchRepo);
-    });
+    // Add repository for snapshot artifacts if japicmp compare target version is snapshot
+    if (japicmpCompareTarget.endsWith("-SNAPSHOT")) {
+        def repos = project.getRepositories();
+        MavenArtifactRepository opensearchRepo = repos.maven(repo -> {
+            repo.setName("opensearch-snapshots");
+            repo.setUrl(mavenSnapshotRepoUrl);
+        });
+
+        repos.exclusiveContent(exclusiveRepo -> {
+            exclusiveRepo.filter(descriptor -> descriptor.includeGroup(groupId));
+            exclusiveRepo.forRepositories(opensearchRepo);
+        });
+    }
 
     configurations {
-        snapshotArtifact {
+        japicmpCompareTargetArtifact {
             exclude group: 'org.apache.lucene'
         }
     }
 
     dependencies {
-        snapshotArtifact("${groupId}:${artifactId}:${version}:")
+        japicmpCompareTargetArtifact("${groupId}:${artifactId}:${japicmpCompareTarget}:")
     }
 
-    from configurations.snapshotArtifact
-    into "$buildDir/snapshot"
+    from configurations.japicmpCompareTargetArtifact
+    into "$buildDir/japicmp-target"
 }
 
 /** Check if the sha is in the current history */