[GEODE-8083] Add API Checking to Geode. (apache#5066)

* Add API check gradle task. * Add CI job that calls API check gradle task. * Fix spotless rules so it doesn't mistakenly mess up our gradle. Co-authored-by: Sean Goller <[email protected]> Co-authored-by: Robert Houghton <[email protected]> Co-authored-by: Robert Houghton <[email protected]>
kekerzzZ · May 7, 2020 · 3963431 · 3963431
1 parent 0db6066
commit 3963431
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 16 deletions.
diff --git a/build.gradle b/build.gradle
@@ -29,6 +29,7 @@ plugins {
   id "org.ajoberstar.grgit" version "4.0.1" apply false
   id "org.nosphere.apache.rat" version "0.6.0" apply false
   id "org.sonarqube" version "2.8" apply false
+  id "me.champeau.gradle.japicmp" version "0.2.8"
 }
 
 

diff --git a/ci/pipelines/shared/jinja.variables.yml b/ci/pipelines/shared/jinja.variables.yml
@@ -202,3 +202,15 @@ tests:
   PLATFORM: windows
   RAM: '64'
   name: WindowsUnit
+- ARTIFACT_SLUG: apicheck
+  CALL_STACK_TIMEOUT: '20700'
+  CPUS: '4'
+  DUNIT_PARALLEL_FORKS: '0'
+  EXECUTE_TEST_TIMEOUT: 1h
+  GRADLE_TASK: geode-assembly:japicmp
+  MAX_IN_FLIGHT: 1
+  PARALLEL_DUNIT: 'false'
+  PARALLEL_GRADLE: 'false'
+  PLATFORM: linux
+  RAM: '16'
+  name: ApiCheck
diff --git a/ci/scripts/archive_results.sh b/ci/scripts/archive_results.sh
@@ -123,6 +123,11 @@ pushd ${GEODE_BUILD}/build/reports/combined
   gsutil -q -m cp -r * gs://${TEST_RESULTS_DESTINATION}
 popd
 
+API_CHECK_REPORT=$(ls ${GEODE_BUILD}/geode-assembly/build/reports/rich-report-japi*.html)
+if [ -n "${API_CHECK_REPORT}" ]; then
+  gsutil -q cp ${API_CHECK_REPORT} gs://${TEST_RESULTS_DESTINATION}api_check_report.html
+fi
+
 gsutil cp ${DEST_DIR}/${FILENAME} gs://${TEST_ARTIFACTS_DESTINATION}
 
 set +x
@@ -134,6 +139,12 @@ printf "\033[92m${ARTIFACT_SCHEME}://${TEST_RESULTS_DESTINATION}\033[0m\n"
 printf "\033[92m=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=\033[0m\n"
 printf "\n"
 
+if [ -n "${API_CHECK_REPORT}" ]; then
+  printf "\033[92m=-=-=-=-=-=-=-=-=-=-=-=-=-=  API Check Results URI -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=\033[0m\n"
+  printf "\033[92m${ARTIFACT_SCHEME}://${TEST_RESULTS_DESTINATION}api_check_report.html\033[0m\n"
+  printf "\033[92m=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=\033[0m\n"
+fi
+
 printf "\033[92mTest report artifacts from this job are available at:\033[0m\n"
 printf "\n"
 printf "\033[92m${ARTIFACT_SCHEME}://${TEST_ARTIFACTS_DESTINATION}${FILENAME}\033[0m\n"

diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
@@ -15,13 +15,19 @@
  * limitations under the License.
  */
 
+import me.champeau.gradle.japicmp.JapicmpTask
+import me.champeau.gradle.japicmp.report.Severity
+
 apply from: "${rootDir}/${scriptDir}/standard-subproject-configuration.gradle"
 apply from: "${rootDir}/${scriptDir}/warnings.gradle"
 
 
 import org.apache.geode.gradle.plugins.DependencyConstraints
+import org.apache.maven.artifact.versioning.ComparableVersion
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion
 
 import java.nio.file.Paths
+import org.gradle.api.file.FileCollection
 
 // This project aggressively reaches into many other projects and must wait for those configurations
 // to be evaluated and resolved.  Evaluation depends on each of these subprojects.
@@ -170,9 +176,9 @@ dependencies {
     geodeArchives(project(it))
   }
   geodeArchives project(':geode-management')
-  geodeArchives project(':geode-web')
-  geodeArchives project(':geode-web-api')
-  geodeArchives project(':geode-web-management')
+  geodeArchives project(path: ':geode-web', configuration: 'war')
+  geodeArchives project(path: ':geode-web-api', configuration: 'war')
+  geodeArchives project(path: ':geode-web-management', configuration: 'war')
 
   javadocOnly project(':extensions:geode-modules')
   javadocOnly project(':extensions:geode-modules-session')
@@ -552,11 +558,13 @@ distributions {
 
       with copySpec {
         into('config')
-        from defaultCacheConfig
-        from defaultDistributionConfig
-        from(project(':geode-log4j').sourceSets.main.resources.files.find {
-          it.name == 'log4j2.xml'
-        })
+        from {defaultCacheConfig}
+        from {defaultDistributionConfig}
+        from {
+          (project(':geode-log4j').sourceSets.main.resources.files.find {
+            it.name == 'log4j2.xml'
+          })
+        }
       }
 
       with copySpec {
@@ -573,8 +581,8 @@ distributions {
 
         //These tasks are included as closures (wrapped in {}) because gradle may evaluate
         //this CopySpec before it evaluates the geode-core build file.
-        from project(':geode-core').tasks.named('raJar')
-        from project(':geode-core').tasks.named('jcaJar')
+        from {project(':geode-core').tasks.named('raJar')}
+        from {project(':geode-core').tasks.named('jcaJar')}
 
         // dependency jars
         from tasks.named('depsJar')
@@ -584,16 +592,16 @@ distributions {
       with copySpec {
         into('tools/Extensions')
 
-        from project(':geode-web').configurations.archives.allArtifacts.files
-        from project(':geode-web-api').configurations.archives.allArtifacts.files
-        from project(':geode-web-management').configurations.archives.allArtifacts.files
+        from {project(':geode-web').configurations.archives.allArtifacts.files}
+        from {project(':geode-web-api').configurations.archives.allArtifacts.files}
+        from {project(':geode-web-management').configurations.archives.allArtifacts.files}
 
         exclude '*.jar'
       }
 
       with copySpec {
         into('tools/ClientProtocol')
-        from project(':geode-protobuf-messages').zip.outputs.files
+        from {project(':geode-protobuf-messages').zip.outputs.files}
       }
 
       with copySpec {
@@ -603,7 +611,7 @@ distributions {
 
       with copySpec {
         into('tools/Pulse')
-        from project(':geode-pulse').configurations.archives.allArtifacts.files
+        from {project(':geode-pulse').configurations.archives.allArtifacts.files}
       }
 
       with copySpec {
@@ -694,3 +702,78 @@ docker {
 acceptanceTest {
   dependsOn(tasks.docker)
 }
+
+// For the list of 'old-versions' find the newest using semver
+// If the newest old release is a differnt major # than us, we can do what we want
+// otherwise, fail on x,y,z
+
+def mostRecentReleaseProj = project(':geode-old-versions').subprojects.max(){ v -> new ComparableVersion(v.name)}
+def newest = mostRecentReleaseProj.name
+
+import me.champeau.gradle.japicmp.report.ViolationRule
+import me.champeau.gradle.japicmp.report.Violation
+import japicmp.model.JApiCompatibility
+import japicmp.model.JApiMethod
+
+class AllowMajorBreakingChanges implements ViolationRule {
+  @Override
+  Violation maybeViolation(final JApiCompatibility member) {
+    if (!member.binaryCompatible()) {
+      Violation.notBinaryCompatible(member, Severity.warning)
+    }
+  }
+}
+tasks.register('japicmp', me.champeau.gradle.japicmp.JapicmpTask) {
+  inputs.files { tasks.named('distTar') }
+  def ourUnpackTaskProvider = project(":geode-old-versions:${newest}").tasks.named('downloadAndUnzipFile')
+  inputs.files { ourUnpackTaskProvider }
+
+  def d = Paths.get(project(":geode-old-versions:${newest}").buildDir.path.toString(), "apache-geode-${newest}", 'lib')
+  oldClasspath = files()
+  oldArchives = files()
+  doFirst {
+    oldClasspath = files(file(d).listFiles())
+    oldArchives = files(file(d).listFiles(new FilenameFilter() {
+      public boolean accept(File dir, String name) {
+        return name.toLowerCase().startsWith("geode-") && name.toLowerCase().endsWith(".jar");
+      };
+    }))
+  }
+
+  newClasspath = configurations.geodeArchives
+  newArchives = configurations.geodeArchives.filter { File f ->
+    f.toString().contains("geode-") && f.toString().endsWith(".jar")
+  }
+
+  ignoreMissingClasses = true
+  onlyModified = true
+  accessModifier = "protected"
+
+  def allowMajorBreaking = false
+  if (new DefaultArtifactVersion(version).majorVersion > new DefaultArtifactVersion(newest).majorVersion) {
+    logger.error("disable fail on binary breaking changes")
+    allowMajorBreaking = true
+  }
+  failOnModification = allowMajorBreaking
+  failOnSourceIncompatibility = allowMajorBreaking
+
+//  onlyBinaryIncompatibleModified = true
+//  includeSynthetic = true
+
+  def reportFileName = "japi-v${newest}-${version}"
+  txtOutputFile = file("$buildDir/reports/${reportFileName}.txt")
+  htmlOutputFile = file("$buildDir/reports/${reportFileName}.html")
+  packageExcludes = ["*internal*"]
+  packageIncludes = ["org.apache.geode.*"]
+  annotationExcludes = ["@Experimental"]
+
+ richReport {
+   title = "Geode API Compatibility Report"
+   description = "Comparing current ${version} against downloaded v${newest}."
+   reportName = "rich-report-${reportFileName}.html"
+   if (allowMajorBreaking) {
+     addRule(AllowMajorBreakingChanges)
+//     addDefaultRules = failBuild
+   }
+ }
+}
diff --git a/geode-web-api/build.gradle b/geode-web-api/build.gradle
@@ -24,6 +24,10 @@ apply from: "${project.projectDir}/../gradle/warnings.gradle"
 
 jar.enabled = false
 
+configurations {
+  war {}
+}
+
 dependencies {
   compile(platform(project(':boms:geode-all-bom'))){
     exclude module: "jackson-annotations"
@@ -121,3 +125,6 @@ war {
   classpath configurations.runtimeClasspath
 }
 
+artifacts {
+  war tasks.war
+}
diff --git a/geode-web-management/build.gradle b/geode-web-management/build.gradle
@@ -44,6 +44,9 @@ sourceSets {
   }
 }
 
+configurations {
+  war {}
+}
 dependencies {
   compile(platform(project(':boms:geode-all-bom'))) {
     exclude module: "jackson-annotations"
@@ -164,3 +167,7 @@ war {
   // from the runtime classpath
   classpath configurations.runtimeClasspath
 }
+
+artifacts {
+  war tasks.war
+}
diff --git a/geode-web/build.gradle b/geode-web/build.gradle
@@ -24,6 +24,9 @@ apply from: "${project.projectDir}/../gradle/warnings.gradle"
 
 jar.enabled = false
 
+configurations {
+  war {}
+}
 dependencies {
   api(platform(project(':boms:geode-all-bom')))
 
@@ -75,3 +78,7 @@ war {
   enabled = true
   duplicatesStrategy = DuplicatesStrategy.EXCLUDE
 }
+
+artifacts {
+  war tasks.war
+}
diff --git a/gradle/spotless.gradle b/gradle/spotless.gradle
@@ -132,7 +132,7 @@ spotless {
     bumpThisNumberIfACustomStepChanges(project.ext.'spotless-file-hash')
 
     custom 'Use single-quote in project directives.', {
-      it.replaceAll(/project\(":([^"]*)"\)/, 'project(\':$1\')')
+      it.replaceAll(/project\(":([^"]*)"\)$/, 'project(\':$1\')')
     }
 
     custom 'Use parenthesis in single-line gradle dependency declarations.', {