Frozen requirements can explain themselves.
Bug: 261198242
Test: aidl_test
Change-Id: I420a00f9bb92687e86c2aa73a2b8b19d2f0386b9
diff --git a/build/aidl_gen_rule.go b/build/aidl_gen_rule.go
index 472a93e..9e7c950 100644
--- a/build/aidl_gen_rule.go
+++ b/build/aidl_gen_rule.go
@@ -63,23 +63,24 @@
)
type aidlGenProperties struct {
- Srcs []string `android:"path"`
- AidlRoot string // base directory for the input aidl file
- Imports []string
- Headers []string
- Stability *string
- Min_sdk_version *string
- Platform_apis bool
- Lang string // target language [java|cpp|ndk|rust]
- BaseName string
- GenLog bool
- Version string
- GenRpc bool
- GenTrace bool
- Unstable *bool
- NotFrozen bool
- Visibility []string
- Flags []string
+ Srcs []string `android:"path"`
+ AidlRoot string // base directory for the input aidl file
+ Imports []string
+ Headers []string
+ Stability *string
+ Min_sdk_version *string
+ Platform_apis bool
+ Lang string // target language [java|cpp|ndk|rust]
+ BaseName string
+ GenLog bool
+ Version string
+ GenRpc bool
+ GenTrace bool
+ Unstable *bool
+ NotFrozen bool
+ RequireFrozenReason string
+ Visibility []string
+ Flags []string
}
type aidlGenRule struct {
diff --git a/build/aidl_interface.go b/build/aidl_interface.go
index e7076b0..de09e0a 100644
--- a/build/aidl_interface.go
+++ b/build/aidl_interface.go
@@ -105,8 +105,9 @@
// AidlVersionInfo keeps the *-source module for each (aidl_interface & lang) and the list of
// not-frozen versions (which shouldn't be used by other modules)
type AidlVersionInfo struct {
- notFrozen []string
- sourceMap map[string]string
+ notFrozen []string
+ requireFrozenReasons []string
+ sourceMap map[string]string
}
var AidlVersionInfoProvider = blueprint.NewMutatorProvider(AidlVersionInfo{}, "checkAidlGeneratedModules")
@@ -118,6 +119,7 @@
// {foo-V1-ndk-source, foo-V2-ndk-source}.
func (info *AidlVersionInfo) merge(other AidlVersionInfo) []string {
info.notFrozen = append(info.notFrozen, other.notFrozen...)
+ info.requireFrozenReasons = append(info.requireFrozenReasons, other.requireFrozenReasons...)
if other.sourceMap == nil {
return nil
@@ -137,14 +139,15 @@
return nil
}
-func reportUsingNotFrozenError(ctx android.BaseModuleContext, notFrozen []string) {
+func reportUsingNotFrozenError(ctx android.BaseModuleContext, notFrozen []string, requireFrozenReason []string) {
// TODO(b/154066686): Replace it with a common method instead of listing up module types.
// Test libraries are exempted.
if android.InList(ctx.ModuleType(), []string{"cc_test_library", "android_test", "cc_benchmark", "cc_test"}) {
return
}
- for _, name := range notFrozen {
- ctx.ModuleErrorf("%v is an unfrozen development version, and it can't be used because it is either explicitly disabled (with `frozen: false`) or this is a release branch and all interfaces without `owners: ...` specified must be frozen.", name)
+ for i, name := range notFrozen {
+ reason := requireFrozenReason[i]
+ ctx.ModuleErrorf("%v is an unfrozen development version, and it can't be used because %q", name, reason)
}
}
@@ -171,11 +174,14 @@
}
if gen, ok := mctx.Module().(*aidlGenRule); ok {
var notFrozen []string
+ var requireFrozenReasons []string
if gen.properties.NotFrozen {
notFrozen = []string{strings.TrimSuffix(mctx.ModuleName(), "-source")}
+ requireFrozenReasons = []string{gen.properties.RequireFrozenReason}
}
mctx.SetProvider(AidlVersionInfoProvider, AidlVersionInfo{
- notFrozen: notFrozen,
+ notFrozen: notFrozen,
+ requireFrozenReasons: requireFrozenReasons,
sourceMap: map[string]string{
gen.properties.BaseName + "-" + gen.properties.Lang: gen.Name(),
},
@@ -193,7 +199,7 @@
}
})
if !isAidlGeneratedModule(mctx.Module()) && len(info.notFrozen) > 0 {
- reportUsingNotFrozenError(mctx, info.notFrozen)
+ reportUsingNotFrozenError(mctx, info.notFrozen, info.requireFrozenReasons)
}
if mctx.Failed() {
return
@@ -846,6 +852,34 @@
})
}
+func (i *aidlInterface) checkRequireFrozenAndReason(mctx android.EarlyModuleContext) (bool, string) {
+ if proptools.Bool(i.properties.Unstable) {
+ return false, "it's an unstable interface"
+ }
+
+ if proptools.Bool(i.properties.Frozen) {
+ return true, "it's explicitly marked as `frozen: true`"
+ }
+
+ if i.Owner() == "" {
+ if !mctx.Config().DefaultAppTargetSdk(mctx).IsPreview() {
+ return true, "this is a release branch - freeze it or set 'owners:'"
+ } else if mctx.Config().IsEnvTrue("AIDL_FROZEN_REL") {
+ return true, "this is a release branch (simulated by setting AIDL_FROZEN_REL) - freeze it or set 'owners:'"
+ }
+ } else {
+ // has an OWNER
+ // REL branches don't enforce downstream interfaces or owned interfaces
+ // to be frozen. Instead, these interfaces are verified by other tests
+ // like vts_treble_vintf_vendor_test
+ if android.InList(i.Owner(), strings.Fields(mctx.Config().Getenv("AIDL_FROZEN_OWNERS"))) {
+ return true, "the owner field is in environment variable AIDL_FROZEN_OWNERS"
+ }
+ }
+
+ return false, "by default, we don't require the interface to be frozen"
+}
+
func aidlInterfaceHook(mctx android.DefaultableHookContext, i *aidlInterface) {
if hasVersionSuffix(i.ModuleBase.Name()) {
mctx.PropertyErrorf("name", "aidl_interface should not have '-V<number> suffix")
@@ -889,22 +923,11 @@
mctx.PropertyErrorf("owner", "aidl_interface in a soong_namespace must have the 'owner' property set.")
}
- sdkIsFinal := !mctx.Config().DefaultAppTargetSdk(mctx).IsPreview()
- requireFrozenNoOwner := i.Owner() == "" && (sdkIsFinal || mctx.Config().IsEnvTrue("AIDL_FROZEN_REL"))
- requireFrozenWithOwner := i.Owner() != "" && android.InList(i.Owner(), strings.Fields(mctx.Config().Getenv("AIDL_FROZEN_OWNERS")))
- requireFrozenByOwner := requireFrozenNoOwner || requireFrozenWithOwner
-
- // Two different types of 'unstable' here
- // - 'unstable: true' meaning the module is never stable
- // - current unfrozen ToT version
- //
- // OEM branches may remove 'i.Owner()' here to apply the check to all interfaces, in
- // addition to core platform interfaces. Otherwise, we rely on vts_treble_vintf_vendor_test.
- requireFrozenVersion := proptools.BoolDefault(i.properties.Frozen, false) || (!unstable && requireFrozenByOwner)
+ requireFrozenVersion, requireFrozenReason := i.checkRequireFrozenAndReason(mctx)
// surface error early, main check is via checkUnstableModuleMutator
if requireFrozenVersion && !i.hasVersion() {
- mctx.PropertyErrorf("versions", "must be set (need to be frozen) when \"unstable\" is false, PLATFORM_VERSION_CODENAME is REL, and \"owner\" property is missing.")
+ mctx.PropertyErrorf("versions", "must be set (need to be frozen) because: %q", requireFrozenReason)
}
vndkEnabled := proptools.Bool(i.properties.VndkProperties.Vndk.Enabled) ||
@@ -938,16 +961,16 @@
if !shouldGenerate {
continue
}
- libs = append(libs, addLibrary(mctx, i, nextVersion, lang, requireFrozenVersion))
+ libs = append(libs, addLibrary(mctx, i, nextVersion, lang, requireFrozenVersion, requireFrozenReason))
for _, version := range versions {
- libs = append(libs, addLibrary(mctx, i, version, lang, false))
+ libs = append(libs, addLibrary(mctx, i, version, lang, false, "this is a known frozen version"))
}
}
// In the future, we may want to force the -cpp backend to be on host,
// and limit its visibility, even if it's not created normally
if i.shouldGenerateCppBackend() && len(i.properties.Imports) == 0 {
- libs = append(libs, addLibrary(mctx, i, nextVersion, langCppAnalyzer, false))
+ libs = append(libs, addLibrary(mctx, i, nextVersion, langCppAnalyzer, false, "analysis always uses latest version even if frozen"))
}
if unstable {
diff --git a/build/aidl_interface_backends.go b/build/aidl_interface_backends.go
index 9c9e47e..71f8964 100644
--- a/build/aidl_interface_backends.go
+++ b/build/aidl_interface_backends.go
@@ -27,21 +27,21 @@
"github.com/google/blueprint/proptools"
)
-func addLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, lang string, notFrozen bool) string {
+func addLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, lang string, notFrozen bool, requireFrozenReason string) string {
if lang == langJava {
- return addJavaLibrary(mctx, i, version, notFrozen)
+ return addJavaLibrary(mctx, i, version, notFrozen, requireFrozenReason)
} else if lang == langRust {
- return addRustLibrary(mctx, i, version, notFrozen)
+ return addRustLibrary(mctx, i, version, notFrozen, requireFrozenReason)
} else if lang == langCppAnalyzer {
- return addCppAnalyzerLibrary(mctx, i, version, notFrozen)
+ return addCppAnalyzerLibrary(mctx, i, version, notFrozen, requireFrozenReason)
} else if lang == langCpp || lang == langNdk || lang == langNdkPlatform {
- return addCppLibrary(mctx, i, version, lang, notFrozen)
+ return addCppLibrary(mctx, i, version, lang, notFrozen, requireFrozenReason)
} else {
panic(fmt.Errorf("unsupported language backend %q\n", lang))
}
}
-func addCppLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, lang string, notFrozen bool) string {
+func addCppLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, lang string, notFrozen bool, requireFrozenReason string) string {
cppSourceGen := i.versionedName(version) + "-" + lang + "-source"
cppModuleGen := i.versionedName(version) + "-" + lang
@@ -79,20 +79,21 @@
mctx.CreateModule(aidlGenFactory, &nameProperties{
Name: proptools.StringPtr(cppSourceGen),
}, &aidlGenProperties{
- Srcs: srcs,
- AidlRoot: aidlRoot,
- Imports: i.getImportsForVersion(version),
- Headers: i.properties.Headers,
- Stability: i.properties.Stability,
- Min_sdk_version: i.minSdkVersion(lang),
- Lang: lang,
- BaseName: i.ModuleBase.Name(),
- GenLog: genLog,
- Version: i.versionForInitVersionCompat(version),
- GenTrace: genTrace,
- Unstable: i.properties.Unstable,
- NotFrozen: notFrozen,
- Flags: i.flagsForAidlGenRule(version),
+ Srcs: srcs,
+ AidlRoot: aidlRoot,
+ Imports: i.getImportsForVersion(version),
+ Headers: i.properties.Headers,
+ Stability: i.properties.Stability,
+ Min_sdk_version: i.minSdkVersion(lang),
+ Lang: lang,
+ BaseName: i.ModuleBase.Name(),
+ GenLog: genLog,
+ Version: i.versionForInitVersionCompat(version),
+ GenTrace: genTrace,
+ Unstable: i.properties.Unstable,
+ NotFrozen: notFrozen,
+ RequireFrozenReason: requireFrozenReason,
+ Flags: i.flagsForAidlGenRule(version),
})
importExportDependencies := []string{}
@@ -220,7 +221,7 @@
return cppModuleGen
}
-func addCppAnalyzerLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, notFrozen bool) string {
+func addCppAnalyzerLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, notFrozen bool, requireFrozenReason string) string {
cppAnalyzerSourceGen := i.versionedName("") + "-cpp-analyzer-source"
cppAnalyzerModuleGen := i.versionedName("") + "-cpp-analyzer"
@@ -232,17 +233,18 @@
mctx.CreateModule(aidlGenFactory, &nameProperties{
Name: proptools.StringPtr(cppAnalyzerSourceGen),
}, &aidlGenProperties{
- Srcs: srcs,
- AidlRoot: aidlRoot,
- Imports: i.getImportsForVersion(version),
- Stability: i.properties.Stability,
- Min_sdk_version: i.minSdkVersion(langCpp),
- Lang: langCppAnalyzer,
- BaseName: i.ModuleBase.Name(),
- Version: i.versionForInitVersionCompat(version),
- Unstable: i.properties.Unstable,
- NotFrozen: notFrozen,
- Flags: i.flagsForAidlGenRule(version),
+ Srcs: srcs,
+ AidlRoot: aidlRoot,
+ Imports: i.getImportsForVersion(version),
+ Stability: i.properties.Stability,
+ Min_sdk_version: i.minSdkVersion(langCpp),
+ Lang: langCppAnalyzer,
+ BaseName: i.ModuleBase.Name(),
+ Version: i.versionForInitVersionCompat(version),
+ Unstable: i.properties.Unstable,
+ NotFrozen: notFrozen,
+ RequireFrozenReason: requireFrozenReason,
+ Flags: i.flagsForAidlGenRule(version),
})
importExportDependencies := []string{}
@@ -306,7 +308,7 @@
return cppAnalyzerModuleGen
}
-func addJavaLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, notFrozen bool) string {
+func addJavaLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, notFrozen bool, requireFrozenReason string) string {
javaSourceGen := i.versionedName(version) + "-java-source"
javaModuleGen := i.versionedName(version) + "-java"
srcs, aidlRoot := i.srcsForVersion(mctx, version)
@@ -330,21 +332,22 @@
mctx.CreateModule(aidlGenFactory, &nameProperties{
Name: proptools.StringPtr(javaSourceGen),
}, &aidlGenProperties{
- Srcs: srcs,
- AidlRoot: aidlRoot,
- Imports: i.getImportsForVersion(version),
- Headers: i.properties.Headers,
- Stability: i.properties.Stability,
- Min_sdk_version: minSdkVersion,
- Platform_apis: proptools.Bool(i.properties.Backend.Java.Platform_apis),
- Lang: langJava,
- BaseName: i.ModuleBase.Name(),
- Version: version,
- GenRpc: proptools.Bool(i.properties.Backend.Java.Gen_rpc),
- GenTrace: i.genTrace(langJava),
- Unstable: i.properties.Unstable,
- NotFrozen: notFrozen,
- Flags: i.flagsForAidlGenRule(version),
+ Srcs: srcs,
+ AidlRoot: aidlRoot,
+ Imports: i.getImportsForVersion(version),
+ Headers: i.properties.Headers,
+ Stability: i.properties.Stability,
+ Min_sdk_version: minSdkVersion,
+ Platform_apis: proptools.Bool(i.properties.Backend.Java.Platform_apis),
+ Lang: langJava,
+ BaseName: i.ModuleBase.Name(),
+ Version: version,
+ GenRpc: proptools.Bool(i.properties.Backend.Java.Gen_rpc),
+ GenTrace: i.genTrace(langJava),
+ Unstable: i.properties.Unstable,
+ NotFrozen: notFrozen,
+ RequireFrozenReason: requireFrozenReason,
+ Flags: i.flagsForAidlGenRule(version),
})
mctx.CreateModule(aidlImplementationGeneratorFactory, &nameProperties{
@@ -382,7 +385,7 @@
return javaModuleGen
}
-func addRustLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, notFrozen bool) string {
+func addRustLibrary(mctx android.DefaultableHookContext, i *aidlInterface, version string, notFrozen bool, requireFrozenReason string) string {
rustSourceGen := i.versionedName(version) + "-rust-source"
rustModuleGen := i.versionedName(version) + "-rust"
srcs, aidlRoot := i.srcsForVersion(mctx, version)
@@ -396,18 +399,19 @@
mctx.CreateModule(aidlGenFactory, &nameProperties{
Name: proptools.StringPtr(rustSourceGen),
}, &aidlGenProperties{
- Srcs: srcs,
- AidlRoot: aidlRoot,
- Imports: i.getImportsForVersion(version),
- Headers: i.properties.Headers,
- Stability: i.properties.Stability,
- Min_sdk_version: i.minSdkVersion(langRust),
- Lang: langRust,
- BaseName: i.ModuleBase.Name(),
- Version: i.versionForInitVersionCompat(version),
- Unstable: i.properties.Unstable,
- NotFrozen: notFrozen,
- Flags: i.flagsForAidlGenRule(version),
+ Srcs: srcs,
+ AidlRoot: aidlRoot,
+ Imports: i.getImportsForVersion(version),
+ Headers: i.properties.Headers,
+ Stability: i.properties.Stability,
+ Min_sdk_version: i.minSdkVersion(langRust),
+ Lang: langRust,
+ BaseName: i.ModuleBase.Name(),
+ Version: i.versionForInitVersionCompat(version),
+ Unstable: i.properties.Unstable,
+ NotFrozen: notFrozen,
+ RequireFrozenReason: requireFrozenReason,
+ Flags: i.flagsForAidlGenRule(version),
})
versionedRustName := fixRustName(i.versionedName(version))
diff --git a/build/aidl_test.go b/build/aidl_test.go
index b6f88ad..198cfb5 100644
--- a/build/aidl_test.go
+++ b/build/aidl_test.go
@@ -262,7 +262,7 @@
},
},
}`
- expectedError := `module "foo_interface": versions: must be set \(need to be frozen\) when "unstable" is false, PLATFORM_VERSION_CODENAME is REL, and "owner" property is missing.`
+ expectedError := `module "foo_interface": versions: must be set \(need to be frozen\) because`
testAidlError(t, expectedError, vintfWithoutVersionBp, setReleaseEnv())
testAidlError(t, expectedError, vintfWithoutVersionBp, setTestFreezeEnv())
@@ -291,9 +291,10 @@
"aidl_api/foo/1/.hash": nil,
})
- expectedError := `foo-V2-java is an unfrozen development version`
- testAidlError(t, expectedError, unstableVersionUsageInJavaBp, setReleaseEnv(), files)
- testAidlError(t, expectedError, unstableVersionUsageInJavaBp, setTestFreezeEnv(), files)
+ expectedError1 := `foo-V2-java is an unfrozen development version, and it can't be used because "this is a release branch - freeze it or set 'owners:'"`
+ testAidlError(t, expectedError1, unstableVersionUsageInJavaBp, setReleaseEnv(), files)
+ expectedError2 := `foo-V2-java is an unfrozen development version, and it can't be used because "this is a release branch \(simulated by setting AIDL_FROZEN_REL\) - freeze it or set 'owners:'"`
+ testAidlError(t, expectedError2, unstableVersionUsageInJavaBp, setTestFreezeEnv(), files)
testAidl(t, unstableVersionUsageInJavaBp, files)
// A stable version can be used in release version
@@ -486,7 +487,7 @@
"aidl_api/xxx/1/.hash": nil,
})
- expectedError := `versions: must be set \(need to be frozen\) when`
+ expectedError := `versions: must be set \(need to be frozen\) because`
testAidlError(t, expectedError, frozenTest, files, setReleaseEnv())
testAidlError(t, expectedError, frozenTest, files, setTestFreezeEnv())
@@ -685,7 +686,7 @@
libs: ["foo-V1-java"],
}`
- expectedError := `"foo_interface": versions: must be set \(need to be frozen\) when "unstable" is false, PLATFORM_VERSION_CODENAME is REL, and "owner" property is missing.`
+ expectedError := `"foo_interface": versions: must be set \(need to be frozen\) because`
testAidlError(t, expectedError, nonVersionedModuleUsageInJavaBp, setReleaseEnv())
testAidlError(t, expectedError, nonVersionedModuleUsageInJavaBp, setTestFreezeEnv())
testAidl(t, nonVersionedModuleUsageInJavaBp)
@@ -724,7 +725,7 @@
libs: ["foo-V1-java"],
}`
- expectedError := `"foo_interface": versions: must be set \(need to be frozen\) when "unstable" is false, PLATFORM_VERSION_CODENAME is REL, and "owner" property is missing.`
+ expectedError := `"foo_interface": versions: must be set \(need to be frozen\) because`
testAidl(t, nonVersionedModuleUsageInJavaBp, setReleaseEnv())
testAidlError(t, expectedError, nonVersionedModuleUsageInJavaBp, setTestFreezeEnv())
testAidl(t, nonVersionedModuleUsageInJavaBp)
