Add current api check as validation for from-text stub generation

Currently, there is no build action ensuring that the API text files are
up to date, unless a user runs `m checkapi` or `m update-api`. This
means that the user must run `m update-api` after making a change that
modifies api surface(s), so that the API text file reflects the local
change. This adds additional layer of action to developers, and it is
not guaranteed that the developer will always run `m update-api` after
making an api surface-affecting changes.

To prevent such mistake, this change adds droidstub-level api check as
validation for from-text stub generation. With this change, the build
will fail if the API text file is not up to date and the user must run
`m update-api` when making api surface-affecting local changes.

The validation is done by adding all droidstubs modules associated with
the java_api_contributions passed to java_api_library via
`api_contributions` as dependency and setting the current api timestamp
files as the validations for the from-text stubs generating build rule.

The full api surface libraries will not run the validations to avoid
circular dependency. However, all java_sdk_library generated
java_api_library modules will run validations, mapped to the droidstubs
in the same api domaion.

If the user sets the environment variable `DISABLE_STUB_VALIDATION=true`, validation
actions are not run. Validation actions run by default.

Test: m nothing --build-from-text-stub and run ninja query to verify `check_current_api.timestamp`s are listed as validation \
      DISABLE_STUB_VALIDATION=true m nothing --build-from-text-stub and run ninja query to verify that validation actions are not added
Bug: 288624417
Change-Id: I329e6438fe8f3ac30d8c6a971d57853ed6b0d150
This commit is contained in:
Jihoon Kang 2023-06-28 01:16:23 +00:00
parent d30ac8a32a
commit 063ec003f9
6 changed files with 220 additions and 53 deletions

View file

