diff --git a/android/visibility.go b/android/visibility.go index 38f80afa5..51d561120 100644 --- a/android/visibility.go +++ b/android/visibility.go @@ -17,6 +17,7 @@ package android import ( "fmt" "regexp" + "sort" "strings" "sync" @@ -496,13 +497,63 @@ func packageDefaultVisibility(config Config, moduleId qualifiedModuleName) compo } } +type VisibilityRuleSet interface { + // Widen the visibility with some extra rules. + Widen(extra []string) error + + Strings() []string +} + +type visibilityRuleSet struct { + rules []string +} + +var _ VisibilityRuleSet = (*visibilityRuleSet)(nil) + +func (v *visibilityRuleSet) Widen(extra []string) error { + // Check the extra rules first just in case they are invalid. Otherwise, if + // the current visibility is public then the extra rules will just be ignored. + if len(extra) == 1 { + singularRule := extra[0] + switch singularRule { + case "//visibility:public": + // Public overrides everything so just discard any existing rules. + v.rules = extra + return nil + case "//visibility:private": + // Extending rule with private is an error. + return fmt.Errorf("%q does not widen the visibility", singularRule) + } + } + + if len(v.rules) == 1 { + switch v.rules[0] { + case "//visibility:public": + // No point in adding rules to something which is already public. + return nil + case "//visibility:private": + // Adding any rules to private means it is no longer private so the + // private can be discarded. + v.rules = nil + } + } + + v.rules = FirstUniqueStrings(append(v.rules, extra...)) + sort.Strings(v.rules) + return nil +} + +func (v *visibilityRuleSet) Strings() []string { + return v.rules +} + // Get the effective visibility rules, i.e. the actual rules that affect the visibility of the // property irrespective of where they are defined. // // Includes visibility rules specified by package default_visibility and/or on defaults. // Short hand forms, e.g. //:__subpackages__ are replaced with their full form, e.g. // //package/containing/rule:__subpackages__. -func EffectiveVisibilityRules(ctx BaseModuleContext, module Module) []string { +func EffectiveVisibilityRules(ctx BaseModuleContext, module Module) VisibilityRuleSet { moduleName := ctx.OtherModuleName(module) dir := ctx.OtherModuleDir(module) qualified := qualifiedModuleName{dir, moduleName} @@ -527,7 +578,7 @@ func EffectiveVisibilityRules(ctx BaseModuleContext, module Module) []string { rule = append(rule, packageRule{dir}) } - return rule.Strings() + return &visibilityRuleSet{rule.Strings()} } // Clear the default visibility properties so they can be replaced. diff --git a/android/visibility_test.go b/android/visibility_test.go index ca0934557..9d9e57438 100644 --- a/android/visibility_test.go +++ b/android/visibility_test.go @@ -1270,3 +1270,49 @@ func newMockParentFactory() Module { }) return m } + +func testVisibilityRuleSet(t *testing.T, rules, extra, expected []string) { + t.Helper() + set := &visibilityRuleSet{rules} + err := set.Widen(extra) + if err != nil { + t.Error(err) + return + } + actual := set.Strings() + if !reflect.DeepEqual(actual, expected) { + t.Errorf("mismatching rules after extend: expected %#v, actual %#v", expected, actual) + } +} + +func TestVisibilityRuleSet(t *testing.T) { + t.Run("extend empty", func(t *testing.T) { + testVisibilityRuleSet(t, nil, []string{"//foo"}, []string{"//foo"}) + }) + t.Run("extend", func(t *testing.T) { + testVisibilityRuleSet(t, []string{"//foo"}, []string{"//bar"}, []string{"//bar", "//foo"}) + }) + t.Run("extend duplicate", func(t *testing.T) { + testVisibilityRuleSet(t, []string{"//foo"}, []string{"//bar", "//foo"}, []string{"//bar", "//foo"}) + }) + t.Run("extend public", func(t *testing.T) { + testVisibilityRuleSet(t, []string{"//visibility:public"}, []string{"//foo"}, []string{"//visibility:public"}) + }) + t.Run("extend private", func(t *testing.T) { + testVisibilityRuleSet(t, []string{"//visibility:private"}, []string{"//foo"}, []string{"//foo"}) + }) + t.Run("extend with public", func(t *testing.T) { + testVisibilityRuleSet(t, []string{"//foo"}, []string{"//visibility:public"}, []string{"//visibility:public"}) + }) + t.Run("extend with private", func(t *testing.T) { + t.Helper() + set := &visibilityRuleSet{[]string{"//foo"}} + err := set.Widen([]string{"//visibility:private"}) + expectedError := `"//visibility:private" does not widen the visibility` + if err == nil { + t.Errorf("missing error") + } else if err.Error() != expectedError { + t.Errorf("expected error %q found error %q", expectedError, err) + } + }) +} diff --git a/sdk/sdk.go b/sdk/sdk.go index 759102028..50b0886d4 100644 --- a/sdk/sdk.go +++ b/sdk/sdk.go @@ -75,6 +75,20 @@ type sdkProperties struct { // True if this is a module_exports (or module_exports_snapshot) module type. Module_exports bool `blueprint:"mutated"` + + // The additional visibility to add to the prebuilt modules to allow them to + // reference each other. + // + // This can only be used to widen the visibility of the members: + // + // * Specifying //visibility:public here will make all members visible and + // essentially ignore their own visibility. + // * Specifying //visibility:private here is an error. + // * Specifying any other rule here will add it to the members visibility and + // be output to the member prebuilt in the snapshot. Duplicates will be + // dropped. Adding a rule to members that have //visibility:private will + // cause the //visibility:private to be discarded. + Prebuilt_visibility []string } // Contains information about the sdk properties that list sdk members, e.g. @@ -211,6 +225,9 @@ func newSdkModule(moduleExports bool) *sdk { // properties for the member type specific list properties. s.dynamicMemberTypeListProperties = s.dynamicSdkMemberTypes.createMemberListProperties() s.AddProperties(&s.properties, s.dynamicMemberTypeListProperties) + + // Make sure that the prebuilt visibility property is verified for errors. + android.AddVisibilityProperty(s, "prebuilt_visibility", &s.properties.Prebuilt_visibility) android.InitCommonOSAndroidMultiTargetsArchModule(s, android.HostAndDeviceSupported, android.MultilibCommon) android.InitDefaultableModule(s) android.AddLoadHook(s, func(ctx android.LoadHookContext) { diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index ef62b79ba..2e6c62a9b 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -108,6 +108,9 @@ func TestSnapshotVisibility(t *testing.T) { // generated sdk_snapshot. ":__subpackages__", ], + prebuilt_visibility: [ + "//prebuilts/mysdk", + ], java_header_libs: [ "myjavalib", "mypublicjavalib", @@ -176,6 +179,7 @@ java_import { visibility: [ "//other/foo", "//package", + "//prebuilts/mysdk", ], jars: ["java/myjavalib.jar"], } @@ -186,6 +190,7 @@ java_import { visibility: [ "//other/foo", "//package", + "//prebuilts/mysdk", ], jars: ["java/myjavalib.jar"], } @@ -210,6 +215,7 @@ java_import { visibility: [ "//other/bar", "//package", + "//prebuilts/mysdk", ], jars: ["java/mydefaultedjavalib.jar"], } @@ -220,6 +226,7 @@ java_import { visibility: [ "//other/bar", "//package", + "//prebuilts/mysdk", ], jars: ["java/mydefaultedjavalib.jar"], } @@ -227,14 +234,20 @@ java_import { java_import { name: "mysdk_myprivatejavalib@current", sdk_member_name: "myprivatejavalib", - visibility: ["//package"], + visibility: [ + "//package", + "//prebuilts/mysdk", + ], jars: ["java/myprivatejavalib.jar"], } java_import { name: "myprivatejavalib", prefer: false, - visibility: ["//package"], + visibility: [ + "//package", + "//prebuilts/mysdk", + ], jars: ["java/myprivatejavalib.jar"], } @@ -254,6 +267,40 @@ sdk_snapshot { `)) } +func TestPrebuiltVisibilityProperty_IsValidated(t *testing.T) { + testSdkError(t, `prebuilt_visibility: cannot mix "//visibility:private" with any other visibility rules`, ` + sdk { + name: "mysdk", + prebuilt_visibility: [ + "//foo", + "//visibility:private", + ], + } +`) +} + +func TestPrebuiltVisibilityProperty_AddPrivate(t *testing.T) { + testSdkError(t, `prebuilt_visibility: "//visibility:private" does not widen the visibility`, ` + sdk { + name: "mysdk", + prebuilt_visibility: [ + "//visibility:private", + ], + java_header_libs: [ + "myjavalib", + ], + } + + java_library { + name: "myjavalib", + // Uses package default visibility + srcs: ["Test.java"], + system_modules: "none", + sdk_version: "none", + } +`) +} + func TestSDkInstall(t *testing.T) { sdk := ` sdk { diff --git a/sdk/update.go b/sdk/update.go index a10e8523c..f29b5a00c 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -300,7 +300,7 @@ func (s *sdk) buildSnapshot(ctx android.ModuleContext, sdkVariants []*sdk) andro snapshotModule.AddProperty("name", snapshotName) // Make sure that the snapshot has the same visibility as the sdk. - visibility := android.EffectiveVisibilityRules(ctx, s) + visibility := android.EffectiveVisibilityRules(ctx, s).Strings() if len(visibility) != 0 { snapshotModule.AddProperty("visibility", visibility) } @@ -719,7 +719,15 @@ func (s *snapshotBuilder) AddPrebuiltModule(member android.SdkMember, moduleType } else { // Extract visibility information from a member variant. All variants have the same // visibility so it doesn't matter which one is used. - visibility := android.EffectiveVisibilityRules(s.ctx, variant) + visibilityRules := android.EffectiveVisibilityRules(s.ctx, variant) + + // Add any additional visibility rules needed for the prebuilts to reference each other. + err := visibilityRules.Widen(s.sdk.properties.Prebuilt_visibility) + if err != nil { + s.ctx.PropertyErrorf("prebuilt_visibility", "%s", err) + } + + visibility := visibilityRules.Strings() if len(visibility) != 0 { m.AddProperty("visibility", visibility) }