Merge changes I6df9a14d,I830c53ef

* changes:
  bp2build: allowlist //external/libcap/...
  bp2build: don't generate filegroups (or error) if it contains a file with the same name.

android/bazel.go

@@ -182,6 +182,7 @@ var (
 		"external/jemalloc_new":           Bp2BuildDefaultTrueRecursively,
 		"external/libcxx":                 Bp2BuildDefaultTrueRecursively,
 		"external/libcxxabi":              Bp2BuildDefaultTrueRecursively,
+		"external/libcap":                 Bp2BuildDefaultTrueRecursively,
 		"external/scudo":                  Bp2BuildDefaultTrueRecursively,
 		"prebuilts/clang/host/linux-x86":  Bp2BuildDefaultTrueRecursively,
 	}
@@ -231,6 +232,10 @@ var (
 		//external/brotli/...
 		"brotli-fuzzer-corpus", // "declared output 'external/brotli/c/fuzz/73231c6592f195ffd41100b8706d1138ff6893b9' was not created by genrule"
 
+		// //external/libcap/...
+		"libcap",      // http://b/198595332, depends on _makenames, a cc_binary
+		"cap_names.h", // http://b/198596102, depends on _makenames, a cc_binary
+
 		// Tests. Handle later.
 		"libbionic_tests_headers_posix", // http://b/186024507, cc_library_static, sched.h, time.h not found
 		"libjemalloc5_integrationtest",

android/bazel_handler.go

@@ -27,9 +27,9 @@ import (
 	"sync"
 
 	"android/soong/bazel/cquery"
+	"android/soong/shared"
 
 	"android/soong/bazel"
-	"android/soong/shared"
 )
 
 type cqueryRequest interface {
@@ -490,6 +490,12 @@ config_node(name = "%s",
     arch = "%s",
     deps = [%s],
 )
+`
+
+	commonArchFilegroupString := `
+filegroup(name = "common",
+    srcs = [%s],
+)
 `
 
 	configNodesSection := ""
@@ -501,14 +507,22 @@ config_node(name = "%s",
 		labelsByArch[archString] = append(labelsByArch[archString], labelString)
 	}
 
-	configNodeLabels := []string{}
+	allLabels := []string{}
 	for archString, labels := range labelsByArch {
-		configNodeLabels = append(configNodeLabels, fmt.Sprintf("\":%s\"", archString))
+		if archString == "common" {
-		labelsString := strings.Join(labels, ",\n            ")
+			// arch-less labels (e.g. filegroups) don't need a config_node
-		configNodesSection += fmt.Sprintf(configNodeFormatString, archString, archString, labelsString)
+			allLabels = append(allLabels, "\":common\"")
+			labelsString := strings.Join(labels, ",\n            ")
+			configNodesSection += fmt.Sprintf(commonArchFilegroupString, labelsString)
+		} else {
+			// Create a config_node, and add the config_node's label to allLabels
+			allLabels = append(allLabels, fmt.Sprintf("\":%s\"", archString))
+			labelsString := strings.Join(labels, ",\n            ")
+			configNodesSection += fmt.Sprintf(configNodeFormatString, archString, archString, labelsString)
+		}
 	}
 
-	return []byte(fmt.Sprintf(formatString, configNodesSection, strings.Join(configNodeLabels, ",\n            ")))
+	return []byte(fmt.Sprintf(formatString, configNodesSection, strings.Join(allLabels, ",\n            ")))
 }
 
 func indent(original string) string {
@@ -573,6 +587,12 @@ def %s(target):
 %s
 
 def get_arch(target):
+  # TODO(b/199363072): filegroups and file targets aren't associated with any
+  # specific platform architecture in mixed builds. This is consistent with how
+  # Soong treats filegroups, but it may not be the case with manually-written
+  # filegroup BUILD targets.
+  if target.kind in ["filegroup", ""]:
+    return "common"
   buildoptions = build_options(target)
   platforms = build_options(target)["//command_line_option:platforms"]
   if len(platforms) != 1:
@@ -671,11 +691,12 @@ func (context *bazelContext) InvokeBazel() error {
 	if err != nil {
 		return err
 	}
+
 	buildrootLabel := "@soong_injection//mixed_builds:buildroot"
 	cqueryOutput, cqueryErr, err = context.issueBazelCommand(
 		context.paths,
 		bazel.CqueryBuildRootRunName,
-		bazelCommand{"cquery", fmt.Sprintf("kind(rule, deps(%s))", buildrootLabel)},
+		bazelCommand{"cquery", fmt.Sprintf("deps(%s)", buildrootLabel)},
 		"--output=starlark",
 		"--starlark:file="+absolutePath(cqueryFileRelpath))
 	err = ioutil.WriteFile(filepath.Join(soongInjectionPath, "cquery.out"),

android/bazel_handler_test.go

@@ -11,7 +11,7 @@ func TestRequestResultsAfterInvokeBazel(t *testing.T) {
 	label := "//foo:bar"
 	arch := Arm64
 	bazelContext, _ := testBazelContext(t, map[bazelCommand]string{
-		bazelCommand{command: "cquery", expression: "kind(rule, deps(@soong_injection//mixed_builds:buildroot))"}: `//foo:bar|arm64>>out/foo/bar.txt`,
+		bazelCommand{command: "cquery", expression: "deps(@soong_injection//mixed_builds:buildroot)"}: `//foo:bar|arm64>>out/foo/bar.txt`,
 	})
 	g, ok := bazelContext.GetOutputFiles(label, arch)
 	if ok {

android/filegroup.go

@@ -42,6 +42,27 @@ func FilegroupBp2Build(ctx TopDownMutatorContext) {
 
 	srcs := bazel.MakeLabelListAttribute(
 		BazelLabelForModuleSrcExcludes(ctx, fg.properties.Srcs, fg.properties.Exclude_srcs))
+
+	// For Bazel compatibility, don't generate the filegroup if there is only 1
+	// source file, and that the source file is named the same as the module
+	// itself. In Bazel, eponymous filegroups like this would be an error.
+	//
+	// Instead, dependents on this single-file filegroup can just depend
+	// on the file target, instead of rule target, directly.
+	//
+	// You may ask: what if a filegroup has multiple files, and one of them
+	// shares the name? The answer: we haven't seen that in the wild, and
+	// should lock Soong itself down to prevent the behavior. For now,
+	// we raise an error if bp2build sees this problem.
+	for _, f := range srcs.Value.Includes {
+		if f.Label == fg.Name() {
+			if len(srcs.Value.Includes) > 1 {
+				ctx.ModuleErrorf("filegroup '%s' cannot contain a file with the same name", fg.Name())
+			}
+			return
+		}
+	}
+
 	attrs := &bazelFilegroupAttributes{
 		Srcs: srcs,
 	}
@@ -97,7 +118,7 @@ func (fg *fileGroup) GenerateBazelBuildActions(ctx ModuleContext) bool {
 	}
 
 	bazelCtx := ctx.Config().BazelContext
-	filePaths, ok := bazelCtx.GetOutputFiles(fg.GetBazelLabel(ctx, fg), ctx.Arch().ArchType)
+	filePaths, ok := bazelCtx.GetOutputFiles(fg.GetBazelLabel(ctx, fg), Common)
 	if !ok {
 		return false
 	}

bp2build/Android.bp

@@ -39,6 +39,7 @@ bootstrap_go_package {
         "cc_library_static_conversion_test.go",
         "cc_object_conversion_test.go",
         "conversion_test.go",
+        "filegroup_conversion_test.go",
         "performance_test.go",
         "prebuilt_etc_conversion_test.go",
         "python_binary_conversion_test.go",

bp2build/build_conversion_test.go

@@ -218,13 +218,9 @@ func TestGenerateSoongModuleTargets(t *testing.T) {
 }
 
 func TestGenerateBazelTargetModules(t *testing.T) {
-	testCases := []struct {
+	testCases := []bp2buildTestCase{
-		name                 string
-		bp                   string
-		expectedBazelTargets []string
-	}{
 		{
-			bp: `custom {
+			blueprint: `custom {
 	name: "foo",
     string_list_prop: ["a", "b"],
     string_prop: "a",
@@ -241,7 +237,7 @@ func TestGenerateBazelTargetModules(t *testing.T) {
 			},
 		},
 		{
-			bp: `custom {
+			blueprint: `custom {
 	name: "control_characters",
     string_list_prop: ["\t", "\n"],
     string_prop: "a\t\n\r",
@@ -258,7 +254,7 @@ func TestGenerateBazelTargetModules(t *testing.T) {
 			},
 		},
 		{
-			bp: `custom {
+			blueprint: `custom {
   name: "has_dep",
   arch_paths: [":dep"],
   bazel_module: { bp2build_available: true },
@@ -280,7 +276,7 @@ custom {
 			},
 		},
 		{
-			bp: `custom {
+			blueprint: `custom {
     name: "arch_paths",
     arch: {
       x86: {
@@ -299,7 +295,7 @@ custom {
 			},
 		},
 		{
-			bp: `custom {
+			blueprint: `custom {
   name: "has_dep",
   arch: {
     x86: {
@@ -331,17 +327,17 @@ custom {
 
 	dir := "."
 	for _, testCase := range testCases {
-		config := android.TestConfig(buildDir, nil, testCase.bp, nil)
+		config := android.TestConfig(buildDir, nil, testCase.blueprint, nil)
 		ctx := android.NewTestContext(config)
 
 		registerCustomModuleForBp2buildConversion(ctx)
 
 		_, errs := ctx.ParseFileList(dir, []string{"Android.bp"})
-		if errored(t, "", errs) {
+		if errored(t, testCase, errs) {
 			continue
 		}
 		_, errs = ctx.ResolveDependencies(config)
-		if errored(t, "", errs) {
+		if errored(t, testCase, errs) {
 			continue
 		}
 
@@ -548,23 +544,13 @@ genrule {
 }`,
 	}
 
-	testCases := []struct {
+	testCases := []bp2buildTestCase{
-		description                        string
-		moduleTypeUnderTest                string
-		moduleTypeUnderTestFactory         android.ModuleFactory
-		moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext)
-		preArchMutators                    []android.RegisterMutatorFunc
-		bp                                 string
-		expectedBazelTargets               []string
-		fs                                 map[string]string
-		dir                                string
-	}{
 		{
 			description:                        "filegroup with does not specify srcs",
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "fg_foo",
     bazel_module: { bp2build_available: true },
 }`,
@@ -579,7 +565,7 @@ genrule {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "fg_foo",
     srcs: [],
     bazel_module: { bp2build_available: true },
@@ -595,7 +581,7 @@ genrule {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "fg_foo",
     srcs: ["a", "b"],
     bazel_module: { bp2build_available: true },
@@ -614,7 +600,7 @@ genrule {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "fg_foo",
     srcs: ["a", "b"],
     exclude_srcs: ["a"],
@@ -631,7 +617,7 @@ genrule {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "foo",
     srcs: ["**/*.txt"],
     bazel_module: { bp2build_available: true },
@@ -645,7 +631,7 @@ genrule {
     ],
 )`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/a.txt":        "",
 				"other/b.txt":        "",
 				"other/subdir/a.txt": "",
@@ -657,7 +643,7 @@ genrule {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "foo",
     srcs: ["a.txt"],
     bazel_module: { bp2build_available: true },
@@ -672,7 +658,7 @@ genrule {
     ],
 )`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/Android.bp": `filegroup {
     name: "fg_foo",
     srcs: ["**/*.txt"],
@@ -689,7 +675,7 @@ genrule {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "foobar",
     srcs: [
         ":foo",
@@ -705,7 +691,7 @@ genrule {
     ],
 )`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/Android.bp": `filegroup {
     name: "foo",
     srcs: ["a", "b"],
@@ -717,7 +703,7 @@ genrule {
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo.tool",
     out: ["foo_tool.out"],
     srcs: ["foo_tool.in"],
@@ -754,7 +740,7 @@ genrule {
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo.tools",
     out: ["foo_tool.out", "foo_tool2.out"],
     srcs: ["foo_tool.in"],
@@ -793,7 +779,7 @@ genrule {
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo",
     out: ["foo.out"],
     srcs: ["foo.in"],
@@ -809,14 +795,14 @@ genrule {
     tools = ["//other:foo.tool"],
 )`,
 			},
-			fs: otherGenruleBp,
+			filesystem: otherGenruleBp,
 		},
 		{
 			description:                        "genrule srcs using $(locations //absolute:label)",
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo",
     out: ["foo.out"],
     srcs: [":other.tool"],
@@ -832,14 +818,14 @@ genrule {
     tools = ["//other:foo.tool"],
 )`,
 			},
-			fs: otherGenruleBp,
+			filesystem: otherGenruleBp,
 		},
 		{
 			description:                        "genrule using $(location) label should substitute first tool label automatically",
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo",
     out: ["foo.out"],
     srcs: ["foo.in"],
@@ -858,14 +844,14 @@ genrule {
     ],
 )`,
 			},
-			fs: otherGenruleBp,
+			filesystem: otherGenruleBp,
 		},
 		{
 			description:                        "genrule using $(locations) label should substitute first tool label automatically",
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo",
     out: ["foo.out"],
     srcs: ["foo.in"],
@@ -884,14 +870,14 @@ genrule {
     ],
 )`,
 			},
-			fs: otherGenruleBp,
+			filesystem: otherGenruleBp,
 		},
 		{
 			description:                        "genrule without tools or tool_files can convert successfully",
 			moduleTypeUnderTest:                "genrule",
 			moduleTypeUnderTestFactory:         genrule.GenRuleFactory,
 			moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build,
-			bp: `genrule {
+			blueprint: `genrule {
     name: "foo",
     out: ["foo.out"],
     srcs: ["foo.in"],
@@ -914,24 +900,24 @@ genrule {
 		toParse := []string{
 			"Android.bp",
 		}
-		for f, content := range testCase.fs {
+		for f, content := range testCase.filesystem {
 			if strings.HasSuffix(f, "Android.bp") {
 				toParse = append(toParse, f)
 			}
 			fs[f] = []byte(content)
 		}
-		config := android.TestConfig(buildDir, nil, testCase.bp, fs)
+		config := android.TestConfig(buildDir, nil, testCase.blueprint, fs)
 		ctx := android.NewTestContext(config)
 		ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
 		ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
 		ctx.RegisterForBazelConversion()
 
 		_, errs := ctx.ParseFileList(dir, toParse)
-		if errored(t, testCase.description, errs) {
+		if errored(t, testCase, errs) {
 			continue
 		}
 		_, errs = ctx.ResolveDependencies(config)
-		if errored(t, testCase.description, errs) {
+		if errored(t, testCase, errs) {
 			continue
 		}
 
@@ -1353,30 +1339,20 @@ filegroup { name: "opt-out-h", bazel_module: { bp2build_available: false } }
 }
 
 func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
-	testCases := []struct {
+	testCases := []bp2buildTestCase{
-		description                        string
-		moduleTypeUnderTest                string
-		moduleTypeUnderTestFactory         android.ModuleFactory
-		moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext)
-		preArchMutators                    []android.RegisterMutatorFunc
-		bp                                 string
-		expectedBazelTargets               []string
-		fs                                 map[string]string
-		dir                                string
-	}{
 		{
 			description:                        "filegroup bazel_module.label",
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "fg_foo",
     bazel_module: { label: "//other:fg_foo" },
 }`,
 			expectedBazelTargets: []string{
 				`// BUILD file`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/BUILD.bazel": `// BUILD file`,
 			},
 		},
@@ -1385,7 +1361,7 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
 		    name: "fg_foo",
 		    bazel_module: { label: "//other:fg_foo" },
 		}
@@ -1397,7 +1373,7 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 			expectedBazelTargets: []string{
 				`// BUILD file`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/BUILD.bazel": `// BUILD file`,
 			},
 		},
@@ -1407,8 +1383,8 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
 			dir:                                "other",
-			bp:                                 ``,
+			blueprint:                          ``,
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/Android.bp": `filegroup {
 				name: "fg_foo",
 				bazel_module: {
@@ -1434,7 +1410,7 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
 		    name: "fg_foo",
 		    bazel_module: {
 		      label: "//other:fg_foo",
@@ -1453,7 +1429,7 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 )`,
 				`// BUILD file`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"other/BUILD.bazel": `// BUILD file`,
 			},
 		},
@@ -1466,24 +1442,24 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 			toParse := []string{
 				"Android.bp",
 			}
-			for f, content := range testCase.fs {
+			for f, content := range testCase.filesystem {
 				if strings.HasSuffix(f, "Android.bp") {
 					toParse = append(toParse, f)
 				}
 				fs[f] = []byte(content)
 			}
-			config := android.TestConfig(buildDir, nil, testCase.bp, fs)
+			config := android.TestConfig(buildDir, nil, testCase.blueprint, fs)
 			ctx := android.NewTestContext(config)
 			ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
 			ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
 			ctx.RegisterForBazelConversion()
 
 			_, errs := ctx.ParseFileList(dir, toParse)
-			if errored(t, testCase.description, errs) {
+			if errored(t, testCase, errs) {
 				return
 			}
 			_, errs = ctx.ResolveDependencies(config)
-			if errored(t, testCase.description, errs) {
+			if errored(t, testCase, errs) {
 				return
 			}
 
@@ -1517,22 +1493,13 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) {
 }
 
 func TestGlobExcludeSrcs(t *testing.T) {
-	testCases := []struct {
+	testCases := []bp2buildTestCase{
-		description                        string
-		moduleTypeUnderTest                string
-		moduleTypeUnderTestFactory         android.ModuleFactory
-		moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext)
-		bp                                 string
-		expectedBazelTargets               []string
-		fs                                 map[string]string
-		dir                                string
-	}{
 		{
 			description:                        "filegroup top level exclude_srcs",
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp: `filegroup {
+			blueprint: `filegroup {
     name: "fg_foo",
     srcs: ["**/*.txt"],
     exclude_srcs: ["c.txt"],
@@ -1548,7 +1515,7 @@ func TestGlobExcludeSrcs(t *testing.T) {
     ],
 )`,
 			},
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"a.txt":          "",
 				"b.txt":          "",
 				"c.txt":          "",
@@ -1562,9 +1529,9 @@ func TestGlobExcludeSrcs(t *testing.T) {
 			moduleTypeUnderTest:                "filegroup",
 			moduleTypeUnderTestFactory:         android.FileGroupFactory,
 			moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
-			bp:                                 "",
+			blueprint:                          "",
 			dir:                                "dir",
-			fs: map[string]string{
+			filesystem: map[string]string{
 				"dir/Android.bp": `filegroup {
     name: "fg_foo",
     srcs: ["**/*.txt"],
@@ -1596,24 +1563,24 @@ func TestGlobExcludeSrcs(t *testing.T) {
 		toParse := []string{
 			"Android.bp",
 		}
-		for f, content := range testCase.fs {
+		for f, content := range testCase.filesystem {
 			if strings.HasSuffix(f, "Android.bp") {
 				toParse = append(toParse, f)
 			}
 			fs[f] = []byte(content)
 		}
-		config := android.TestConfig(buildDir, nil, testCase.bp, fs)
+		config := android.TestConfig(buildDir, nil, testCase.blueprint, fs)
 		ctx := android.NewTestContext(config)
 		ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
 		ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
 		ctx.RegisterForBazelConversion()
 
 		_, errs := ctx.ParseFileList(dir, toParse)
-		if errored(t, testCase.description, errs) {
+		if errored(t, testCase, errs) {
 			continue
 		}
 		_, errs = ctx.ResolveDependencies(config)
-		if errored(t, testCase.description, errs) {
+		if errored(t, testCase, errs) {
 			continue
 		}
 

bp2build/filegroup_conversion_test.go

@@ -0,0 +1,62 @@
+// Copyright 2021 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package bp2build
+
+import (
+	"android/soong/android"
+	"fmt"
+
+	"testing"
+)
+
+func runFilegroupTestCase(t *testing.T, tc bp2buildTestCase) {
+	t.Helper()
+	runBp2BuildTestCase(t, registerFilegroupModuleTypes, tc)
+}
+
+func registerFilegroupModuleTypes(ctx android.RegistrationContext) {}
+
+func TestFilegroupSameNameAsFile_OneFile(t *testing.T) {
+	runFilegroupTestCase(t, bp2buildTestCase{
+		description:                        "filegroup - same name as file, with one file",
+		moduleTypeUnderTest:                "filegroup",
+		moduleTypeUnderTestFactory:         android.FileGroupFactory,
+		moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
+		filesystem:                         map[string]string{},
+		blueprint: `
+filegroup {
+    name: "foo",
+    srcs: ["foo"],
+}
+`,
+		expectedBazelTargets: []string{}})
+}
+
+func TestFilegroupSameNameAsFile_MultipleFiles(t *testing.T) {
+	runFilegroupTestCase(t, bp2buildTestCase{
+		description:                        "filegroup - same name as file, with multiple files",
+		moduleTypeUnderTest:                "filegroup",
+		moduleTypeUnderTestFactory:         android.FileGroupFactory,
+		moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build,
+		filesystem:                         map[string]string{},
+		blueprint: `
+filegroup {
+	name: "foo",
+	srcs: ["foo", "bar"],
+}
+`,
+		expectedErr: fmt.Errorf("filegroup 'foo' cannot contain a file with the same name"),
+	})
+}

bp2build/testing.go

@@ -36,14 +36,35 @@ var (
 	buildDir string
 )
 
-func errored(t *testing.T, desc string, errs []error) bool {
+func checkError(t *testing.T, errs []error, expectedErr error) bool {
 	t.Helper()
+
+	// expectedErr is not nil, find it in the list of errors
+	if len(errs) != 1 {
+		t.Errorf("Expected only 1 error, got %d: %q", len(errs), errs)
+	}
+	if errs[0].Error() == expectedErr.Error() {
+		return true
+	}
+
+	return false
+}
+
+func errored(t *testing.T, tc bp2buildTestCase, errs []error) bool {
+	t.Helper()
+	if tc.expectedErr != nil {
+		// Rely on checkErrors, as this test case is expected to have an error.
+		return false
+	}
+
 	if len(errs) > 0 {
 		for _, err := range errs {
-			t.Errorf("%s: %s", desc, err)
+			t.Errorf("%s: %s", tc.description, err)
 		}
 		return true
 	}
+
+	// All good, continue execution.
 	return false
 }
 
@@ -61,6 +82,7 @@ type bp2buildTestCase struct {
 	expectedBazelTargets               []string
 	filesystem                         map[string]string
 	dir                                string
+	expectedErr                        error
 }
 
 func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.RegistrationContext), tc bp2buildTestCase) {
@@ -85,12 +107,17 @@ func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi
 	ctx.RegisterBp2BuildMutator(tc.moduleTypeUnderTest, tc.moduleTypeUnderTestBp2BuildMutator)
 	ctx.RegisterForBazelConversion()
 
-	_, errs := ctx.ParseFileList(dir, toParse)
+	_, parseErrs := ctx.ParseFileList(dir, toParse)
-	if errored(t, tc.description, errs) {
+	if errored(t, tc, parseErrs) {
 		return
 	}
-	_, errs = ctx.ResolveDependencies(config)
+	_, resolveDepsErrs := ctx.ResolveDependencies(config)
-	if errored(t, tc.description, errs) {
+	if errored(t, tc, resolveDepsErrs) {
+		return
+	}
+
+	errs := append(parseErrs, resolveDepsErrs...)
+	if tc.expectedErr != nil && checkError(t, errs, tc.expectedErr) {
 		return
 	}