@ -66,6 +66,7 @@ java_api_library {
libs: [ libs: [
"stub-annotations", "stub-annotations",
], ],
enable_validation: false,
} }
java_library { java_library {

View file

@ -171,6 +171,10 @@ type ApiStubsProvider interface {
ApiStubsSrcProvider ApiStubsSrcProvider
} }
type currentApiTimestampProvider interface {
CurrentApiTimestamp() android.Path
}
// droidstubs passes sources files through Metalava to generate stub .java files that only contain the API to be // droidstubs passes sources files through Metalava to generate stub .java files that only contain the API to be
// documented, filtering out hidden classes and methods. The resulting .java files are intended to be passed to // documented, filtering out hidden classes and methods. The resulting .java files are intended to be passed to
// a droiddoc module to generate documentation. // a droiddoc module to generate documentation.
@ -238,10 +242,15 @@ func (d *Droidstubs) StubsSrcJar() android.Path {
return d.stubsSrcJar return d.stubsSrcJar
} }
func (d *Droidstubs) CurrentApiTimestamp() android.Path {
return d.checkCurrentApiTimestamp
}
var metalavaMergeAnnotationsDirTag = dependencyTag{name: "metalava-merge-annotations-dir"} var metalavaMergeAnnotationsDirTag = dependencyTag{name: "metalava-merge-annotations-dir"}
var metalavaMergeInclusionAnnotationsDirTag = dependencyTag{name: "metalava-merge-inclusion-annotations-dir"} var metalavaMergeInclusionAnnotationsDirTag = dependencyTag{name: "metalava-merge-inclusion-annotations-dir"}
var metalavaAPILevelsAnnotationsDirTag = dependencyTag{name: "metalava-api-levels-annotations-dir"} var metalavaAPILevelsAnnotationsDirTag = dependencyTag{name: "metalava-api-levels-annotations-dir"}
var metalavaAPILevelsModuleTag = dependencyTag{name: "metalava-api-levels-module-tag"} var metalavaAPILevelsModuleTag = dependencyTag{name: "metalava-api-levels-module-tag"}
var metalavaCurrentApiTimestampTag = dependencyTag{name: "metalava-current-api-timestamp-tag"}
func (d *Droidstubs) DepsMutator(ctx android.BottomUpMutatorContext) { func (d *Droidstubs) DepsMutator(ctx android.BottomUpMutatorContext) {
d.Javadoc.addDeps(ctx) d.Javadoc.addDeps(ctx)

View file

@ -392,7 +392,7 @@ func TestGeneratedApiContributionVisibilityTest(t *testing.T) {
removed_api_file: "A/removed.txt", removed_api_file: "A/removed.txt",
} }
}, },
visibility: ["//a"], visibility: ["//a", "//b"],
} }
`, `,
map[string][]byte{ map[string][]byte{

View file

@ -1664,6 +1664,8 @@ type ApiLibrary struct {
extractedSrcJar android.WritablePath extractedSrcJar android.WritablePath
// .dex of stubs, used for hiddenapi processing // .dex of stubs, used for hiddenapi processing
dexJarFile OptionalDexJarPath dexJarFile OptionalDexJarPath
validationPaths android.Paths
} }
type JavaApiLibraryProperties struct { type JavaApiLibraryProperties struct {
@ -1699,6 +1701,12 @@ type JavaApiLibraryProperties struct {
// The jar will also be passed to metalava as a classpath to // The jar will also be passed to metalava as a classpath to
// generate compilable stubs. // generate compilable stubs.
System_modules *string System_modules *string
// If true, the module runs validation on the API signature files provided
// by the modules passed via api_contributions by checking if the files are
// in sync with the source Java files. However, the environment variable
// DISABLE_STUB_VALIDATION has precedence over this property.
Enable_validation *bool
} }
func ApiLibraryFactory() android.Module { func ApiLibraryFactory() android.Module {
@ -1787,6 +1795,12 @@ func (al *ApiLibrary) stubsFlags(ctx android.ModuleContext, cmd *android.RuleBui
} }
} }
func (al *ApiLibrary) addValidation(ctx android.ModuleContext, cmd *android.RuleBuilderCommand, validationPaths android.Paths) {
for _, validationPath := range validationPaths {
cmd.Validation(validationPath)
}
}
// This method extracts the stub class files from the stub jar file provided // This method extracts the stub class files from the stub jar file provided
// from full_api_surface_stub module instead of compiling the srcjar generated from invoking metalava. // from full_api_surface_stub module instead of compiling the srcjar generated from invoking metalava.
// This method is used because metalava can generate compilable from-text stubs only when // This method is used because metalava can generate compilable from-text stubs only when
@ -1823,8 +1837,28 @@ func (al *ApiLibrary) extractApiSrcs(ctx android.ModuleContext, rule *android.Ru
func (al *ApiLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { func (al *ApiLibrary) DepsMutator(ctx android.BottomUpMutatorContext) {
apiContributions := al.properties.Api_contributions apiContributions := al.properties.Api_contributions
addValidations := !ctx.Config().IsEnvTrue("DISABLE_STUB_VALIDATION") &&
proptools.BoolDefault(al.properties.Enable_validation, true)
for _, apiContributionName := range apiContributions { for _, apiContributionName := range apiContributions {
ctx.AddDependency(ctx.Module(), javaApiContributionTag, apiContributionName) ctx.AddDependency(ctx.Module(), javaApiContributionTag, apiContributionName)
// Add the java_api_contribution module generating droidstubs module
// as dependency when validation adding conditions are met and
// the java_api_contribution module name has ".api.contribution" suffix.
// All droidstubs-generated modules possess the suffix in the name,
// but there is no such guarantee for tests.
if addValidations {
if strings.HasSuffix(apiContributionName, ".api.contribution") {
ctx.AddDependency(ctx.Module(), metalavaCurrentApiTimestampTag, strings.TrimSuffix(apiContributionName, ".api.contribution"))
} else {
ctx.ModuleErrorf("Validation is enabled for module %s but a "+
"current timestamp provider is not found for the api "+
"contribution %s",
ctx.ModuleName(),
apiContributionName,
)
}
}
} }
ctx.AddVariationDependencies(nil, libTag, al.properties.Libs...) ctx.AddVariationDependencies(nil, libTag, al.properties.Libs...)
ctx.AddVariationDependencies(nil, staticLibTag, al.properties.Static_libs...) ctx.AddVariationDependencies(nil, staticLibTag, al.properties.Static_libs...)
@ -1862,8 +1896,7 @@ func (al *ApiLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
android.PathForModuleOut(ctx, "metalava.sbox.textproto")). android.PathForModuleOut(ctx, "metalava.sbox.textproto")).
SandboxInputs() SandboxInputs()
var stubsDir android.OptionalPath stubsDir := android.OptionalPathForPath(android.PathForModuleOut(ctx, "metalava", "stubsDir"))
stubsDir = android.OptionalPathForPath(android.PathForModuleOut(ctx, "metalava", "stubsDir"))
rule.Command().Text("rm -rf").Text(stubsDir.String()) rule.Command().Text("rm -rf").Text(stubsDir.String())
rule.Command().Text("mkdir -p").Text(stubsDir.String()) rule.Command().Text("mkdir -p").Text(stubsDir.String())
@ -1895,6 +1928,10 @@ func (al *ApiLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
case systemModulesTag: case systemModulesTag:
module := dep.(SystemModulesProvider) module := dep.(SystemModulesProvider)
systemModulesPaths = append(systemModulesPaths, module.HeaderJars()...) systemModulesPaths = append(systemModulesPaths, module.HeaderJars()...)
case metalavaCurrentApiTimestampTag:
if currentApiTimestampProvider, ok := dep.(currentApiTimestampProvider); ok {
al.validationPaths = append(al.validationPaths, currentApiTimestampProvider.CurrentApiTimestamp())
}
} }
}) })
@ -1918,6 +1955,8 @@ func (al *ApiLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
cmd.FlagWithInput("--migrate-nullness ", previousApi) cmd.FlagWithInput("--migrate-nullness ", previousApi)
} }
al.addValidation(ctx, cmd, al.validationPaths)
al.stubsSrcJar = android.PathForModuleOut(ctx, "metalava", ctx.ModuleName()+"-"+"stubs.srcjar") al.stubsSrcJar = android.PathForModuleOut(ctx, "metalava", ctx.ModuleName()+"-"+"stubs.srcjar")
al.stubsJarWithoutStaticLibs = android.PathForModuleOut(ctx, "metalava", "stubs.jar") al.stubsJarWithoutStaticLibs = android.PathForModuleOut(ctx, "metalava", "stubs.jar")
al.stubsJar = android.PathForModuleOut(ctx, ctx.ModuleName(), fmt.Sprintf("%s.jar", ctx.ModuleName())) al.stubsJar = android.PathForModuleOut(ctx, ctx.ModuleName(), fmt.Sprintf("%s.jar", ctx.ModuleName()))

