From 0a8a27500ef49c16a0f53963ce27eb90efe1b11d Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 21 Jun 2023 01:50:33 +0000 Subject: [PATCH] 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 --- android/bazel_paths.go | 14 +++++++------- android/bazel_paths_test.go | 2 +- bp2build/build_conversion.go | 25 ++++++++++++++++--------- bp2build/go_conversion_test.go | 34 +++++++++++++++++++++++++++++----- 4 files changed, 53 insertions(+), 22 deletions(-) diff --git a/android/bazel_paths.go b/android/bazel_paths.go index 872e908e8..2f5ff643d 100644 --- a/android/bazel_paths.go +++ b/android/bazel_paths.go @@ -198,7 +198,7 @@ func BazelLabelForModuleSrcExcludes(ctx BazelConversionPathContext, paths, exclu } labels := expandSrcsForBazel(ctx, paths, excluded) labels.Excludes = excludeLabels.Includes - labels = transformSubpackagePaths(ctx, labels) + labels = TransformSubpackagePaths(ctx.Config(), ctx.ModuleDir(), 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. // // 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 // Don't transform OriginalModuleName @@ -281,7 +281,7 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b for i := len(pathComponents) - 1; i >= 0; i-- { pathComponent := pathComponents[i] var sep string - if !foundPackageBoundary && isPackageBoundary(ctx.Config(), ctx.ModuleDir(), pathComponents, i) { + if !foundPackageBoundary && isPackageBoundary(cfg, dir, pathComponents, i) { sep = ":" foundPackageBoundary = true } else { @@ -295,7 +295,7 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b } if foundPackageBoundary { // Ensure paths end up looking like //bionic/... instead of //./bionic/... - moduleDir := ctx.ModuleDir() + moduleDir := dir if strings.HasPrefix(moduleDir, ".") { moduleDir = moduleDir[1:] } @@ -313,13 +313,13 @@ func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) b // Transform paths to acknowledge package boundaries // 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 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 { - newPaths.Excludes = append(newPaths.Excludes, transformSubpackagePath(ctx, exclude)) + newPaths.Excludes = append(newPaths.Excludes, transformSubpackagePath(cfg, dir, exclude)) } return newPaths } diff --git a/android/bazel_paths_test.go b/android/bazel_paths_test.go index 450bf7674..60c0a1478 100644 --- a/android/bazel_paths_test.go +++ b/android/bazel_paths_test.go @@ -175,7 +175,7 @@ func TestTransformSubpackagePath(t *testing.T) { "./z/b.c": "z/b.c", } 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 { t.Errorf("expected:\n%v\nactual:\n%v", out, actual) } diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 9921a7f23..a817386ac 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -302,21 +302,25 @@ func targetNotCompatibleWithAndroid() bazel.LabelListAttribute { // 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 -// 1. wildcard patterns in srcs -// 2. package boundary violations -// (1) is ok for go since build/blueprint does not support it. (2) _might_ be ok too. +// - wildcard patterns in srcs +// This is ok for go since build/blueprint does not support it. // // 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 { ret := []bazel.Label{} for _, src := range srcs { srcLabel := bazel.Label{ - Label: ":" + src, // TODO - b/284483729: Fix for possible package boundary violations + Label: src, } ret = append(ret, srcLabel) } - return bazel.MakeLabelList(ret) + // Respect package boundaries + return android.TransformSubpackagePaths( + cfg, + moduleDir, + bazel.MakeLabelList(ret), + ) } ret := bazel.LabelListAttribute{} @@ -371,8 +375,11 @@ func generateBazelTargetsGoPackage(ctx *android.Context, g *bootstrap.GoPackage, Importpath: bazel.StringAttribute{ Value: proptools.StringPtr(g.GoPkgPath()), }, - Srcs: goSrcLabels(g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), - Deps: goDepLabels(transitiveDeps, goModulesMap), + Srcs: goSrcLabels(ctx.Config(), ctx.ModuleDir(g), g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), + Deps: goDepLabels( + android.FirstUniqueStrings(transitiveDeps), + goModulesMap, + ), Target_compatible_with: targetNotCompatibleWithAndroid(), } @@ -444,7 +451,7 @@ func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, g transitiveDeps := transitiveGoDeps(g.Deps(), goModulesMap) 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), Target_compatible_with: targetNotCompatibleWithAndroid(), } diff --git a/bp2build/go_conversion_test.go b/bp2build/go_conversion_test.go index a872fc7db..507fbf0ae 100644 --- a/bp2build/go_conversion_test.go +++ b/bp2build/go_conversion_test.go @@ -75,11 +75,11 @@ bootstrap_go_package { "deps": `["//bar:bar"]`, "importpath": `"android/foo"`, "srcs": `[ - ":foo1.go", - ":foo2.go", + "foo1.go", + "foo2.go", ] + select({ - "//build/bazel/platforms/os:darwin": [":foo_darwin.go"], - "//build/bazel/platforms/os:linux_glibc": [":foo_linux.go"], + "//build/bazel/platforms/os:darwin": ["foo_darwin.go"], + "//build/bazel/platforms/os:linux_glibc": ["foo_linux.go"], "//conditions:default": [], })`, }, @@ -118,7 +118,31 @@ bootstrap_go_package { "//bar:bar", "//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, )},