diff --git a/android/bazel_paths.go b/android/bazel_paths.go index bbdae96e7..0d38bdad2 100644 --- a/android/bazel_paths.go +++ b/android/bazel_paths.go @@ -202,20 +202,26 @@ func BazelLabelForModuleSrcExcludes(ctx BazelConversionPathContext, paths, exclu return labels } -// Returns true if a prefix + components[:i] + /Android.bp exists -// TODO(b/185358476) Could check for BUILD file instead of checking for Android.bp file, or ensure BUILD is always generated? -func directoryHasBlueprint(fs pathtools.FileSystem, prefix string, components []string, componentIndex int) bool { - blueprintPath := prefix - if blueprintPath != "" { - blueprintPath = blueprintPath + "/" - } - blueprintPath = blueprintPath + strings.Join(components[:componentIndex+1], "/") - blueprintPath = blueprintPath + "/Android.bp" - if exists, _, _ := fs.Exists(blueprintPath); exists { +// Returns true if a prefix + components[:i] is a package boundary. +// +// A package boundary is determined by a BUILD file in the directory. This can happen in 2 cases: +// +// 1. An Android.bp exists, which bp2build will always convert to a sibling BUILD file. +// 2. An Android.bp doesn't exist, but a checked-in BUILD/BUILD.bazel file exists, and that file +// is allowlisted by the bp2build configuration to be merged into the symlink forest workspace. +func isPackageBoundary(config Config, prefix string, components []string, componentIndex int) bool { + prefix = filepath.Join(prefix, filepath.Join(components[:componentIndex+1]...)) + if exists, _, _ := config.fs.Exists(filepath.Join(prefix, "Android.bp")); exists { return true - } else { - return false + } else if config.Bp2buildPackageConfig.ShouldKeepExistingBuildFileForDir(prefix) { + if exists, _, _ := config.fs.Exists(filepath.Join(prefix, "BUILD")); exists { + return true + } else if exists, _, _ := config.fs.Exists(filepath.Join(prefix, "BUILD.bazel")); exists { + return true + } } + + return false } // Transform a path (if necessary) to acknowledge package boundaries @@ -245,14 +251,14 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b newLabel := "" pathComponents := strings.Split(path.Label, "/") - foundBlueprint := false + foundPackageBoundary := false // Check the deepest subdirectory first and work upwards for i := len(pathComponents) - 1; i >= 0; i-- { pathComponent := pathComponents[i] var sep string - if !foundBlueprint && directoryHasBlueprint(ctx.Config().fs, ctx.ModuleDir(), pathComponents, i) { + if !foundPackageBoundary && isPackageBoundary(ctx.Config(), ctx.ModuleDir(), pathComponents, i) { sep = ":" - foundBlueprint = true + foundPackageBoundary = true } else { sep = "/" } @@ -262,7 +268,7 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b newLabel = pathComponent + sep + newLabel } } - if foundBlueprint { + if foundPackageBoundary { // Ensure paths end up looking like //bionic/... instead of //./bionic/... moduleDir := ctx.ModuleDir() if strings.HasPrefix(moduleDir, ".") { diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index dd28c3c94..d677c0f5b 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -986,57 +986,6 @@ func TestModuleTypeBp2Build(t *testing.T) { }), }, }, - { - Description: "filegroup with glob", - ModuleTypeUnderTest: "filegroup", - ModuleTypeUnderTestFactory: android.FileGroupFactory, - Blueprint: `filegroup { - name: "fg_foo", - srcs: ["**/*.txt"], - bazel_module: { bp2build_available: true }, -}`, - ExpectedBazelTargets: []string{ - MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ - "srcs": `[ - "other/a.txt", - "other/b.txt", - "other/subdir/a.txt", - ]`, - }), - }, - Filesystem: map[string]string{ - "other/a.txt": "", - "other/b.txt": "", - "other/subdir/a.txt": "", - "other/file": "", - }, - }, - { - Description: "filegroup with glob in subdir", - ModuleTypeUnderTest: "filegroup", - ModuleTypeUnderTestFactory: android.FileGroupFactory, - Dir: "other", - Filesystem: map[string]string{ - "other/Android.bp": `filegroup { - name: "fg_foo", - srcs: ["**/*.txt"], - bazel_module: { bp2build_available: true }, -}`, - "other/a.txt": "", - "other/b.txt": "", - "other/subdir/a.txt": "", - "other/file": "", - }, - ExpectedBazelTargets: []string{ - MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ - "srcs": `[ - "a.txt", - "b.txt", - "subdir/a.txt", - ]`, - }), - }, - }, { Description: "depends_on_other_dir_module", ModuleTypeUnderTest: "filegroup", @@ -1429,6 +1378,226 @@ func TestCombineBuildFilesBp2buildTargets(t *testing.T) { } } +func TestGlob(t *testing.T) { + testCases := []Bp2buildTestCase{ + { + Description: "filegroup with glob", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + Blueprint: `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "other/a.txt", + "other/b.txt", + "other/subdir/a.txt", + ]`, + }), + }, + Filesystem: map[string]string{ + "other/a.txt": "", + "other/b.txt": "", + "other/subdir/a.txt": "", + "other/file": "", + }, + }, + { + Description: "filegroup with glob in subdir", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + Dir: "other", + Filesystem: map[string]string{ + "other/Android.bp": `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + "other/a.txt": "", + "other/b.txt": "", + "other/subdir/a.txt": "", + "other/file": "", + }, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "a.txt", + "b.txt", + "subdir/a.txt", + ]`, + }), + }, + }, + { + Description: "filegroup with glob with no kept BUILD files", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + KeepBuildFileForDirs: []string{ + // empty + }, + Blueprint: `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + Filesystem: map[string]string{ + "a.txt": "", + "b.txt": "", + "foo/BUILD": "", + "foo/a.txt": "", + "foo/bar/BUILD": "", + "foo/bar/b.txt": "", + }, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "a.txt", + "b.txt", + "foo/a.txt", + "foo/bar/b.txt", + ]`, + }), + }, + }, + { + Description: "filegroup with glob with kept BUILD file", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + KeepBuildFileForDirs: []string{ + "foo", + }, + Blueprint: `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + Filesystem: map[string]string{ + "a.txt": "", + "b.txt": "", + "foo/BUILD": "", + "foo/a.txt": "", + "foo/bar/BUILD": "", + "foo/bar/b.txt": "", + }, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "a.txt", + "b.txt", + "//foo:a.txt", + "//foo:bar/b.txt", + ]`, + }), + }, + }, + { + Description: "filegroup with glob with kept BUILD.bazel file", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + KeepBuildFileForDirs: []string{ + "foo", + }, + Blueprint: `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + Filesystem: map[string]string{ + "a.txt": "", + "b.txt": "", + "foo/BUILD.bazel": "", + "foo/a.txt": "", + "foo/bar/BUILD.bazel": "", + "foo/bar/b.txt": "", + }, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "a.txt", + "b.txt", + "//foo:a.txt", + "//foo:bar/b.txt", + ]`, + }), + }, + }, + { + Description: "filegroup with glob with Android.bp file as boundary", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + Blueprint: `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + Filesystem: map[string]string{ + "a.txt": "", + "b.txt": "", + "foo/Android.bp": "", + "foo/a.txt": "", + "foo/bar/Android.bp": "", + "foo/bar/b.txt": "", + }, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "a.txt", + "b.txt", + "//foo:a.txt", + "//foo/bar:b.txt", + ]`, + }), + }, + }, + { + Description: "filegroup with glob in subdir with kept BUILD and BUILD.bazel file", + ModuleTypeUnderTest: "filegroup", + ModuleTypeUnderTestFactory: android.FileGroupFactory, + Dir: "other", + KeepBuildFileForDirs: []string{ + "other/foo", + "other/foo/bar", + // deliberately not other/foo/baz/BUILD. + }, + Filesystem: map[string]string{ + "other/Android.bp": `filegroup { + name: "fg_foo", + srcs: ["**/*.txt"], + bazel_module: { bp2build_available: true }, +}`, + "other/a.txt": "", + "other/b.txt": "", + "other/foo/BUILD": "", + "other/foo/a.txt": "", + "other/foo/bar/BUILD.bazel": "", + "other/foo/bar/b.txt": "", + "other/foo/baz/BUILD": "", + "other/foo/baz/c.txt": "", + }, + ExpectedBazelTargets: []string{ + MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{ + "srcs": `[ + "a.txt", + "b.txt", + "//other/foo:a.txt", + "//other/foo/bar:b.txt", + "//other/foo:baz/c.txt", + ]`, + }), + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Description, func(t *testing.T) { + RunBp2BuildTestCaseSimple(t, testCase) + }) + } +} + func TestGlobExcludeSrcs(t *testing.T) { testCases := []Bp2buildTestCase{ { diff --git a/bp2build/testing.go b/bp2build/testing.go index 8ce8bb28e..c2c1b197d 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -30,13 +30,6 @@ import ( ) var ( - // A default configuration for tests to not have to specify bp2build_available on top level targets. - bp2buildConfig = android.NewBp2BuildAllowlist().SetDefaultConfig( - allowlists.Bp2BuildConfig{ - android.Bp2BuildTopLevel: allowlists.Bp2BuildDefaultTrueRecursively, - }, - ) - buildDir string ) @@ -87,6 +80,11 @@ type Bp2buildTestCase struct { // An error with a string contained within the string of the expected error ExpectedErr error UnconvertedDepsMode unconvertedDepsMode + + // For every directory listed here, the BUILD file for that directory will + // be merged with the generated BUILD file. This allows custom BUILD targets + // to be used in tests, or use BUILD files to draw package boundaries. + KeepBuildFileForDirs []string } func RunBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.RegistrationContext), tc Bp2buildTestCase) { @@ -107,6 +105,18 @@ func RunBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi registerModuleTypes(ctx) ctx.RegisterModuleType(tc.ModuleTypeUnderTest, tc.ModuleTypeUnderTestFactory) + + // A default configuration for tests to not have to specify bp2build_available on top level targets. + bp2buildConfig := android.NewBp2BuildAllowlist().SetDefaultConfig( + allowlists.Bp2BuildConfig{ + android.Bp2BuildTopLevel: allowlists.Bp2BuildDefaultTrueRecursively, + }, + ) + for _, f := range tc.KeepBuildFileForDirs { + bp2buildConfig.SetKeepExistingBuildFile(map[string]bool{ + f: /*recursive=*/ false, + }) + } ctx.RegisterBp2BuildConfig(bp2buildConfig) ctx.RegisterForBazelConversion()