From 15a434d29f3f9540fe1f1c1b95f768f05508af56 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Fri, 19 Apr 2024 15:29:18 -0400 Subject: [PATCH] Reconsider the breaking changes check policy to detect breaking changes against released versions (#13292) (#13310) (cherry picked from commit b4692c897ec4b19161417147d29b116671616fe1) Signed-off-by: Andriy Redko --- .github/workflows/detect-breaking-change.yml | 3 +- CHANGELOG.md | 1 + DEVELOPER_GUIDE.md | 15 +++++ server/build.gradle | 60 +++++++++++++------- 4 files changed, 59 insertions(+), 20 deletions(-) diff --git a/.github/workflows/detect-breaking-change.yml b/.github/workflows/detect-breaking-change.yml index 1913d070e8c24..f39eabd67cc9d 100644 --- a/.github/workflows/detect-breaking-change.yml +++ b/.github/workflows/detect-breaking-change.yml @@ -14,7 +14,8 @@ jobs: - uses: gradle/gradle-build-action@v3 with: cache-disabled: true - arguments: japicmp + # The 2.14.0 is already reporting breaking changes (see please https://github.com/opensearch-project/OpenSearch/issues/13308) + arguments: japicmp -Djapicmp.compare.version=2.14.0-SNAPSHOT gradle-version: 8.7 build-root-directory: server - if: failure() diff --git a/CHANGELOG.md b/CHANGELOG.md index 38692f3de5162..b5a76433096ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,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 index 3393740a49ac3..45b07db685949 100644 --- 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 ` 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 index 4481284dcd0e6..76abf6d575a7d 100644 --- 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 modules = project(':modules').subprojects.collect { it.name } File modulesFile = new File(buildDir, 'generated-resources/modules.txt') @@ -416,9 +432,10 @@ 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 @@ -426,43 +443,48 @@ tasks.register("japicmp", me.champeau.gradle.japicmp.JapicmpTask) { 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" - repositories { - maven { - url mavenSnapshotRepoUrl - } + // 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 */