Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7123cc53 authored by Chris Parsons's avatar Chris Parsons Committed by Christopher Parsons
Browse files

Dont write data attrs for cc lib rules 

The data attributes for cc library rules is dropped by their starlark
macros, so serves no real purpose. Furthermore, this unused dependency
proves problematic for allowlist v2, as there are many cases at HEAD
where the corner-case "requires" dependency would otherwise mark a module
as unconvertible.

Fixes: 303307456
Test: New unit test
Test: Manual verification to unblock allowlist v2 `bp2build.sh` runs in
AOSP

Change-Id: I6ca6104b958d2b428fc2ca5b3fa794106571acca
parent b7a67206

android/module.go

+20 −10
Original line number Diff line number Diff line
@@ -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)

	}



	// 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

+54 −0
Original line number Diff line number Diff line
@@ -5245,3 +5245,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

+15 −3
Original line number Diff line number Diff line
@@ -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 {