From 88b9905d74cf55d22bc29ff180f81c500133ff30 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Mon, 27 Apr 2020 17:22:16 -0700 Subject: [PATCH] Merge the "big 3" metalava invocations into one This CL merges the biggest three metalava invocations -- stub and signature file generation, api lint, and "check released" -- into one call. For each of the existing separate rules for api lint and check released this change: 1) Stopped creating a separate rule containing its own metalava invocation. 2) Moved the code for creating the rule earlier so it can add extra flags to the combined metalava command. 3) Moved the message handling into metalava, allowing the message to be passed in. 4) Added a flag to record whether the specific check was needed so that you can separate the timestamp file creation out to run after the merged metalava command. A couple of minor bug fixes and clarifications. Bug: 151160048 Test: "m" and treehugger Test: Apply https://android-review.googlesource.com/c/platform/frameworks/base/+/1295541 and run `NINJA_ARGS="-k 999" m checkapi` -> Make sure there are 4 kinds of lint failures ("AddedFinal" and "VisiblySynchronized") with the valid error messages, from: - api-stubs-docs - system-api-stubs-docs - test-api-stubs-docs - module-lib-api- Test: Run the 4 cp commands shown by lint -> Make sure the following 4 baseline files are updated: frameworks/base/api/lint-baseline.txt frameworks/base/api/module-lib-lint-baseline.txt frameworks/base/api/system-lint-baseline.txt frameworks/base/api/test-lint-baseline.txt Test: Then run `NINJA_ARGS="-k 999" m checkapi` again. -> Make sure the API lint errors are gone -> Make sure "api-stubs-docs" shows the "You have tried to change the API from what has been previously released in ..." error. (because of "AddedFinal") -> Make sure it the other 3 API surfaces show the "You have tried to change the API from what has been previously approved" error. Test: Remove "final" from Intent.java and run "NINJA_ARGS="-k 999" m checkapi" again. -> This time, all the 4 API surfaces say "You have tried to change the API from what has been previously approved" error." Test: Run the 4 "make *-update-current-api" commands shown in the above errors and "NINJA_ARGS="-k 999" m checkapi" -> The exact command is: $ make \ module-lib-api-update-current-api \ api-stubs-docs-update-current-api \ test-api-stubs-docs-update-current-api \ system-api-stubs-docs-update-current-api -> Make sure frameworks/base/api/*-current.txt are properly updated. Test: Run "m update-api" -> Make sure no text signature files get updated. Test: Run "m droid". -> Make sure everything builds fine. Change-Id: Ia1a5510b57d58748b4655473d4585636deb7f45e Merged-In: Ia1a5510b57d58748b4655473d4585636deb7f45e --- java/droiddoc.go | 241 ++++++++++++++++++++++------------------------- 1 file changed, 111 insertions(+), 130 deletions(-) diff --git a/java/droiddoc.go b/java/droiddoc.go index 1c46a2b0d..879353d8b 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -1473,6 +1473,104 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { cmd.ImplicitOutput(android.PathForModuleGen(ctx, o)) } + // Add options for the other optional tasks: API-lint and check-released. + // We generate separate timestamp files for them. + + doApiLint := false + doCheckReleased := false + + // Add API lint options. + + if BoolDefault(d.properties.Check_api.Api_lint.Enabled, false) && !ctx.Config().IsPdkBuild() { + doApiLint = true + + newSince := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Api_lint.New_since) + if newSince.Valid() { + cmd.FlagWithInput("--api-lint ", newSince.Path()) + } else { + cmd.Flag("--api-lint") + } + d.apiLintReport = android.PathForModuleOut(ctx, "api_lint_report.txt") + cmd.FlagWithOutput("--report-even-if-suppressed ", d.apiLintReport) // TODO: Change to ":api-lint" + + baselineFile := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Api_lint.Baseline_file) + updatedBaselineOutput := android.PathForModuleOut(ctx, "api_lint_baseline.txt") + d.apiLintTimestamp = android.PathForModuleOut(ctx, "api_lint.timestamp") + + // Note this string includes a special shell quote $' ... ', which decodes the "\n"s. + // However, because $' ... ' doesn't expand environmental variables, we can't just embed + // $PWD, so we have to terminate $'...', use "$PWD", then start $' ... ' again, + // which is why we have '"$PWD"$' in it. + // + // TODO: metalava also has a slightly different message hardcoded. Should we unify this + // message and metalava's one? + msg := `$'` + // Enclose with $' ... ' + `************************************************************\n` + + `Your API changes are triggering API Lint warnings or errors.\n` + + `To make these errors go away, fix the code according to the\n` + + `error and/or warning messages above.\n` + + `\n` + + `If it is not possible to do so, there are workarounds:\n` + + `\n` + + `1. You can suppress the errors with @SuppressLint("")\n` + + if baselineFile.Valid() { + cmd.FlagWithInput("--baseline:api-lint ", baselineFile.Path()) + cmd.FlagWithOutput("--update-baseline:api-lint ", updatedBaselineOutput) + + msg += fmt.Sprintf(``+ + `2. You can update the baseline by executing the following\n`+ + ` command:\n`+ + ` cp \\ \n`+ + ` "'"$PWD"$'/%s" \\ \n`+ + ` "'"$PWD"$'/%s" \n`+ + ` To submit the revised baseline.txt to the main Android\n`+ + ` repository, you will need approval.\n`, updatedBaselineOutput, baselineFile.Path()) + } else { + msg += fmt.Sprintf(``+ + `2. You can add a baseline file of existing lint failures\n`+ + ` to the build rule of %s.\n`, d.Name()) + } + // Note the message ends with a ' (single quote), to close the $' ... ' . + msg += `************************************************************\n'` + + cmd.FlagWithArg("--error-message:api-lint ", msg) + } + + // Add "check released" options. (Detect incompatible API changes from the last public release) + + if apiCheckEnabled(ctx, d.properties.Check_api.Last_released, "last_released") && + !ctx.Config().IsPdkBuild() { + doCheckReleased = true + + if len(d.Javadoc.properties.Out) > 0 { + ctx.PropertyErrorf("out", "out property may not be combined with check_api") + } + + apiFile := android.PathForModuleSrc(ctx, String(d.properties.Check_api.Last_released.Api_file)) + removedApiFile := android.PathForModuleSrc(ctx, String(d.properties.Check_api.Last_released.Removed_api_file)) + baselineFile := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Last_released.Baseline_file) + updatedBaselineOutput := android.PathForModuleOut(ctx, "last_released_baseline.txt") + + d.checkLastReleasedApiTimestamp = android.PathForModuleOut(ctx, "check_last_released_api.timestamp") + + cmd.FlagWithInput("--check-compatibility:api:released ", apiFile) + cmd.FlagWithInput("--check-compatibility:removed:released ", removedApiFile) + + if baselineFile.Valid() { + cmd.FlagWithInput("--baseline:compatibility:released ", baselineFile.Path()) + cmd.FlagWithOutput("--update-baseline:compatibility:released ", updatedBaselineOutput) + } + + // Note this string includes quote ($' ... '), which decodes the "\n"s. + msg := `$'\n******************************\n` + + `You have tried to change the API from what has been previously released in\n` + + `an SDK. Please fix the errors listed above.\n` + + `******************************\n'` + + cmd.FlagWithArg("--error-message:compatibility:released ", msg) + } + if generateStubs { rule.Command(). BuiltTool(ctx, "soong_zip"). @@ -1494,83 +1592,20 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { FlagWithArg("-D ", d.metadataDir.String()) } + // TODO: We don't really need two separate API files, but this is a reminiscence of how + // we used to run metalava separately for API lint and the "last_released" check. Unify them. + if doApiLint { + rule.Command().Text("touch").Output(d.apiLintTimestamp) + } + if doCheckReleased { + rule.Command().Text("touch").Output(d.checkLastReleasedApiTimestamp) + } + rule.Restat() zipSyncCleanupCmd(rule, srcJarDir) - rule.Build(pctx, ctx, "metalava", "metalava") - - // Create rule for apicheck - - if BoolDefault(d.properties.Check_api.Api_lint.Enabled, false) && !ctx.Config().IsPdkBuild() { - rule := android.NewRuleBuilder() - rule.Command().Text("( true") - - srcJarDir := android.PathForModuleOut(ctx, "api_lint", "srcjars") - srcJarList := zipSyncCmd(ctx, rule, srcJarDir, d.Javadoc.srcJars) - - cmd := metalavaCmd(ctx, rule, javaVersion, d.Javadoc.srcFiles, srcJarList, - deps.bootClasspath, deps.classpath, d.Javadoc.sourcepaths) - - cmd.Flag(d.Javadoc.args).Implicits(d.Javadoc.argFiles) - - newSince := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Api_lint.New_since) - if newSince.Valid() { - cmd.FlagWithInput("--api-lint ", newSince.Path()) - } else { - cmd.Flag("--api-lint") - } - d.apiLintReport = android.PathForModuleOut(ctx, "api_lint_report.txt") - cmd.FlagWithOutput("--report-even-if-suppressed ", d.apiLintReport) - - d.inclusionAnnotationsFlags(ctx, cmd) - d.mergeAnnoDirFlags(ctx, cmd) - - baselineFile := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Api_lint.Baseline_file) - updatedBaselineOutput := android.PathForModuleOut(ctx, "api_lint_baseline.txt") - d.apiLintTimestamp = android.PathForModuleOut(ctx, "api_lint.timestamp") - - msg := `` + - `************************************************************\n` + - `Your API changes are triggering API Lint warnings or errors.\n` + - `To make these errors go away, fix the code according to the\n` + - `error and/or warning messages above.\n` + - `\n` + - `If it's not possible to do so, there are workarounds:\n` + - `\n` + - `1. You can suppress the errors with @SuppressLint(\"\")\n` - - if baselineFile.Valid() { - cmd.FlagWithInput("--baseline ", baselineFile.Path()) - cmd.FlagWithOutput("--update-baseline ", updatedBaselineOutput) - - msg += fmt.Sprintf(``+ - `2. You can update the baseline by executing the following\n`+ - ` command:\n`+ - ` cp \\ \n`+ - ` \"$PWD/%s\" \\ \n`+ - ` \"$PWD/%s\" \n`+ - ` To submit the revised baseline.txt to the main Android\n`+ - ` repository, you will need approval.\n`, updatedBaselineOutput, baselineFile.Path()) - } else { - msg += fmt.Sprintf(``+ - `2. You can add a baseline file of existing lint failures\n`+ - ` to the build rule of %s.\n`, d.Name()) - } - msg += `************************************************************\n` - - zipSyncCleanupCmd(rule, srcJarDir) - - rule.Command(). - Text("touch").Output(d.apiLintTimestamp). - Text(") || ("). - Text("echo").Flag("-e").Flag(`"` + msg + `"`). - Text("; exit 38"). - Text(")") - - rule.Build(pctx, ctx, "metalavaApiLint", "metalava API lint") - - } + rule.Build(pctx, ctx, "metalava", "metalava merged") if apiCheckEnabled(ctx, d.properties.Check_api.Current, "current") && !ctx.Config().IsPdkBuild() { @@ -1584,7 +1619,7 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { baselineFile := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Current.Baseline_file) if baselineFile.Valid() { - ctx.PropertyErrorf("current API check can't have a baseline file. (module %s)", ctx.ModuleName()) + ctx.PropertyErrorf("baseline_file", "current API check can't have a baseline file. (module %s)", ctx.ModuleName()) } d.checkCurrentApiTimestamp = android.PathForModuleOut(ctx, "check_current_api.timestamp") @@ -1592,8 +1627,8 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { rule := android.NewRuleBuilder() // Diff command line. - // -F matches the closest "opening" line, such as "package xxx{" - // and " public class Yyy {". + // -F matches the closest "opening" line, such as "package android {" + // and " public class Intent {". diff := `diff -u -F '{ *$'` rule.Command().Text("( true") @@ -1652,60 +1687,6 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) { rule.Build(pctx, ctx, "metalavaCurrentApiUpdate", "update current API") } - if apiCheckEnabled(ctx, d.properties.Check_api.Last_released, "last_released") && - !ctx.Config().IsPdkBuild() { - - if len(d.Javadoc.properties.Out) > 0 { - ctx.PropertyErrorf("out", "out property may not be combined with check_api") - } - - apiFile := android.PathForModuleSrc(ctx, String(d.properties.Check_api.Last_released.Api_file)) - removedApiFile := android.PathForModuleSrc(ctx, String(d.properties.Check_api.Last_released.Removed_api_file)) - baselineFile := android.OptionalPathForModuleSrc(ctx, d.properties.Check_api.Last_released.Baseline_file) - updatedBaselineOutput := android.PathForModuleOut(ctx, "last_released_baseline.txt") - - d.checkLastReleasedApiTimestamp = android.PathForModuleOut(ctx, "check_last_released_api.timestamp") - - rule := android.NewRuleBuilder() - - rule.Command().Text("( true") - - srcJarDir := android.PathForModuleOut(ctx, "last-apicheck", "srcjars") - srcJarList := zipSyncCmd(ctx, rule, srcJarDir, d.Javadoc.srcJars) - - cmd := metalavaCmd(ctx, rule, javaVersion, d.Javadoc.srcFiles, srcJarList, - deps.bootClasspath, deps.classpath, d.Javadoc.sourcepaths) - - cmd.Flag(d.Javadoc.args).Implicits(d.Javadoc.argFiles). - FlagWithInput("--check-compatibility:api:released ", apiFile) - - d.inclusionAnnotationsFlags(ctx, cmd) - - cmd.FlagWithInput("--check-compatibility:removed:released ", removedApiFile) - - d.mergeAnnoDirFlags(ctx, cmd) - - if baselineFile.Valid() { - cmd.FlagWithInput("--baseline ", baselineFile.Path()) - cmd.FlagWithOutput("--update-baseline ", updatedBaselineOutput) - } - - zipSyncCleanupCmd(rule, srcJarDir) - - msg := `\n******************************\n` + - `You have tried to change the API from what has been previously released in\n` + - `an SDK. Please fix the errors listed above.\n` + - `******************************\n` - rule.Command(). - Text("touch").Output(d.checkLastReleasedApiTimestamp). - Text(") || ("). - Text("echo").Flag("-e").Flag(`"` + msg + `"`). - Text("; exit 38"). - Text(")") - - rule.Build(pctx, ctx, "metalavaLastApiCheck", "metalava check last API") - } - if String(d.properties.Check_nullability_warnings) != "" { if d.nullabilityWarningsFile == nil { ctx.PropertyErrorf("check_nullability_warnings",