Merge "Dont write data attrs for cc lib rules" into main am: f7fe400fbc am: 1140168066 am: 100e5a256b

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2773605

Change-Id: Ibe08bb69abd443fe479da6afedb48baf88093559
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

android/module.go

@@ -1272,6 +1272,22 @@ func InitCommonOSAndroidMultiTargetsArchModule(m Module, hod HostOrDeviceSupport
 	m.base().commonProperties.CreateCommonOSVariant = true
 }
 
+func (attrs *CommonAttributes) getRequiredWithoutCycles(ctx *bottomUpMutatorContext, props *commonProperties) []string {
+	// Treat `required` as if it's empty if data should be skipped for this target,
+	// as `required` is only used for the `data` attribute at this time, and we want
+	// to avoid lookups of labels that won't actually be dependencies of this target.
+	// TODO: b/202299295 - Refactor this to use `required` dependencies, once they
+	// are handled other than passing to `data`.
+	if proptools.Bool(attrs.SkipData) {
+		return []string{}
+	}
+	// The required property can contain the module itself. This causes a cycle
+	// when generated as the 'data' label list attribute in Bazel. Remove it if
+	// it exists. See b/247985196.
+	_, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), props.Required)
+	return FirstUniqueStrings(requiredWithoutCycles)
+}
+
 func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *bottomUpMutatorContext,
 	enabledPropertyOverrides bazel.BoolAttribute) constraintAttributes {
 
@@ -1340,18 +1356,13 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *bottomUpMutato
 
 	attrs.Applicable_licenses = bazel.MakeLabelListAttribute(BazelLabelForModuleDeps(ctx, mod.commonProperties.Licenses))
 
-	// The required property can contain the module itself. This causes a cycle
-	// when generated as the 'data' label list attribute in Bazel. Remove it if
-	// it exists. See b/247985196.
-	_, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), mod.commonProperties.Required)
-	requiredWithoutCycles = FirstUniqueStrings(requiredWithoutCycles)
+	requiredWithoutCycles := attrs.getRequiredWithoutCycles(ctx, &mod.commonProperties)
 	required := depsToLabelList(requiredWithoutCycles)
 	archVariantProps := mod.GetArchVariantProperties(ctx, &commonProperties{})
 	for axis, configToProps := range archVariantProps {
 		for config, _props := range configToProps {
 			if archProps, ok := _props.(*commonProperties); ok {
-				_, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), archProps.Required)
-				requiredWithoutCycles = FirstUniqueStrings(requiredWithoutCycles)
+				requiredWithoutCycles := attrs.getRequiredWithoutCycles(ctx, archProps)
 				required.SetSelectValue(axis, config, depsToLabelList(requiredWithoutCycles).Value)
 				if !neitherHostNorDevice {
 					if archProps.Enabled != nil {
@@ -1408,9 +1419,8 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *bottomUpMutato
 		platformEnabledAttribute.Add(&l)
 	}
 
-	if !proptools.Bool(attrs.SkipData) {
-		attrs.Data.Append(required)
-	}
+	attrs.Data.Append(required)
+
 	// SkipData is not an attribute of any Bazel target
 	// Set this to nil so that it does not appear in the generated build file
 	attrs.SkipData = nil

bp2build/cc_library_conversion_test.go

@@ -5251,3 +5251,57 @@ versioned_ndk_headers {
 	}
 	runCcLibraryTestCase(t, tc)
 }
+
+// Regression test for b/303307456.
+// TODO: b/202299295 - Remove this test when cc rules have proper support
+// for the `required` property
+func TestCcModules_requiredProperty(t *testing.T) {
+	runCcLibrarySharedTestCase(t, Bp2buildTestCase{
+		Description: "cc modules do not use the required property",
+		Filesystem: map[string]string{
+			"foo.c": "",
+			"bar.c": "",
+		},
+		Blueprint: soongCcLibraryPreamble + `
+cc_library {
+    name: "foo_both",
+    srcs: ["foo.c"],
+    include_build_directory: false,
+    required: ["bar"],
+}
+cc_library_shared {
+    name: "foo_shared",
+    srcs: ["foo.c"],
+    include_build_directory: false,
+    required: ["bar"],
+}
+cc_library_static {
+    name: "foo_static",
+    srcs: ["foo.c"],
+    include_build_directory: false,
+    required: ["bar"],
+}
+cc_library_static {
+    name: "bar",
+    srcs: ["bar.c"],
+    include_build_directory: false,
+}`,
+		ExpectedBazelTargets: []string{
+			MakeBazelTarget("cc_library_static", "foo_both_bp2build_cc_library_static", AttrNameToString{
+				"srcs_c": `["foo.c"]`,
+			}),
+			MakeBazelTarget("cc_library_shared", "foo_both", AttrNameToString{
+				"srcs_c": `["foo.c"]`,
+			}),
+			MakeBazelTarget("cc_library_shared", "foo_shared", AttrNameToString{
+				"srcs_c": `["foo.c"]`,
+			}),
+			MakeBazelTarget("cc_library_static", "foo_static", AttrNameToString{
+				"srcs_c": `["foo.c"]`,
+			}),
+			MakeBazelTarget("cc_library_static", "bar", AttrNameToString{
+				"srcs_c": `["bar.c"]`,
+			}),
+		},
+	})
+}

cc/library.go

@@ -468,12 +468,16 @@ func libraryBp2Build(ctx android.Bp2buildMutatorContext, m *Module) {
 		android.CommonAttributes{
 			Name: m.Name() + "_bp2build_cc_library_static",
 			Tags: tagsForStaticVariant,
+			// TODO: b/303307456 - Remove this when data is properly supported in cc rules.
+			SkipData: proptools.BoolPtr(true),
 		},
 		staticTargetAttrs, staticAttrs.Enabled)
 	ctx.CreateBazelTargetModuleWithRestrictions(sharedProps,
 		android.CommonAttributes{
 			Name: m.Name(),
 			Tags: tagsForSharedVariant,
+			// TODO: b/303307456 - Remove this when data is properly supported in cc rules.
+			SkipData: proptools.BoolPtr(true),
 		},
 		sharedTargetAttrs, sharedAttrs.Enabled)
 
@@ -496,8 +500,11 @@ func createStubsBazelTargetIfNeeded(ctx android.Bp2buildMutatorContext, m *Modul
 			Deps:                 baseAttributes.deps,
 			Api_surface:          proptools.StringPtr("module-libapi"),
 		}
-		ctx.CreateBazelTargetModule(stubSuitesProps,
-			android.CommonAttributes{Name: m.Name() + "_stub_libs"},
+		ctx.CreateBazelTargetModule(stubSuitesProps, android.CommonAttributes{
+			Name: m.Name() + "_stub_libs",
+			// TODO: b/303307456 - Remove this when data is properly supported in cc rules.
+			SkipData: proptools.BoolPtr(true),
+		},
 			stubSuitesAttrs)
 
 		// Add alias for the stub shared_library in @api_surfaces repository
@@ -2935,7 +2942,12 @@ func sharedOrStaticLibraryBp2Build(ctx android.Bp2buildMutatorContext, module *M
 
 	tags := android.ApexAvailableTagsWithoutTestApexes(ctx, module)
 
-	ctx.CreateBazelTargetModule(props, android.CommonAttributes{Name: module.Name(), Tags: tags}, attrs)
+	ctx.CreateBazelTargetModule(props, android.CommonAttributes{
+		Name: module.Name(),
+		Tags: tags,
+		// TODO: b/303307456 - Remove this when data is properly supported in cc rules.
+		SkipData: proptools.BoolPtr(true),
+	}, attrs)
 }
 
 type includesAttributes struct {