Skip to content

Commit

Permalink
GEODE-8087: Fix Java binary compatibility errors reported by japicmp
Browse files Browse the repository at this point in the history
* Fix 'exludeAnnotations' to use FQN of the annotation.
* Allow changes if a class or superclass is marked @experimental
  • Loading branch information
robbadler committed May 9, 2020
1 parent 8f0650e commit 18b1036
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 82 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
}


Expand Down
4 changes: 4 additions & 0 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ plugins {

repositories {
mavenCentral()
maven {
url "https://plugins.gradle.org/m2/"
}
}

dependencies {
Expand All @@ -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')

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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 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

class AllowMajorBreakingChanges extends AbstractRecordingSeenMembers {
@Override
Violation maybeAddViolation(final JApiCompatibility member) {
if (!member.isBinaryCompatible()) {
return Violation.notBinaryCompatible(member, Severity.warning)
} else {
return null
}
}
}

Original file line number Diff line number Diff line change
@@ -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
}
}
82 changes: 1 addition & 81 deletions geode-assembly/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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')
88 changes: 88 additions & 0 deletions gradle/japicmp.gradle
Original file line number Diff line number Diff line change
@@ -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
}
}

0 comments on commit 18b1036

Please sign in to comment.