diff --git a/android/bazel.go b/android/bazel.go index 692fbcdd2..373e292f2 100644 --- a/android/bazel.go +++ b/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", diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 9c922dda4..50b79fa15 100644 --- a/android/bazel_handler.go +++ b/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)) - labelsString := strings.Join(labels, ",\n ") - configNodesSection += fmt.Sprintf(configNodeFormatString, archString, archString, labelsString) + if archString == "common" { + // arch-less labels (e.g. filegroups) don't need a config_node + 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"), diff --git a/android/bazel_handler_test.go b/android/bazel_handler_test.go index 557faea21..cdf1a6316 100644 --- a/android/bazel_handler_test.go +++ b/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 { diff --git a/android/filegroup.go b/android/filegroup.go index 54d01d39f..4db165f87 100644 --- a/android/filegroup.go +++ b/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 } diff --git a/bp2build/Android.bp b/bp2build/Android.bp index b1ccc963c..38518ed41 100644 --- a/bp2build/Android.bp +++ b/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", diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 0d9106c96..55acab4f4 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -218,13 +218,9 @@ func TestGenerateSoongModuleTargets(t *testing.T) { } func TestGenerateBazelTargetModules(t *testing.T) { - testCases := []struct { - name string - bp string - expectedBazelTargets []string - }{ + testCases := []bp2buildTestCase{ { - 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 { - description string - moduleTypeUnderTest string - moduleTypeUnderTestFactory android.ModuleFactory - moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext) - preArchMutators []android.RegisterMutatorFunc - bp string - expectedBazelTargets []string - fs map[string]string - dir string - }{ + testCases := []bp2buildTestCase{ { 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 { - description string - moduleTypeUnderTest string - moduleTypeUnderTestFactory android.ModuleFactory - moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext) - preArchMutators []android.RegisterMutatorFunc - bp string - expectedBazelTargets []string - fs map[string]string - dir string - }{ + testCases := []bp2buildTestCase{ { 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: ``, - fs: map[string]string{ + blueprint: ``, + 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 { - description string - moduleTypeUnderTest string - moduleTypeUnderTestFactory android.ModuleFactory - moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext) - bp string - expectedBazelTargets []string - fs map[string]string - dir string - }{ + testCases := []bp2buildTestCase{ { 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 } diff --git a/bp2build/filegroup_conversion_test.go b/bp2build/filegroup_conversion_test.go new file mode 100644 index 000000000..ad9923610 --- /dev/null +++ b/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"), + }) +} diff --git a/bp2build/testing.go b/bp2build/testing.go index a549a9369..4995b0965 100644 --- a/bp2build/testing.go +++ b/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) - if errored(t, tc.description, errs) { + _, parseErrs := ctx.ParseFileList(dir, toParse) + if errored(t, tc, parseErrs) { return } - _, errs = ctx.ResolveDependencies(config) - if errored(t, tc.description, errs) { + _, resolveDepsErrs := ctx.ResolveDependencies(config) + if errored(t, tc, resolveDepsErrs) { + return + } + + errs := append(parseErrs, resolveDepsErrs...) + if tc.expectedErr != nil && checkError(t, errs, tc.expectedErr) { return }