Respect package boundaries in bp2build conversion of go modules

bp2build's codegen context does not implement
BazelPathConversionContext. To reuse the utility function
transformPackagePaths, update its signature

(Also make deps of go_library unique to make the conversion resilient)

Test: go test ./bp2build
Change-Id: I126b1057d2b26bc6c7d3be2780f1b62d28323cf0
This commit is contained in:
Spandan Das
2023-06-21 01:50:33 +00:00
parent 69afa98fbd
commit 0a8a27500e
4 changed files with 53 additions and 22 deletions

View File

@@ -198,7 +198,7 @@ func BazelLabelForModuleSrcExcludes(ctx BazelConversionPathContext, paths, exclu
} }
labels := expandSrcsForBazel(ctx, paths, excluded) labels := expandSrcsForBazel(ctx, paths, excluded)
labels.Excludes = excludeLabels.Includes labels.Excludes = excludeLabels.Includes
labels = transformSubpackagePaths(ctx, labels) labels = TransformSubpackagePaths(ctx.Config(), ctx.ModuleDir(), labels)
return labels return labels
} }
@@ -237,7 +237,7 @@ func isPackageBoundary(config Config, prefix string, components []string, compon
// if the "async_safe" directory is actually a package and not just a directory. // if the "async_safe" directory is actually a package and not just a directory.
// //
// In particular, paths that extend into packages are transformed into absolute labels beginning with //. // In particular, paths that extend into packages are transformed into absolute labels beginning with //.
func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) bazel.Label { func transformSubpackagePath(cfg Config, dir string, path bazel.Label) bazel.Label {
var newPath bazel.Label var newPath bazel.Label
// Don't transform OriginalModuleName // Don't transform OriginalModuleName
@@ -281,7 +281,7 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b
for i := len(pathComponents) - 1; i >= 0; i-- { for i := len(pathComponents) - 1; i >= 0; i-- {
pathComponent := pathComponents[i] pathComponent := pathComponents[i]
var sep string var sep string
if !foundPackageBoundary && isPackageBoundary(ctx.Config(), ctx.ModuleDir(), pathComponents, i) { if !foundPackageBoundary && isPackageBoundary(cfg, dir, pathComponents, i) {
sep = ":" sep = ":"
foundPackageBoundary = true foundPackageBoundary = true
} else { } else {
@@ -295,7 +295,7 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b
} }
if foundPackageBoundary { if foundPackageBoundary {
// Ensure paths end up looking like //bionic/... instead of //./bionic/... // Ensure paths end up looking like //bionic/... instead of //./bionic/...
moduleDir := ctx.ModuleDir() moduleDir := dir
if strings.HasPrefix(moduleDir, ".") { if strings.HasPrefix(moduleDir, ".") {
moduleDir = moduleDir[1:] moduleDir = moduleDir[1:]
} }
@@ -313,13 +313,13 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b
// Transform paths to acknowledge package boundaries // Transform paths to acknowledge package boundaries
// See transformSubpackagePath() for more information // See transformSubpackagePath() for more information
func transformSubpackagePaths(ctx BazelConversionPathContext, paths bazel.LabelList) bazel.LabelList { func TransformSubpackagePaths(cfg Config, dir string, paths bazel.LabelList) bazel.LabelList {
var newPaths bazel.LabelList var newPaths bazel.LabelList
for _, include := range paths.Includes { for _, include := range paths.Includes {
newPaths.Includes = append(newPaths.Includes, transformSubpackagePath(ctx, include)) newPaths.Includes = append(newPaths.Includes, transformSubpackagePath(cfg, dir, include))
} }
for _, exclude := range paths.Excludes { for _, exclude := range paths.Excludes {
newPaths.Excludes = append(newPaths.Excludes, transformSubpackagePath(ctx, exclude)) newPaths.Excludes = append(newPaths.Excludes, transformSubpackagePath(cfg, dir, exclude))
} }
return newPaths return newPaths
} }

View File

@@ -175,7 +175,7 @@ func TestTransformSubpackagePath(t *testing.T) {
"./z/b.c": "z/b.c", "./z/b.c": "z/b.c",
} }
for in, out := range pairs { for in, out := range pairs {
actual := transformSubpackagePath(ctx, bazel.Label{Label: in}).Label actual := transformSubpackagePath(ctx.Config(), ctx.ModuleDir(), bazel.Label{Label: in}).Label
if actual != out { if actual != out {
t.Errorf("expected:\n%v\nactual:\n%v", out, actual) t.Errorf("expected:\n%v\nactual:\n%v", out, actual)
} }

View File

@@ -302,21 +302,25 @@ func targetNotCompatibleWithAndroid() bazel.LabelListAttribute {
// helper function to return labels for srcs used in bootstrap_go_package and bootstrap_go_binary // helper function to return labels for srcs used in bootstrap_go_package and bootstrap_go_binary
// this function has the following limitations which make it unsuitable for widespread use // this function has the following limitations which make it unsuitable for widespread use
// 1. wildcard patterns in srcs // - wildcard patterns in srcs
// 2. package boundary violations // This is ok for go since build/blueprint does not support it.
// (1) is ok for go since build/blueprint does not support it. (2) _might_ be ok too.
// //
// Prefer to use `BazelLabelForModuleSrc` instead // Prefer to use `BazelLabelForModuleSrc` instead
func goSrcLabels(srcs []string, linuxSrcs, darwinSrcs []string) bazel.LabelListAttribute { func goSrcLabels(cfg android.Config, moduleDir string, srcs []string, linuxSrcs, darwinSrcs []string) bazel.LabelListAttribute {
labels := func(srcs []string) bazel.LabelList { labels := func(srcs []string) bazel.LabelList {
ret := []bazel.Label{} ret := []bazel.Label{}
for _, src := range srcs { for _, src := range srcs {
srcLabel := bazel.Label{ srcLabel := bazel.Label{
Label: ":" + src, // TODO - b/284483729: Fix for possible package boundary violations Label: src,
} }
ret = append(ret, srcLabel) ret = append(ret, srcLabel)
} }
return bazel.MakeLabelList(ret) // Respect package boundaries
return android.TransformSubpackagePaths(
cfg,
moduleDir,
bazel.MakeLabelList(ret),
)
} }
ret := bazel.LabelListAttribute{} ret := bazel.LabelListAttribute{}
@@ -371,8 +375,11 @@ func generateBazelTargetsGoPackage(ctx *android.Context, g *bootstrap.GoPackage,
Importpath: bazel.StringAttribute{ Importpath: bazel.StringAttribute{
Value: proptools.StringPtr(g.GoPkgPath()), Value: proptools.StringPtr(g.GoPkgPath()),
}, },
Srcs: goSrcLabels(g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), Srcs: goSrcLabels(ctx.Config(), ctx.ModuleDir(g), g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()),
Deps: goDepLabels(transitiveDeps, goModulesMap), Deps: goDepLabels(
android.FirstUniqueStrings(transitiveDeps),
goModulesMap,
),
Target_compatible_with: targetNotCompatibleWithAndroid(), Target_compatible_with: targetNotCompatibleWithAndroid(),
} }
@@ -444,7 +451,7 @@ func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, g
transitiveDeps := transitiveGoDeps(g.Deps(), goModulesMap) transitiveDeps := transitiveGoDeps(g.Deps(), goModulesMap)
ga := goAttributes{ ga := goAttributes{
Srcs: goSrcLabels(g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), Srcs: goSrcLabels(ctx.Config(), ctx.ModuleDir(g), g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()),
Deps: goDepLabels(transitiveDeps, goModulesMap), Deps: goDepLabels(transitiveDeps, goModulesMap),
Target_compatible_with: targetNotCompatibleWithAndroid(), Target_compatible_with: targetNotCompatibleWithAndroid(),
} }

View File

@@ -75,11 +75,11 @@ bootstrap_go_package {
"deps": `["//bar:bar"]`, "deps": `["//bar:bar"]`,
"importpath": `"android/foo"`, "importpath": `"android/foo"`,
"srcs": `[ "srcs": `[
":foo1.go", "foo1.go",
":foo2.go", "foo2.go",
] + select({ ] + select({
"//build/bazel/platforms/os:darwin": [":foo_darwin.go"], "//build/bazel/platforms/os:darwin": ["foo_darwin.go"],
"//build/bazel/platforms/os:linux_glibc": [":foo_linux.go"], "//build/bazel/platforms/os:linux_glibc": ["foo_linux.go"],
"//conditions:default": [], "//conditions:default": [],
})`, })`,
}, },
@@ -118,7 +118,31 @@ bootstrap_go_package {
"//bar:bar", "//bar:bar",
"//bar:baz", "//bar:baz",
]`, ]`,
"srcs": `[":main.go"]`, "srcs": `["main.go"]`,
},
android.HostSupported,
)},
})
}
func TestConvertGoBinaryWithSrcInDifferentPackage(t *testing.T) {
bp := `
blueprint_go_binary {
name: "foo",
srcs: ["subdir/main.go"],
}
`
t.Parallel()
runGoTests(t, Bp2buildTestCase{
Description: "Convert blueprint_go_binary with src in different package",
Blueprint: bp,
Filesystem: map[string]string{
"subdir/Android.bp": "",
},
ExpectedBazelTargets: []string{makeBazelTargetHostOrDevice("go_binary", "foo",
AttrNameToString{
"deps": `[]`,
"srcs": `["//subdir:main.go"]`,
}, },
android.HostSupported, android.HostSupported,
)}, )},