Bladeren bron

File paths to allow "./" prefix

  bazel_path conversion would treat "./" as a package boundary
  which would be incorrect

Bug: b/243072036
Test: unit test build_conversion_test.go and `m libc`
Change-Id: I0fc44d2fd7a4270442c8755219a11fd585244c5c
Usta Shrestha 1 jaar geleden
bovenliggende
commit
d558031e0c
3 gewijzigde bestanden met toevoegingen van 119 en 6 verwijderingen
  1. 25 5
      android/bazel_paths.go
  2. 76 1
      android/bazel_paths_test.go
  3. 18 0
      bp2build/build_conversion_test.go

+ 25 - 5
android/bazel_paths.go

@@ -206,9 +206,9 @@ func BazelLabelForModuleSrcExcludes(ctx BazelConversionPathContext, paths, exclu
 //
 // 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.
+//  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 {
@@ -248,9 +248,29 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b
 		newPath.Label = path.Label
 		return newPath
 	}
-
-	newLabel := ""
+	if strings.HasPrefix(path.Label, "./") {
+		// Drop "./" for consistent handling of paths.
+		// Specifically, to not let "." be considered a package boundary.
+		// Say `inputPath` is `x/Android.bp` and that file has some module
+		// with `srcs=["y/a.c", "z/b.c"]`.
+		// And say the directory tree is:
+		//     x
+		//     ├── Android.bp
+		//     ├── y
+		//     │   ├── a.c
+		//     │   └── Android.bp
+		//     └── z
+		//         └── b.c
+		// Then bazel equivalent labels in srcs should be:
+		//   //x/y:a.c, x/z/b.c
+		// The above should still be the case if `x/Android.bp` had
+		//   srcs=["./y/a.c", "./z/b.c"]
+		// However, if we didn't strip "./", we'd get
+		//   //x/./y:a.c, //x/.:z/b.c
+		path.Label = strings.TrimPrefix(path.Label, "./")
+	}
 	pathComponents := strings.Split(path.Label, "/")
+	newLabel := ""
 	foundPackageBoundary := false
 	// Check the deepest subdirectory first and work upwards
 	for i := len(pathComponents) - 1; i >= 0; i-- {

+ 76 - 1
android/bazel_paths_test.go

@@ -17,6 +17,10 @@ package android
 import (
 	"path/filepath"
 	"testing"
+
+	"android/soong/bazel"
+	"github.com/google/blueprint"
+	"github.com/google/blueprint/pathtools"
 )
 
 type TestBazelPathContext struct{}
@@ -29,7 +33,7 @@ func (*TestBazelPathContext) Config() Config {
 	return cfg
 }
 
-func (*TestBazelPathContext) AddNinjaFileDeps(deps ...string) {
+func (*TestBazelPathContext) AddNinjaFileDeps(...string) {
 	panic("Unimplemented")
 }
 
@@ -106,3 +110,74 @@ func TestPathForBazelOutRelativeWithParentDirectoryRoot(t *testing.T) {
 		t.Errorf("incorrect OutputPath.Rel(): expected %q, got %q", expectedRelPath, out.Rel())
 	}
 }
+
+type TestBazelConversionPathContext struct {
+	TestBazelConversionContext
+	moduleDir string
+	cfg       Config
+}
+
+func (ctx *TestBazelConversionPathContext) AddNinjaFileDeps(...string) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) GlobWithDeps(string, []string) ([]string, error) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) PropertyErrorf(string, string, ...interface{}) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) GetDirectDep(string) (blueprint.Module, blueprint.DependencyTag) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) ModuleFromName(string) (blueprint.Module, bool) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) AddUnconvertedBp2buildDep(string) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) AddMissingBp2buildDep(string) {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) Module() Module {
+	panic("Unimplemented")
+}
+
+func (ctx *TestBazelConversionPathContext) Config() Config {
+	return ctx.cfg
+}
+
+func (ctx *TestBazelConversionPathContext) ModuleDir() string {
+	return ctx.moduleDir
+}
+
+func TestTransformSubpackagePath(t *testing.T) {
+	cfg := NullConfig("out", "out/soong")
+	cfg.fs = pathtools.MockFs(map[string][]byte{
+		"x/Android.bp":   nil,
+		"x/y/Android.bp": nil,
+	})
+
+	var ctx BazelConversionPathContext = &TestBazelConversionPathContext{
+		moduleDir: "x",
+		cfg:       cfg,
+	}
+	pairs := map[string]string{
+		"y/a.c":   "//x/y:a.c",
+		"./y/a.c": "//x/y:a.c",
+		"z/b.c":   "z/b.c",
+		"./z/b.c": "z/b.c",
+	}
+	for in, out := range pairs {
+		actual := transformSubpackagePath(ctx, bazel.Label{Label: in}).Label
+		if actual != out {
+			t.Errorf("expected:\n%v\nactual:\n%v", out, actual)
+		}
+	}
+}

+ 18 - 0
bp2build/build_conversion_test.go

@@ -960,6 +960,24 @@ func TestModuleTypeBp2Build(t *testing.T) {
     name: "fg_foo",
     srcs: ["a", "b"],
     bazel_module: { bp2build_available: true },
+}`,
+			ExpectedBazelTargets: []string{
+				MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{
+					"srcs": `[
+        "a",
+        "b",
+    ]`,
+				}),
+			},
+		},
+		{
+			Description:                "filegroup with dot-slash-prefixed srcs",
+			ModuleTypeUnderTest:        "filegroup",
+			ModuleTypeUnderTestFactory: android.FileGroupFactory,
+			Blueprint: `filegroup {
+    name: "fg_foo",
+    srcs: ["./a", "./b"],
+    bazel_module: { bp2build_available: true },
 }`,
 			ExpectedBazelTargets: []string{
 				MakeBazelTargetNoRestrictions("filegroup", "fg_foo", map[string]string{