Add more specific partition visibility rules

//visibility:any_system_partition, //visibility:any_vendor_partition,
etc.

Then, if a partition visibility rule is not specificed, but the module
is installed on a non-system partition via the `vendor: true` or other
properties, the visibility rule for that partition will be added by
default.

This is so that "any_partition" doesn't imply that modules could be put
on the vendor partition when they weren't designed for that, and so that
modules that do need to go on the vendor partition don't need to specify
both vendor: true and visibility:any_vendor_partition.

Eventually, the partition properties should be deprecated, and replaced
with just these visibility rules.

Bug: 321000103
Test: go tests
Change-Id: I24dba36bbc20921941f892480bf7c050e93827c6

android/singleton.go

@@ -251,7 +251,7 @@ func (s *singletonContextAdaptor) FinalModule(module Module) Module {
 
 func (s *singletonContextAdaptor) ModuleVariantsFromName(referer Module, name string) []Module {
 	// get module reference for visibility enforcement
-	qualified := createVisibilityModuleReference(s.ModuleName(referer), s.ModuleDir(referer), s.ModuleType(referer))
+	qualified := createVisibilityModuleReference(s.ModuleName(referer), s.ModuleDir(referer), referer)
 
 	modules := s.SingletonContext.ModuleVariantsFromName(referer, name)
 	result := make([]Module, 0, len(modules))

android/visibility.go

@@ -59,18 +59,13 @@ var visibilityRuleRegexp = regexp.MustCompile(visibilityRulePattern)
 
 type visibilityModuleReference struct {
 	name   qualifiedModuleName
-	isPartitionModule bool
+	module Module
 }
 
-func createVisibilityModuleReference(name, dir, typ string) visibilityModuleReference {
+func createVisibilityModuleReference(name, dir string, module Module) visibilityModuleReference {
-	isPartitionModule := false
-	switch typ {
-	case "android_filesystem", "android_system_image":
-		isPartitionModule = true
-	}
 	return visibilityModuleReference{
 		name:   createQualifiedModuleName(name, dir),
-		isPartitionModule: isPartitionModule,
+		module: module,
 	}
 }
 
@@ -214,21 +209,37 @@ func (r privateRule) String() string {
 	return "//visibility:private"
 }
 
+var anyPartitionRegex = regexp.MustCompile("^any_(system|system_ext|vendor|product|data|odm)_partition$")
+
 // visibilityRule for //visibility:any_partition
-type anyPartitionRule struct{}
+type anyPartitionRule struct {
+	partitionType string
+}
 
 var _ visibilityRule = anyPartitionRule{}
 
+type PartitionTypeInterface interface {
+	PartitionType() string
+}
+
 func (r anyPartitionRule) matches(m visibilityModuleReference) bool {
-	return m.isPartitionModule
+	if m2, ok := m.module.(PartitionTypeInterface); ok {
+		return m2.PartitionType() == r.partitionType
+	}
+	return false
 }
 
 func (r anyPartitionRule) String() string {
-	return "//visibility:any_partition"
+	return "//visibility:any_" + r.partitionType + "_partition"
 }
 
 var visibilityRuleMap = NewOnceKey("visibilityRuleMap")
 
+type visibilityRulesForModule struct {
+	rule                   compositeRule
+	implicitPartitionRules compositeRule
+}
+
 // The map from qualifiedModuleName to visibilityRule.
 func moduleToVisibilityRuleMap(config Config) *sync.Map {
 	return config.Once(visibilityRuleMap, func() interface{} {
@@ -304,9 +315,6 @@ func checkRules(ctx BaseModuleContext, currentPkg, property string, visibility [
 		if pkg == "visibility" {
 			switch name {
 			case "private", "public":
-			case "any_partition":
-				// any_partition can be used with another visibility fields
-				continue
 			case "legacy_public":
 				ctx.PropertyErrorf(property, "//visibility:legacy_public must not be used")
 				continue
@@ -314,6 +322,10 @@ func checkRules(ctx BaseModuleContext, currentPkg, property string, visibility [
 				// This keyword does not create a rule so pretend it does not exist.
 				ruleCount -= 1
 			default:
+				if anyPartitionRegex.MatchString(name) {
+					// any_*_partition can be used with another visibility fields
+					continue
+				}
 				ctx.PropertyErrorf(property, "unrecognized visibility rule %q", v)
 				continue
 			}
@@ -352,14 +364,19 @@ func visibilityRuleGatherer(ctx BottomUpMutatorContext) {
 
 	// Parse the visibility rules that control access to the module and store them by id
 	// for use when enforcing the rules.
+	var rule compositeRule
 	primaryProperty := m.base().primaryVisibilityProperty
 	if primaryProperty != nil {
 		if visibility := primaryProperty.getStrings(); visibility != nil {
-			rule := parseRules(ctx, currentPkg, primaryProperty.getName(), visibility)
+			rule = parseRules(ctx, currentPkg, primaryProperty.getName(), visibility)
-			if rule != nil {
-				moduleToVisibilityRuleMap(ctx.Config()).Store(qualifiedModuleId, rule)
 		}
 	}
+	ipr := implicitPartitionRules(ctx)
+	if rule != nil || ipr != nil {
+		moduleToVisibilityRuleMap(ctx.Config()).Store(qualifiedModuleId, visibilityRulesForModule{
+			rule:                   rule,
+			implicitPartitionRules: ipr,
+		})
 	}
 }
 
@@ -392,8 +409,13 @@ func parseRules(ctx BaseModuleContext, currentPkg, property string, visibility [
 				hasNonPrivateRule = false
 				// This does not actually create a rule so continue onto the next rule.
 				continue
-			case "any_partition":
+			default:
-				r = anyPartitionRule{}
+				match := anyPartitionRegex.FindStringSubmatch(name)
+				if match != nil {
+					r = anyPartitionRule{
+						partitionType: match[1],
+					}
+				}
 			}
 		} else {
 			switch name {
@@ -432,6 +454,22 @@ func parseRules(ctx BaseModuleContext, currentPkg, property string, visibility [
 	return rules
 }
 
+func implicitPartitionRules(ctx BaseModuleContext) compositeRule {
+	var result compositeRule
+	if ctx.SocSpecific() {
+		result = append(result, anyPartitionRule{partitionType: "vendor"})
+	} else if ctx.ProductSpecific() {
+		result = append(result, anyPartitionRule{partitionType: "product"})
+	} else if ctx.Module().InstallInData() {
+		result = append(result, anyPartitionRule{partitionType: "data"})
+	} else if ctx.SystemExtSpecific() {
+		result = append(result, anyPartitionRule{partitionType: "system_ext"})
+	} else if ctx.DeviceSpecific() {
+		result = append(result, anyPartitionRule{partitionType: "odm"})
+	}
+	return result
+}
+
 func isAllowedFromOutsideVendor(pkg string, name string) bool {
 	if pkg == "vendor" {
 		return name == "__subpackages__"
@@ -470,7 +508,7 @@ func splitRule(ctx BaseModuleContext, ruleExpression string, currentPkg, propert
 }
 
 func visibilityRuleEnforcer(ctx TopDownMutatorContext) {
-	qualified := createVisibilityModuleReference(ctx.ModuleName(), ctx.ModuleDir(), ctx.ModuleType())
+	qualified := createVisibilityModuleReference(ctx.ModuleName(), ctx.ModuleDir(), ctx.Module())
 
 	// Visit all the dependencies making sure that this module has access to them all.
 	ctx.VisitDirectDeps(func(dep Module) {
@@ -505,10 +543,13 @@ var defaultVisibility = compositeRule{publicRule{}}
 // which is currently //visibility:public.
 func effectiveVisibilityRules(config Config, qualified qualifiedModuleName) compositeRule {
 	moduleToVisibilityRule := moduleToVisibilityRuleMap(config)
-	value, ok := moduleToVisibilityRule.Load(qualified)
+	value := visibilityRulesForModule{}
+	if valueRaw, ok := moduleToVisibilityRule.Load(qualified); ok {
+		value = valueRaw.(visibilityRulesForModule)
+	}
 	var rule compositeRule
-	if ok {
+	if value.rule != nil {
-		rule = value.(compositeRule)
+		rule = value.rule
 	} else {
 		rule = packageDefaultVisibility(moduleToVisibilityRule, qualified)
 	}
@@ -518,6 +559,20 @@ func effectiveVisibilityRules(config Config, qualified qualifiedModuleName) comp
 	if rule == nil {
 		rule = defaultVisibility
 	}
+
+	// If a partition rule wasn't specified, add implicit partition visibility
+	// rules based on the partition properties like vendor: true.
+	foundPartitionRule := false
+	for _, r := range rule {
+		if _, ok := r.(anyPartitionRule); ok {
+			foundPartitionRule = true
+			break
+		}
+	}
+	if !foundPartitionRule {
+		rule = append(rule, value.implicitPartitionRules...)
+	}
+
 	return rule
 }
 
@@ -531,7 +586,7 @@ func packageDefaultVisibility(moduleToVisibilityRule *sync.Map, moduleId qualifi
 	for {
 		value, ok := moduleToVisibilityRule.Load(packageQualifiedId)
 		if ok {
-			return value.(compositeRule)
+			return value.(visibilityRulesForModule).rule
 		}
 
 		if packageQualifiedId.isRootPackage() {
@@ -605,7 +660,7 @@ func EffectiveVisibilityRules(ctx BaseModuleContext, module Module) VisibilityRu
 
 	rule := effectiveVisibilityRules(ctx.Config(), qualified)
 
-	currentModule := createVisibilityModuleReference(moduleName, dir, ctx.OtherModuleType(module))
+	currentModule := createVisibilityModuleReference(moduleName, dir, module)
 
 	// Modules are implicitly visible to other modules in the same package,
 	// without checking the visibility rules. Here we need to add that visibility

android/visibility_test.go

@@ -1905,7 +1905,7 @@ var visibilityTests = []struct {
 		},
 	},
 	{
-		name: "any_partition visibility works",
+		name: "any_system_partition visibility works",
 		fs: MockFS{
 			"top/Android.bp": []byte(`
 				android_filesystem {
@@ -1916,12 +1916,12 @@ var visibilityTests = []struct {
 				package(default_visibility=["//visibility:private"])
 				mock_library {
 					name: "bar",
-					visibility: ["//visibility:any_partition"],
+					visibility: ["//visibility:any_system_partition"],
 				}`),
 		},
 	},
 	{
-		name: "any_partition visibility works with the other visibility",
+		name: "any_system_partition visibility works with the other visibility",
 		fs: MockFS{
 			"top/Android.bp": []byte(`
 				android_filesystem {
@@ -1935,13 +1935,13 @@ var visibilityTests = []struct {
 					name: "bar",
 					visibility: [
 						"//top2",
-						"//visibility:any_partition"
+						"//visibility:any_system_partition"
 					],
 				}`),
 		},
 	},
 	{
-		name: "any_partition visibility doesn't work for non-partitions",
+		name: "any_system_partition visibility doesn't work for non-partitions",
 		fs: MockFS{
 			"top/Android.bp": []byte(`
 				mock_library {
@@ -1951,11 +1951,77 @@ var visibilityTests = []struct {
 			"top/nested/Android.bp": []byte(`
 				mock_library {
 					name: "bar",
-					visibility: ["//visibility:any_partition"],
+					visibility: ["//visibility:any_system_partition"],
 				}`),
 		},
 		expectedErrors: []string{`module "foo" variant "android_common": depends on //top/nested:bar which is not visible to this module`},
 	},
+	{
+		name: "any_system_partition visibility doesn't work for vendor partitions",
+		fs: MockFS{
+			"top/Android.bp": []byte(`
+				android_filesystem {
+					name: "foo",
+					partition_type: "vendor",
+					deps: ["bar"],
+				}`),
+			"top/nested/Android.bp": []byte(`
+				package(default_visibility=["//visibility:private"])
+				mock_library {
+					name: "bar",
+					visibility: ["//visibility:any_system_partition"],
+				}`),
+		},
+		expectedErrors: []string{`module "foo" variant "android_common": depends on //top/nested:bar which is not visible to this module`},
+	},
+	{
+		name: "Vendor modules are visible to any vendor partition by default",
+		fs: MockFS{
+			"top/Android.bp": []byte(`
+				android_filesystem {
+					name: "foo",
+					partition_type: "vendor",
+					deps: ["bar"],
+				}`),
+			"top/nested/Android.bp": []byte(`
+				package(default_visibility=["//visibility:private"])
+				mock_library {
+					name: "bar",
+					vendor: true,
+				}`),
+		},
+	},
+	{
+		name: "Not visible to vendor partitions when using any_system_partiton, even if vendor: true",
+		fs: MockFS{
+			"top/Android.bp": []byte(`
+				android_filesystem {
+					name: "foo",
+					partition_type: "vendor",
+					deps: ["bar"],
+				}`),
+			"top/nested/Android.bp": []byte(`
+				package(default_visibility=["//visibility:private"])
+				mock_library {
+					name: "bar",
+					vendor: true,
+					visibility: ["//visibility:any_system_partition"],
+				}`),
+		},
+		expectedErrors: []string{`module "foo" variant "android_common": depends on //top/nested:bar which is not visible to this module`},
+	},
+	{
+		name: "unknown any_partition specs throw errors",
+		fs: MockFS{
+			"top/nested/Android.bp": []byte(`
+				package(default_visibility=["//visibility:private"])
+				mock_library {
+					name: "bar",
+					visibility: ["//visibility:any_unknown_partition"],
+				}`),
+		},
+		expectedErrors: []string{`unrecognized visibility rule "//visibility:any_unknown_partition"`},
+	},
 }
 
 func TestVisibility(t *testing.T) {
@@ -1977,8 +2043,7 @@ func TestVisibility(t *testing.T) {
 					ctx.RegisterModuleType("mock_library", newMockLibraryModule)
 					ctx.RegisterModuleType("mock_parent", newMockParentFactory)
 					ctx.RegisterModuleType("mock_defaults", defaultsFactory)
-					// For testing //visibility:any_partition. The module type doesn't matter, just that it's registered under the name "android_filesystem"
+					ctx.RegisterModuleType("android_filesystem", newMockFilesystemModule)
-					ctx.RegisterModuleType("android_filesystem", newMockLibraryModule)
 				}),
 				prepareForTestWithFakePrebuiltModules,
 				// Add additional files to the mock filesystem
@@ -2032,6 +2097,37 @@ func (j *mockLibraryModule) DepsMutator(ctx BottomUpMutatorContext) {
 func (p *mockLibraryModule) GenerateAndroidBuildActions(ModuleContext) {
 }
 
+type mockFilesystemModuleProperties struct {
+	Partition_type *string
+	Deps           []string
+}
+
+type mockFilesystemModule struct {
+	ModuleBase
+	properties mockFilesystemModuleProperties
+}
+
+func (j *mockFilesystemModule) DepsMutator(ctx BottomUpMutatorContext) {
+	ctx.AddVariationDependencies(nil, dependencyTag{name: "mockdeps"}, j.properties.Deps...)
+}
+
+func (p *mockFilesystemModule) GenerateAndroidBuildActions(ModuleContext) {
+}
+
+func (p *mockFilesystemModule) PartitionType() string {
+	if p.properties.Partition_type == nil {
+		return "system"
+	}
+	return *p.properties.Partition_type
+}
+
+func newMockFilesystemModule() Module {
+	m := &mockFilesystemModule{}
+	m.AddProperties(&m.properties)
+	InitAndroidArchModule(m, DeviceSupported, MultilibCommon)
+	return m
+}
+
 type mockDefaults struct {
 	ModuleBase
 	DefaultsModuleBase

filesystem/filesystem.go

@@ -88,6 +88,10 @@ type filesystemProperties struct {
 	// is ext4.
 	Type *string
 
+	// Identifies which partition this is for //visibility:any_system_image (and others) visibility
+	// checks, and will be used in the future for API surface checks.
+	Partition_type *string
+
 	// file_contexts file to make image. Currently, only ext4 is supported.
 	File_contexts *string `android:"path"`
 
@@ -175,6 +179,9 @@ func (f *filesystem) installFileName() string {
 var pctx = android.NewPackageContext("android/soong/filesystem")
 
 func (f *filesystem) GenerateAndroidBuildActions(ctx android.ModuleContext) {
+	if !android.InList(f.PartitionType(), validPartitions) {
+		ctx.PropertyErrorf("partition_type", "partition_type must be one of %s, found: %s", validPartitions, f.PartitionType())
+	}
 	switch f.fsType(ctx) {
 	case ext4Type:
 		f.output = f.buildImageUsingBuildImage(ctx)
@@ -441,6 +448,10 @@ func (f *filesystem) addMakeBuiltFiles(ctx android.ModuleContext, builder *andro
 		Text(android.PathForArbitraryOutput(ctx, stagingDir).String())
 }
 
+func (f *filesystem) PartitionType() string {
+	return proptools.StringDefault(f.properties.Partition_type, "system")
+}
+
 var _ android.AndroidMkEntriesProvider = (*filesystem)(nil)
 
 // Implements android.AndroidMkEntriesProvider

filesystem/system_image.go

@@ -43,6 +43,9 @@ func systemImageFactory() android.Module {
 }
 
 func (s *systemImage) buildExtraFiles(ctx android.ModuleContext, root android.OutputPath) android.OutputPaths {
+	if s.filesystem.properties.Partition_type != nil {
+		ctx.PropertyErrorf("partition_type", "partition_type must be unset on an android_system_image module. It is assumed to be 'system'.")
+	}
 	lc := s.buildLinkerConfigFile(ctx, root)
 	// Add more files if needed
 	return []android.OutputPath{lc}