From a5ecb37d64deb6d84d1407f561b473bac13ac974 Mon Sep 17 00:00:00 2001
From: Jingwen Chen <jingwen@google.com>
Date: Wed, 21 Sep 2022 09:05:37 +0000
Subject: [PATCH] Remove apexer.required workaround for aapt2.

Add a filter for cyclic references in required module names in bp2build.

Test: CI
Bug: 247985196
Bug: 225291837
Fixes: 234748998
Change-Id: Id1cfd0b2e5f217cdb01218ec3bbe11d5070cc9df
---
 android/module.go                 | 19 +++++++------------
 bp2build/build_conversion_test.go | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/android/module.go b/android/module.go
index 5d520f407..16172591b 100644
--- a/android/module.go
+++ b/android/module.go
@@ -1231,22 +1231,17 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator
 		}
 	}
 
-	required := depsToLabelList(mod.commonProperties.Required)
+	// 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)
+	required := depsToLabelList(requiredWithoutCycles)
 	archVariantProps := mod.GetArchVariantProperties(ctx, &commonProperties{})
 	for axis, configToProps := range archVariantProps {
 		for config, _props := range configToProps {
 			if archProps, ok := _props.(*commonProperties); ok {
-				// TODO(b/234748998) Remove this requiredFiltered workaround when aapt2 converts successfully
-				requiredFiltered := archProps.Required
-				if attrs.Name == "apexer" {
-					requiredFiltered = make([]string, 0, len(archProps.Required))
-					for _, req := range archProps.Required {
-						if req != "aapt2" && req != "apexer" {
-							requiredFiltered = append(requiredFiltered, req)
-						}
-					}
-				}
-				required.SetSelectValue(axis, config, depsToLabelList(requiredFiltered).Value)
+				_, requiredWithoutCycles := RemoveFromList(ctx.ModuleName(), archProps.Required)
+				required.SetSelectValue(axis, config, depsToLabelList(requiredWithoutCycles).Value)
 				if !neitherHostNorDevice {
 					if archProps.Enabled != nil {
 						if axis != bazel.OsConfigurationAxis || osSupport[config] {
diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go
index d677c0f5b..9f4f7c1b3 100644
--- a/bp2build/build_conversion_test.go
+++ b/bp2build/build_conversion_test.go
@@ -1679,6 +1679,22 @@ filegroup {
     name: "fg_foo",
     required: ["reqd"],
     bazel_module: { bp2build_available: true },
+}`,
+			ExpectedBazelTargets: []string{
+				MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{
+					"data": `[":reqd"]`,
+				}),
+			},
+		},
+		{
+			Description:                "Required into data test, cyclic self reference is filtered out",
+			ModuleTypeUnderTest:        "filegroup",
+			ModuleTypeUnderTestFactory: android.FileGroupFactory,
+			Blueprint: simpleModuleDoNotConvertBp2build("filegroup", "reqd") + `
+filegroup {
+    name: "fg_foo",
+    required: ["reqd", "fg_foo"],
+    bazel_module: { bp2build_available: true },
 }`,
 			ExpectedBazelTargets: []string{
 				MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{