diff --git a/src/main/java/com/android/tools/metalava/Diff.kt b/src/main/java/com/android/tools/metalava/Diff.kt index d0120eaacd626f86d6240019ca3b297b07e832ff..cd7f2e4bae7d3db74b2943cdd1d10bbc2f9f08b2 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: String, after: String, windowSize: Int): String { ) } -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 38b5ead3a7156e1310714561e1cd63c8d7521cf9..44eed3b410b3c0c72b61d468d2f5bfad8846b55b 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 @@ class DocAnalyzer( } 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 f9cdaabfaddf7ba7d48bebd2cbfdfbb25dc6b0e4..400c139b48cbaaccce9d7590f0b54c1546039bac 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 @@ package com.android.tools.metalava 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 @@ fun run( 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 @@ fun run( stdout.println("\"$arg\",") } stdout.println("----------------------------") + stdout.flush() } newArgs } @@ -145,6 +144,8 @@ fun run( } exitValue = true } catch (e: DriverException) { + stdout.flush() + stderr.flush() if (e.stderr.isNotBlank()) { stderr.println("\n${e.stderr}") } @@ -166,7 +167,7 @@ fun run( stdout.flush() stderr.flush() - if (setExitCode && reporter.hasErrors()) { + if (setExitCode) { exit(exitCode) } @@ -621,14 +622,18 @@ fun checkCompatibility( 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 @@ private fun createProjectEnvironment(): LintCoreProjectEnvironment { 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(file: File): String? { 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 2f32b2697dc7cd87dd125340477c0c2d196e14d9..33a26da263eca01836e683ffc9d7a7fb77963d92 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_API_RELEASED = "--check-compatibility:api:rele 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 @@ class Options( */ 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 @@ class Options( 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 @@ class Options( } 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 @@ class Options( 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 @@ class Options( } } 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 32c9cdf10798cddfd32bdeb9674dbe23f36722f8..233e17ea97e1c8f6cf1cc9d0fa73c97a81797f0f 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 9b73c0fa27d43f21381c12156cc3afa8c65522ce..6c01e32849a29e733c05e0fc4d821d9d86937be1 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 @@ CompatibilityCheckTest : DriverTest() { 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 138ed7b43ec33130cc8c62efde2c396150508554..bc85a784ddef4862dbe33b8210f43aeb5de34a58 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 @@ class DocAnalyzerTest : DriverTest() { 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 2553a5acff0c7b2b2fd1f2163d8531b5b851ee5b..f862f3494177a1bf9ac964a1116617ac84b32c6a 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 @@ abstract class DriverTest { // the signature was passed at the same time // ignore } else { + assertEquals(expectedFail, actualFail) fail(actualFail) } } @@ -384,7 +385,7 @@ abstract class DriverTest { } Errors.resetLevels() - /** Expected output if exiting with an error code */ + @Suppress("NAME_SHADOWING") val expectedFail = expectedFail ?: if (checkCompatibilityApi != null || checkCompatibilityApiReleased != null || checkCompatibilityRemovedApiCurrent != null ||