Merge "Further improvements to the signature update check"
diff --git a/src/main/java/com/android/tools/metalava/Diff.kt b/src/main/java/com/android/tools/metalava/Diff.kt
index d0120ea..cd7f2e4 100644
--- a/src/main/java/com/android/tools/metalava/Diff.kt
+++ b/src/main/java/com/android/tools/metalava/Diff.kt
@@ -16,10 +16,44 @@
package com.android.tools.metalava
-// Copied from lint's test suite: TestUtils.diff in tools/base
+import com.android.ide.common.process.CachedProcessOutputHandler
+import com.android.ide.common.process.DefaultProcessExecutor
+import com.android.ide.common.process.ProcessInfoBuilder
+import com.android.utils.LineCollector
+import com.android.utils.StdLogger
+import java.io.File
-fun getDiff(before: String, after: String): String {
- return getDiff(before, after, 0)
+// Copied from lint's test suite: TestUtils.diff in tools/base with
+// some changes to allow running native diff.
+
+/** Returns the universal diff for the two given files, or null if not supported or working */
+fun getNativeDiff(before: File, after: File): String? {
+ if (options.noNativeDiff) {
+ return null
+ }
+
+ // A lot faster for big files:
+ val native = File("/usr/bin/diff")
+ if (native.isFile) {
+ try {
+ val builder = ProcessInfoBuilder()
+ builder.setExecutable(native)
+ builder.addArgs("-u", before.path, after.path)
+ val processOutputHandler = CachedProcessOutputHandler()
+ DefaultProcessExecutor(StdLogger(StdLogger.Level.ERROR))
+ .execute(builder.createProcess(), processOutputHandler)
+ .rethrowFailure()
+
+ val output = processOutputHandler.processOutput
+ val lineCollector = LineCollector()
+ output.processStandardOutputLines(lineCollector)
+
+ return lineCollector.result.joinToString(separator = "\n")
+ } catch (ignore: Throwable) {
+ }
+ }
+
+ return null
}
fun getDiff(before: String, after: String, windowSize: Int): String {
@@ -30,15 +64,10 @@
)
}
-fun getDiff(before: Array<String>, after: Array<String>): String {
- return getDiff(before, after, 0)
-}
-
fun getDiff(
before: Array<String>,
after: Array<String>,
- windowSize: Int,
- max: Int = 7
+ windowSize: Int
): String {
// Based on the LCS section in http://introcs.cs.princeton.edu/java/96optimization/
val sb = StringBuilder()
diff --git a/src/main/java/com/android/tools/metalava/DocAnalyzer.kt b/src/main/java/com/android/tools/metalava/DocAnalyzer.kt
index 38b5ead..44eed3b 100644
--- a/src/main/java/com/android/tools/metalava/DocAnalyzer.kt
+++ b/src/main/java/com/android/tools/metalava/DocAnalyzer.kt
@@ -624,7 +624,7 @@
}
override fun getCacheDir(name: String?, create: Boolean): File? {
- if (create && java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING)) {
+ if (create && isUnderTest()) {
// Pick unique directory during unit tests
return Files.createTempDir()
}
diff --git a/src/main/java/com/android/tools/metalava/Driver.kt b/src/main/java/com/android/tools/metalava/Driver.kt
index f9cdaab..400c139 100644
--- a/src/main/java/com/android/tools/metalava/Driver.kt
+++ b/src/main/java/com/android/tools/metalava/Driver.kt
@@ -20,7 +20,6 @@
import com.android.SdkConstants
import com.android.SdkConstants.DOT_JAVA
import com.android.SdkConstants.DOT_KT
-import com.android.SdkConstants.DOT_TXT
import com.android.ide.common.process.CachedProcessOutputHandler
import com.android.ide.common.process.DefaultProcessExecutor
import com.android.ide.common.process.ProcessInfoBuilder
@@ -92,8 +91,7 @@
setExitCode: Boolean = false
): Boolean {
- if (System.getenv(ENV_VAR_METALAVA_DUMP_ARGV) != null &&
- !java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING)
+ if (System.getenv(ENV_VAR_METALAVA_DUMP_ARGV) != null && !isUnderTest()
) {
stdout.println("---Running $PROGRAM_NAME----")
stdout.println("pwd=${File("").absolutePath}")
@@ -131,6 +129,7 @@
stdout.println("\"$arg\",")
}
stdout.println("----------------------------")
+ stdout.flush()
}
newArgs
}
@@ -145,6 +144,8 @@
}
exitValue = true
} catch (e: DriverException) {
+ stdout.flush()
+ stderr.flush()
if (e.stderr.isNotBlank()) {
stderr.println("\n${e.stderr}")
}
@@ -166,7 +167,7 @@
stdout.flush()
stderr.flush()
- if (setExitCode && reporter.hasErrors()) {
+ if (setExitCode) {
exit(exitCode)
}
@@ -621,14 +622,18 @@
val currentTxt = getCanonicalSignatures(signatureFile)
val newTxt = getCanonicalSignatures(apiFile)
if (newTxt != currentTxt) {
- val diff = getDiff(currentTxt, newTxt, 1)
+ val diff = getNativeDiff(signatureFile, apiFile) ?: getDiff(currentTxt, newTxt, 1)
+ val updateApi = if (isBuildingAndroid())
+ "Run make update-api to update.\n"
+ else
+ ""
val message =
"""
- Aborting: Your changes have resulted in differences in
- the signature file for the ${apiType.displayName} API.
- The changes are compatible, but the signature file needs
- to be updated.
+ Aborting: Your changes have resulted in differences in the signature file
+ for the ${apiType.displayName} API.
+ The changes may be compatible, but the signature file needs to be updated.
+ $updateApi
Diffs:
""".trimIndent() + "\n" + diff
@@ -897,7 +902,7 @@
if (!assertionsEnabled() &&
System.getenv(ENV_VAR_METALAVA_DUMP_ARGV) == null &&
- !java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING)
+ !isUnderTest()
) {
DefaultLogger.disableStderrDumping(parentDisposable)
}
@@ -1229,3 +1234,9 @@
fun findPackage(source: String): String? {
return ClassName(source).packageName
}
+
+/** Whether metalava is running unit tests */
+fun isUnderTest() = java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING)
+
+/** Whether metalava is being invoked as part of an Android platform build */
+fun isBuildingAndroid() = System.getenv("ANDROID_BUILD_TOP") != null && !isUnderTest()
diff --git a/src/main/java/com/android/tools/metalava/Options.kt b/src/main/java/com/android/tools/metalava/Options.kt
index 2f32b26..33a26da 100644
--- a/src/main/java/com/android/tools/metalava/Options.kt
+++ b/src/main/java/com/android/tools/metalava/Options.kt
@@ -87,6 +87,7 @@
const val ARG_CHECK_COMPATIBILITY_REMOVED_CURRENT = "--check-compatibility:removed:current"
const val ARG_CHECK_COMPATIBILITY_REMOVED_RELEASED = "--check-compatibility:removed:released"
const val ARG_ALLOW_COMPATIBLE_DIFFERENCES = "--allow-compatible-differences"
+const val ARG_NO_NATIVE_DIFF = "--no-native-diff"
const val ARG_INPUT_KOTLIN_NULLS = "--input-kotlin-nulls"
const val ARG_OUTPUT_KOTLIN_NULLS = "--output-kotlin-nulls"
const val ARG_OUTPUT_DEFAULT_VALUES = "--output-default-values"
@@ -431,6 +432,9 @@
*/
var allowCompatibleDifferences = false
+ /** If false, attempt to use the native diff utility on the system */
+ var noNativeDiff = false
+
/** Existing external annotation files to merge in */
var mergeQualifierAnnotations: List<File> = mutableMergeQualifierAnnotations
var mergeInclusionAnnotations: List<File> = mutableMergeInclusionAnnotations
@@ -563,6 +567,7 @@
var updateBaselineFile: File? = null
var baselineFile: File? = null
var mergeBaseline = false
+ var delayedCheckApiFiles = false
var index = 0
while (index < args.size) {
@@ -838,29 +843,37 @@
}
ARG_ALLOW_COMPATIBLE_DIFFERENCES -> allowCompatibleDifferences = true
+ ARG_NO_NATIVE_DIFF -> noNativeDiff = true
// Compat flag for the old API check command, invoked from build/make/core/definitions.mk:
"--check-api-files" -> {
- val stableApiFile = stringToExistingFile(getValue(args, ++index))
- val apiFileToBeTested = stringToExistingFile(getValue(args, ++index))
- val stableRemovedApiFile = stringToExistingFile(getValue(args, ++index))
- val removedApiFileToBeTested = stringToExistingFile(getValue(args, ++index))
- mutableCompatibilityChecks.add(
- CheckRequest(
- stableApiFile,
- ApiType.PUBLIC_API,
- ReleaseType.RELEASED,
- apiFileToBeTested
+ if (index < args.size - 1 && args[index + 1].startsWith("-")) {
+ // Work around bug where --check-api-files is invoked with all
+ // the other metalava args before the 4 files; this will be
+ // fixed by https://android-review.googlesource.com/c/platform/build/+/874473
+ delayedCheckApiFiles = true
+ } else {
+ val stableApiFile = stringToExistingFile(getValue(args, ++index))
+ val apiFileToBeTested = stringToExistingFile(getValue(args, ++index))
+ val stableRemovedApiFile = stringToExistingFile(getValue(args, ++index))
+ val removedApiFileToBeTested = stringToExistingFile(getValue(args, ++index))
+ mutableCompatibilityChecks.add(
+ CheckRequest(
+ stableApiFile,
+ ApiType.PUBLIC_API,
+ ReleaseType.RELEASED,
+ apiFileToBeTested
+ )
)
- )
- mutableCompatibilityChecks.add(
- CheckRequest(
- stableRemovedApiFile,
- ApiType.REMOVED,
- ReleaseType.RELEASED,
- removedApiFileToBeTested
+ mutableCompatibilityChecks.add(
+ CheckRequest(
+ stableRemovedApiFile,
+ ApiType.REMOVED,
+ ReleaseType.RELEASED,
+ removedApiFileToBeTested
+ )
)
- )
+ }
}
ARG_ANNOTATION_COVERAGE_STATS -> dumpAnnotationStatistics = true
@@ -1255,8 +1268,32 @@
throw DriverException(stderr = "Invalid argument $arg\n\n$usage")
}
} else {
- // All args that don't start with "-" are taken to be filenames
- mutableSources.addAll(stringToExistingFiles(arg))
+ if (delayedCheckApiFiles) {
+ delayedCheckApiFiles = false
+ val stableApiFile = stringToExistingFile(arg)
+ val apiFileToBeTested = stringToExistingFile(getValue(args, ++index))
+ val stableRemovedApiFile = stringToExistingFile(getValue(args, ++index))
+ val removedApiFileToBeTested = stringToExistingFile(getValue(args, ++index))
+ mutableCompatibilityChecks.add(
+ CheckRequest(
+ stableApiFile,
+ ApiType.PUBLIC_API,
+ ReleaseType.RELEASED,
+ apiFileToBeTested
+ )
+ )
+ mutableCompatibilityChecks.add(
+ CheckRequest(
+ stableRemovedApiFile,
+ ApiType.REMOVED,
+ ReleaseType.RELEASED,
+ removedApiFileToBeTested
+ )
+ )
+ } else {
+ // All args that don't start with "-" are taken to be filenames
+ mutableSources.addAll(stringToExistingFiles(arg))
+ }
}
}
}
@@ -1318,7 +1355,7 @@
}
} else {
// Add helpful doc in AOSP baseline files?
- val headerComment = if (System.getenv("ANDROID_BUILD_TOP") != null)
+ val headerComment = if (isBuildingAndroid())
"// See tools/metalava/API-LINT.md for how to update this file.\n\n"
else
""
diff --git a/src/main/resources/version.properties b/src/main/resources/version.properties
index 32c9cdf..233e17e 100644
--- a/src/main/resources/version.properties
+++ b/src/main/resources/version.properties
@@ -2,4 +2,4 @@
# Version definition
# This file is read by gradle build scripts, but also packaged with metalava
# as a resource for the Version classes to read.
-metalavaVersion=1.2.4
+metalavaVersion=1.2.5
diff --git a/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt b/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt
index 9b73c0f..6c01e32 100644
--- a/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt
+++ b/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt
@@ -2484,12 +2484,13 @@
fun `Fail on compatible changes that affect signature file contents`() {
// Regression test for 122916999
check(
+ extraArguments = arrayOf(ARG_NO_NATIVE_DIFF),
allowCompatibleDifferences = false,
expectedFail = """
- Aborting: Your changes have resulted in differences in
- the signature file for the public API.
- The changes are compatible, but the signature file needs
- to be updated.
+ Aborting: Your changes have resulted in differences in the signature file
+ for the public API.
+
+ The changes may be compatible, but the signature file needs to be updated.
Diffs:
@@ -5 +5
diff --git a/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt b/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt
index 138ed7b..bc85a78 100644
--- a/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt
+++ b/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt
@@ -1472,6 +1472,13 @@
println("Ignoring external doc test: JDK not found")
return
}
+
+ val version = System.getProperty("java.version")
+ if (!version.startsWith("1.8.")) {
+ println("Javadoc invocation test does not work on Java 1.9 and later; bootclasspath not supported")
+ return
+ }
+
val javadoc = File(jdkPath, "bin/javadoc")
if (!javadoc.isFile) {
println("Ignoring external doc test: javadoc not found *or* not running on Linux/OSX")
diff --git a/src/test/java/com/android/tools/metalava/DriverTest.kt b/src/test/java/com/android/tools/metalava/DriverTest.kt
index 2553a5a..f862f34 100644
--- a/src/test/java/com/android/tools/metalava/DriverTest.kt
+++ b/src/test/java/com/android/tools/metalava/DriverTest.kt
@@ -108,6 +108,7 @@
// the signature was passed at the same time
// ignore
} else {
+ assertEquals(expectedFail, actualFail)
fail(actualFail)
}
}
@@ -384,7 +385,7 @@
}
Errors.resetLevels()
- /** Expected output if exiting with an error code */
+ @Suppress("NAME_SHADOWING")
val expectedFail = expectedFail ?: if (checkCompatibilityApi != null ||
checkCompatibilityApiReleased != null ||
checkCompatibilityRemovedApiCurrent != null ||