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

Commit 0eeaeb8e authored by Jingwen Chen's avatar Jingwen Chen
Browse files

Fix package boundary in glob expansion with checked in BUILD files. 

For directories without an Android.bp file, if they contain a merged
checked in BUILD file, it becomes a package boundary in the symlink
forest. However, the current glob expansion is only aware of Android.bp
files, but not merged BUILD files, so it generates glob expansions
incorrectly for files that cross the package boundary.

This CL fixes that by making the package boundary function aware of the
keepExistingBuildFile allowlist. See the new test cases for example.

Also stop using the same global bp2buildConfig for every test case to
allow each test to define their own bp2build allowlists.

Bug: 185358476
Test: new unit tests

Change-Id: Ifcd1283480a09a642ef4343e5bbc599583524577
parent 94a0373f

android/bazel_paths.go

+22 −16
Original line number Diff line number Diff line
@@ -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, ".") {

bp2build/build_conversion_test.go

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

		{

bp2build/testing.go

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