You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by rh...@apache.org on 2020/05/09 03:05:21 UTC
[geode] 01/02: GEODE-8087: Fix Java binary compatibility errors
reported by japicmp
This is an automated email from the ASF dual-hosted git repository.
rhoughton pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 18b1036ac9f2dcaaa90e570e0c53b3186af3b72e
Author: Robert Houghton <rh...@pivotal.io>
AuthorDate: Thu May 7 15:06:30 2020 -0700
GEODE-8087: Fix Java binary compatibility errors reported by japicmp
* Fix 'exludeAnnotations' to use FQN of the annotation.
* Allow changes if a class or superclass is marked @Experimental
---
build.gradle | 2 +-
buildSrc/build.gradle | 4 +
.../japicmp/AllowMajorBreakingChanges.groovy} | 41 ++++------
.../gradle/japicmp/ParentIsExperimental.groovy | 63 ++++++++++++++++
geode-assembly/build.gradle | 82 +-------------------
gradle/japicmp.gradle | 88 ++++++++++++++++++++++
6 files changed, 170 insertions(+), 110 deletions(-)
diff --git a/build.gradle b/build.gradle
index 928fd4a..0f8f163 100755
--- a/build.gradle
+++ b/build.gradle
@@ -29,7 +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"
+ id "me.champeau.gradle.japicmp" apply false // Version defined in buildSrc/build.gradle
}
diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle
index 4354c3b..3de39ee 100644
--- a/buildSrc/build.gradle
+++ b/buildSrc/build.gradle
@@ -22,6 +22,9 @@ plugins {
repositories {
mavenCentral()
+ maven {
+ url "https://plugins.gradle.org/m2/"
+ }
}
dependencies {
@@ -32,6 +35,7 @@ dependencies {
implementation('org.apache.commons:commons-lang3:3.3.2')
implementation('org.apache.maven:maven-artifact:3.3.3')
implementation('com.github.docker-java:docker-java:3.0.14')
+ implementation('me.champeau.gradle:japicmp-gradle-plugin:0.2.8')
implementation('junit:junit:4.12')
diff --git a/buildSrc/build.gradle b/buildSrc/src/main/groovy/org/apache/geode/gradle/japicmp/AllowMajorBreakingChanges.groovy
similarity index 58%
copy from buildSrc/build.gradle
copy to buildSrc/src/main/groovy/org/apache/geode/gradle/japicmp/AllowMajorBreakingChanges.groovy
index 4354c3b..bf79658 100644
--- a/buildSrc/build.gradle
+++ b/buildSrc/src/main/groovy/org/apache/geode/gradle/japicmp/AllowMajorBreakingChanges.groovy
@@ -13,38 +13,23 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
- *
*/
-plugins {
- id 'java'
-}
-
-repositories {
- mavenCentral()
-}
-
-dependencies {
- testRuntime('org.apache.geode:geode-junit:1.3.0') {
- exclude group: 'org.apache.logging.log4j'
- }
- implementation(gradleApi())
- implementation('org.apache.commons:commons-lang3:3.3.2')
- implementation('org.apache.maven:maven-artifact:3.3.3')
- implementation('com.github.docker-java:docker-java:3.0.14')
-
- implementation('junit:junit:4.12')
+package org.apache.geode.gradle.japicmp
- testAnnotationProcessor(this.project)
-}
+import japicmp.model.JApiCompatibility
+import me.champeau.gradle.japicmp.report.Violation
+import me.champeau.gradle.japicmp.report.stdrules.AbstractRecordingSeenMembers
+import me.champeau.gradle.japicmp.report.Severity
-sourceSets {
- main {
- java {
- srcDirs = []
- }
- groovy {
- srcDirs += ['src/main/java']
+class AllowMajorBreakingChanges extends AbstractRecordingSeenMembers {
+ @Override
+ Violation maybeAddViolation(final JApiCompatibility member) {
+ if (!member.isBinaryCompatible()) {
+ return Violation.notBinaryCompatible(member, Severity.warning)
+ } else {
+ return null
}
}
}
+
diff --git a/buildSrc/src/main/groovy/org/apache/geode/gradle/japicmp/ParentIsExperimental.groovy b/buildSrc/src/main/groovy/org/apache/geode/gradle/japicmp/ParentIsExperimental.groovy
new file mode 100644
index 0000000..1f8e0bc
--- /dev/null
+++ b/buildSrc/src/main/groovy/org/apache/geode/gradle/japicmp/ParentIsExperimental.groovy
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.geode.gradle.japicmp;
+
+import me.champeau.gradle.japicmp.report.Violation
+import me.champeau.gradle.japicmp.report.stdrules.AbstractRecordingSeenMembers
+import japicmp.model.JApiMethod
+import japicmp.model.JApiClass
+import japicmp.model.JApiSuperclass
+import japicmp.model.JApiConstructor
+import japicmp.model.JApiCompatibility
+
+
+class ParentIsExperimental extends AbstractRecordingSeenMembers {
+ @Override
+ public Violation maybeAddViolation(final JApiCompatibility member) {
+ boolean isExperimental = true
+ if (!member.isBinaryCompatible()) {
+ if (member instanceof JApiMethod || member instanceof JApiConstructor) {
+ isExperimental = isClassExperimental(member.jApiClass)
+ } else if (member instanceof JApiClass) {
+ isExperimental = isClassExperimental(member)
+ }
+
+ if (isExperimental) {
+ println("return accept [${member}]")
+ return Violation.accept(member, "Parent class is @Experimental")
+ }
+ }
+ }
+
+ boolean isClassExperimental(final JApiClass member) {
+ println(" member annotations: [${member.annotations}]")
+ boolean isExperimental = false
+ if (!member.annotations.find {
+ it.fullyQualifiedName == 'org.apache.geode.annotations.Experimental'
+ }) {
+ JApiSuperclass sup = member.superclass
+ if (sup.getJApiClass().present) {
+ isExperimental = isClassExperimental(sup.getJApiClass().get())
+ } else {
+ }
+ } else {
+ isExperimental = true
+ }
+ return isExperimental
+ }
+}
diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index 7230327..9f2e7f3 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -15,19 +15,12 @@
* 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.
@@ -703,77 +696,4 @@ 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
- }
- }
-}
+apply from: Paths.get("${rootDir}", 'gradle', 'japicmp.gradle')
diff --git a/gradle/japicmp.gradle b/gradle/japicmp.gradle
new file mode 100644
index 0000000..9c94a8a
--- /dev/null
+++ b/gradle/japicmp.gradle
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import japicmp.model.JApiChangeStatus
+import org.apache.maven.artifact.versioning.ComparableVersion
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion
+
+import org.apache.geode.gradle.japicmp.AllowMajorBreakingChanges
+import org.apache.geode.gradle.japicmp.ParentIsExperimental
+
+
+def mostRecentReleaseProj = project(':geode-old-versions').subprojects.max(){ v -> new ComparableVersion(v.name)}
+def newest = mostRecentReleaseProj.name
+
+tasks.register('japicmp', me.champeau.gradle.japicmp.JapicmpTask) {
+ inputs.files { configurations.geodeArchives }
+ def ourUnpackTaskProvider = project(":geode-old-versions:${newest}").tasks.named('downloadAndUnzipFile')
+ inputs.files { ourUnpackTaskProvider }
+
+ def d = java.nio.file.Paths.get(project(":geode-old-versions:${newest}").buildDir.path.toString(), "apache-geode-${newest}", 'lib')
+ // Dirty hack, to set the value as requied in configuration,
+ // but then reset to the acutal unpacked jars on runtime.
+ 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("japicmp will always pass when comparing across major versions.")
+ 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 = ['@org.apache.geode.annotations.Experimental']
+
+ richReport {
+ title = "Geode API Compatibility Report"
+ description = "Comparing current ${version} against downloaded v${newest}."
+ reportName = "rich-report-${reportFileName}.html"
+
+ if (allowMajorBreaking) {
+ addRule(AllowMajorBreakingChanges)
+ } else {
+ addRule(JApiChangeStatus.REMOVED, ParentIsExperimental)
+ addRule(JApiChangeStatus.MODIFIED, ParentIsExperimental)
+ }
+ addDefaultRules = true
+ }
+}