View file

@ -1847,9 +1847,17 @@ func TestDeviceBinaryWrapperGeneration(t *testing.T) {
} }
func TestJavaApiContributionEmptyApiFile(t *testing.T) { func TestJavaApiContributionEmptyApiFile(t *testing.T) {
testJavaError(t, android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(
"Error: foo has an empty api file.", "Error: foo has an empty api file.",
`java_api_contribution { )).RunTestWithBp(t, `
java_api_contribution {
name: "foo", name: "foo",
} }
java_api_library { java_api_library {
@ -1874,7 +1882,20 @@ func TestJavaApiLibraryAndProviderLink(t *testing.T) {
api_surface: "public", api_surface: "public",
} }
` `
ctx, _ := testJavaWithFS(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeMockFs(
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
},
),
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_api_library { java_api_library {
name: "bar1", name: "bar1",
api_surface: "public", api_surface: "public",
@ -1886,11 +1907,7 @@ func TestJavaApiLibraryAndProviderLink(t *testing.T) {
api_surface: "system", api_surface: "system",
api_contributions: ["foo1", "foo2"], api_contributions: ["foo1", "foo2"],
} }
`, `)
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
})
testcases := []struct { testcases := []struct {
moduleName string moduleName string
@ -1944,7 +1961,22 @@ func TestJavaApiLibraryAndDefaultsLink(t *testing.T) {
api_surface: "system", api_surface: "system",
} }
` `
ctx, _ := testJavaWithFS(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeMockFs(
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(provider_bp_c),
"d/Android.bp": []byte(provider_bp_d),
},
),
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_defaults { java_defaults {
name: "baz1", name: "baz1",
api_surface: "public", api_surface: "public",
@ -1975,13 +2007,7 @@ func TestJavaApiLibraryAndDefaultsLink(t *testing.T) {
defaults:["baz1", "baz2"], defaults:["baz1", "baz2"],
api_contributions: ["foo4"], api_contributions: ["foo4"],
} }
`, `)
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(provider_bp_c),
"d/Android.bp": []byte(provider_bp_d),
})
testcases := []struct { testcases := []struct {
moduleName string moduleName string
@ -2026,7 +2052,20 @@ func TestJavaApiLibraryJarGeneration(t *testing.T) {
api_surface: "public", api_surface: "public",
} }
` `
ctx, _ := testJavaWithFS(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeMockFs(
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
},
),
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_api_library { java_api_library {
name: "bar1", name: "bar1",
api_surface: "public", api_surface: "public",
@ -2038,11 +2077,7 @@ func TestJavaApiLibraryJarGeneration(t *testing.T) {
api_surface: "system", api_surface: "system",
api_contributions: ["foo1", "foo2"], api_contributions: ["foo1", "foo2"],
} }
`, `)
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
})
testcases := []struct { testcases := []struct {
moduleName string moduleName string
@ -2094,7 +2129,24 @@ func TestJavaApiLibraryLibsLink(t *testing.T) {
} }
` `
ctx, _ := testJavaWithFS(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeMockFs(
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(lib_bp_a),
"c/Lib.java": {},
"d/Android.bp": []byte(lib_bp_b),
"d/Lib.java": {},
},
),
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_api_library { java_api_library {
name: "bar1", name: "bar1",
api_surface: "public", api_surface: "public",
@ -2108,15 +2160,7 @@ func TestJavaApiLibraryLibsLink(t *testing.T) {
api_contributions: ["foo1", "foo2"], api_contributions: ["foo1", "foo2"],
libs: ["lib1", "lib2", "bar1"], libs: ["lib1", "lib2", "bar1"],
} }
`, `)
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(lib_bp_a),
"c/Lib.java": {},
"d/Android.bp": []byte(lib_bp_b),
"d/Lib.java": {},
})
testcases := []struct { testcases := []struct {
moduleName string moduleName string
@ -2171,7 +2215,24 @@ func TestJavaApiLibraryStaticLibsLink(t *testing.T) {
} }
` `
ctx, _ := testJavaWithFS(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeMockFs(
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(lib_bp_a),
"c/Lib.java": {},
"d/Android.bp": []byte(lib_bp_b),
"d/Lib.java": {},
},
),
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_api_library { java_api_library {
name: "bar1", name: "bar1",
api_surface: "public", api_surface: "public",
@ -2185,15 +2246,7 @@ func TestJavaApiLibraryStaticLibsLink(t *testing.T) {
api_contributions: ["foo1", "foo2"], api_contributions: ["foo1", "foo2"],
static_libs: ["lib1", "lib2", "bar1"], static_libs: ["lib1", "lib2", "bar1"],
} }
`, `)
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(lib_bp_a),
"c/Lib.java": {},
"d/Android.bp": []byte(lib_bp_b),
"d/Lib.java": {},
})
testcases := []struct { testcases := []struct {
moduleName string moduleName string
@ -2242,19 +2295,28 @@ func TestJavaApiLibraryFullApiSurfaceStub(t *testing.T) {
} }
` `
ctx, _ := testJavaWithFS(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeMockFs(
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(lib_bp_a),
},
),
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_api_library { java_api_library {
name: "bar1", name: "bar1",
api_surface: "public", api_surface: "public",
api_contributions: ["foo1"], api_contributions: ["foo1"],
full_api_surface_stub: "lib1", full_api_surface_stub: "lib1",
} }
`, `)
map[string][]byte{
"a/Android.bp": []byte(provider_bp_a),
"b/Android.bp": []byte(provider_bp_b),
"c/Android.bp": []byte(lib_bp_a),
})
m := ctx.ModuleForTests("bar1", "android_common") m := ctx.ModuleForTests("bar1", "android_common")
manifest := m.Output("metalava.sbox.textproto") manifest := m.Output("metalava.sbox.textproto")
@ -2402,7 +2464,14 @@ func TestHeadersOnly(t *testing.T) {
} }
func TestJavaApiContributionImport(t *testing.T) { func TestJavaApiContributionImport(t *testing.T) {
ctx, _ := testJava(t, ` ctx := android.GroupFixturePreparers(
prepareForJavaTest,
android.FixtureMergeEnv(
map[string]string{
"DISABLE_STUB_VALIDATION": "true",
},
),
).RunTestWithBp(t, `
java_api_library { java_api_library {
name: "foo", name: "foo",
api_contributions: ["bar"], api_contributions: ["bar"],
@ -2483,3 +2552,50 @@ func TestSdkLibraryProvidesSystemModulesToApiLibrary(t *testing.T) {
classPathFlag := "--classpath __SBOX_SANDBOX_DIR__/out/.intermediates/bar/android_common/turbine-combined/bar.jar" classPathFlag := "--classpath __SBOX_SANDBOX_DIR__/out/.intermediates/bar/android_common/turbine-combined/bar.jar"
android.AssertStringDoesContain(t, "command expected to contain classpath flag", manifestCommand, classPathFlag) android.AssertStringDoesContain(t, "command expected to contain classpath flag", manifestCommand, classPathFlag)
} }
func TestApiLibraryDroidstubsDependency(t *testing.T) {
result := android.GroupFixturePreparers(
prepareForJavaTest,
PrepareForTestWithJavaSdkLibraryFiles,
FixtureWithLastReleaseApis("foo"),
android.FixtureModifyConfig(func(config android.Config) {
config.SetApiLibraries([]string{"foo"})
}),
android.FixtureMergeMockFs(
map[string][]byte{
"A.java": nil,
},
),
).RunTestWithBp(t, `
java_api_library {
name: "foo",
api_contributions: [
"api-stubs-docs-non-updatable.api.contribution",
],
enable_validation: true,
}
java_api_library {
name: "bar",
api_contributions: [
"api-stubs-docs-non-updatable.api.contribution",
],
enable_validation: false,
}
`)
currentApiTimestampPath := "api-stubs-docs-non-updatable/android_common/metalava/check_current_api.timestamp"
foo := result.ModuleForTests("foo", "android_common").Module().(*ApiLibrary)
fooValidationPathsString := strings.Join(foo.validationPaths.Strings(), " ")
bar := result.ModuleForTests("bar", "android_common").Module().(*ApiLibrary)
barValidationPathsString := strings.Join(bar.validationPaths.Strings(), " ")
android.AssertStringDoesContain(t,
"Module expected to have validation",
fooValidationPathsString,
currentApiTimestampPath,
)
android.AssertStringDoesNotContain(t,
"Module expected to not have validation",
barValidationPathsString,
currentApiTimestampPath,
)
}

View file

@ -1838,6 +1838,7 @@ func (module *SdkLibrary) createApiLibrary(mctx android.DefaultableHookContext,
Static_libs []string Static_libs []string
Full_api_surface_stub *string Full_api_surface_stub *string
System_modules *string System_modules *string
Enable_validation *bool
}{} }{}
props.Name = proptools.StringPtr(module.apiLibraryModuleName(apiScope)) props.Name = proptools.StringPtr(module.apiLibraryModuleName(apiScope))
@ -1885,6 +1886,7 @@ func (module *SdkLibrary) createApiLibrary(mctx android.DefaultableHookContext,
} }
props.System_modules = module.deviceProperties.System_modules props.System_modules = module.deviceProperties.System_modules
props.Enable_validation = proptools.BoolPtr(true)
mctx.CreateModule(ApiLibraryFactory, &props) mctx.CreateModule(ApiLibraryFactory, &props)
} }