From ea2abba3a90d9459761bb7021995c2c54e34c0b0 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 14 Jun 2023 21:30:38 +0000 Subject: [PATCH 1/4] Partial bp2build conversion of bootstratp_go_package This module type does not implement android.Module, and therefore we cannot write a conventional bp2build converter for this module type. Instead this has been special-cased inside bp2build/build_conversion.go. Because of the above, we also do not have access to useful functions available in the ctx object of ConvertWithBp2build. This includes 1. Finding the package (directory) of a dep. This requires getting a handle of the underlying module from its name (string). To solve, we do a pre-visit to collect this information. This did not increase the wall time. On my machine, `m bp2build --skip-soong-tests` takes ~14s before and after this CL 2. Converting srcs to labels. This requires glob and package boundary resolution. This CL introduces a partial implementation for this function. (glob patterns are not used in go tools) For (1), I considered creating a `ModuleFromName` on `blueprint.Context` instead of a pre-run, but this increased the time to ~27s. Test: unit tests Test: TH Bug: 284483729 Change-Id: Ifeb029103d14947352556dba295591dd7038b090 --- bp2build/Android.bp | 2 + bp2build/build_conversion.go | 164 ++++++++++++++++++++++++++++++++- bp2build/go_conversion_test.go | 89 ++++++++++++++++++ 3 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 bp2build/go_conversion_test.go diff --git a/bp2build/Android.bp b/bp2build/Android.bp index 782a88c72..f889693c4 100644 --- a/bp2build/Android.bp +++ b/bp2build/Android.bp @@ -19,6 +19,7 @@ bootstrap_go_package { "testing.go", ], deps: [ + "blueprint-bootstrap", "soong-aidl-library", "soong-android", "soong-android-allowlists", @@ -37,6 +38,7 @@ bootstrap_go_package { "soong-ui-metrics", ], testSrcs: [ + "go_conversion_test.go", "aar_conversion_test.go", "aidl_library_conversion_test.go", "android_app_certificate_conversion_test.go", diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 46a5bd8cb..e1252b2d8 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -30,6 +30,7 @@ import ( "android/soong/starlark_fmt" "android/soong/ui/metrics/bp2build_metrics_proto" "github.com/google/blueprint" + "github.com/google/blueprint/bootstrap" "github.com/google/blueprint/proptools" ) @@ -252,6 +253,157 @@ func (r conversionResults) BuildDirToTargets() map[string]BazelTargets { return r.buildFileToTargets } +// struct to store state of go bazel targets +// this implements bp2buildModule interface and is passed to generateBazelTargets +type goBazelTarget struct { + targetName string + targetPackage string + bazelRuleClass string + bazelRuleLoadLocation string + bazelAttributes []interface{} +} + +var _ bp2buildModule = (*goBazelTarget)(nil) + +func (g goBazelTarget) TargetName() string { + return g.targetName +} + +func (g goBazelTarget) TargetPackage() string { + return g.targetPackage +} + +func (g goBazelTarget) BazelRuleClass() string { + return g.bazelRuleClass +} + +func (g goBazelTarget) BazelRuleLoadLocation() string { + return g.bazelRuleLoadLocation +} + +func (g goBazelTarget) BazelAttributes() []interface{} { + return g.bazelAttributes +} + +// Creates a target_compatible_with entry that is *not* compatible with android +func targetNotCompatibleWithAndroid() bazel.LabelListAttribute { + ret := bazel.LabelListAttribute{} + ret.SetSelectValue(bazel.OsConfigurationAxis, bazel.OsAndroid, + bazel.MakeLabelList( + []bazel.Label{ + bazel.Label{ + Label: "@platforms//:incompatible", + }, + }, + ), + ) + return ret +} + +// 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. +// +// Prefer to use `BazelLabelForModuleSrc` instead +func goSrcLabels(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 + } + ret = append(ret, srcLabel) + } + return bazel.MakeLabelList(ret) + } + + ret := bazel.LabelListAttribute{} + // common + ret.SetSelectValue(bazel.NoConfigAxis, "", labels(srcs)) + // linux + ret.SetSelectValue(bazel.OsConfigurationAxis, bazel.OsLinux, labels(linuxSrcs)) + // darwin + ret.SetSelectValue(bazel.OsConfigurationAxis, bazel.OsDarwin, labels(darwinSrcs)) + return ret +} + +func goDepLabels(deps []string, goModulesMap nameToGoLibraryModule) bazel.LabelListAttribute { + labels := []bazel.Label{} + for _, dep := range deps { + moduleDir := goModulesMap[dep].Dir + if moduleDir == "." { + moduleDir = "" + } + label := bazel.Label{ + Label: fmt.Sprintf("//%s:%s", moduleDir, dep), + } + labels = append(labels, label) + } + return bazel.MakeLabelListAttribute(bazel.MakeLabelList(labels)) +} + +// attributes common to blueprint_go_binary and bootstap_go_package +type goAttributes struct { + Importpath bazel.StringAttribute + Srcs bazel.LabelListAttribute + Deps bazel.LabelListAttribute + Target_compatible_with bazel.LabelListAttribute +} + +func generateBazelTargetsGoPackage(ctx *android.Context, g *bootstrap.GoPackage, goModulesMap nameToGoLibraryModule) ([]BazelTarget, []error) { + ca := android.CommonAttributes{ + Name: g.Name(), + } + ga := goAttributes{ + Importpath: bazel.StringAttribute{ + Value: proptools.StringPtr(g.GoPkgPath()), + }, + Srcs: goSrcLabels(g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), + Deps: goDepLabels(g.Deps(), goModulesMap), + Target_compatible_with: targetNotCompatibleWithAndroid(), + } + + lib := goBazelTarget{ + targetName: g.Name(), + targetPackage: ctx.ModuleDir(g), + bazelRuleClass: "go_library", + bazelRuleLoadLocation: "@io_bazel_rules_go//go:def.bzl", + bazelAttributes: []interface{}{&ca, &ga}, + } + // TODO - b/284483729: Create go_test target from testSrcs + libTarget, err := generateBazelTarget(ctx, lib) + if err != nil { + return []BazelTarget{}, []error{err} + } + return []BazelTarget{libTarget}, nil +} + +type goLibraryModule struct { + Dir string + Deps []string +} + +type nameToGoLibraryModule map[string]goLibraryModule + +// Visit each module in the graph +// If a module is of type `bootstrap_go_package`, return a map containing metadata like its dir and deps +func createGoLibraryModuleMap(ctx *android.Context) nameToGoLibraryModule { + ret := nameToGoLibraryModule{} + ctx.VisitAllModules(func(m blueprint.Module) { + moduleType := ctx.ModuleType(m) + // We do not need to store information about blueprint_go_binary since it does not have any rdeps + if moduleType == "bootstrap_go_package" { + ret[m.Name()] = goLibraryModule{ + Dir: ctx.ModuleDir(m), + Deps: m.(*bootstrap.GoPackage).Deps(), + } + } + }) + return ret +} + func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (conversionResults, []error) { buildFileToTargets := make(map[string]BazelTargets) @@ -262,6 +414,10 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers var errs []error + // Visit go libraries in a pre-run and store its state in a map + // The time complexity remains O(N), and this does not add significant wall time. + nameToGoLibMap := createGoLibraryModuleMap(ctx.Context()) + bpCtx := ctx.Context() bpCtx.VisitAllModules(func(m blueprint.Module) { dir := bpCtx.ModuleDir(m) @@ -269,6 +425,7 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers dirs[dir] = true var targets []BazelTarget + var targetErrs []error switch ctx.Mode() { case Bp2Build: @@ -317,7 +474,6 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers return } } - var targetErrs []error targets, targetErrs = generateBazelTargets(bpCtx, aModule) errs = append(errs, targetErrs...) for _, t := range targets { @@ -336,6 +492,12 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers metrics.AddUnconvertedModule(m, moduleType, dir, *reason) } return + } else if glib, ok := m.(*bootstrap.GoPackage); ok { + targets, targetErrs = generateBazelTargetsGoPackage(bpCtx, glib, nameToGoLibMap) + errs = append(errs, targetErrs...) + metrics.IncrementRuleClassCount("go_library") + } else if _, ok := m.(*bootstrap.GoBinary); ok { + // TODO - b/284483729: Create bazel targets for go binaries } else { metrics.AddUnconvertedModule(m, moduleType, dir, android.UnconvertedReason{ ReasonType: int(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED), diff --git a/bp2build/go_conversion_test.go b/bp2build/go_conversion_test.go new file mode 100644 index 000000000..13d25137b --- /dev/null +++ b/bp2build/go_conversion_test.go @@ -0,0 +1,89 @@ +// Copyright 2023 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bp2build + +import ( + "testing" + + "github.com/google/blueprint/bootstrap" + + "android/soong/android" +) + +func runGoTests(t *testing.T, tc Bp2buildTestCase) { + RunBp2BuildTestCase(t, func(ctx android.RegistrationContext) { + tCtx := ctx.(*android.TestContext) + bootstrap.RegisterGoModuleTypes(tCtx.Context.Context) // android.TestContext --> android.Context --> blueprint.Context + }, tc) +} + +func TestConvertGoPackage(t *testing.T) { + bp := ` +bootstrap_go_package { + name: "foo", + pkgPath: "android/foo", + deps: [ + "bar", + ], + srcs: [ + "foo1.go", + "foo2.go", + ], + linux: { + srcs: [ + "foo_linux.go", + ], + }, + darwin: { + srcs: [ + "foo_darwin.go", + ], + }, + testSrcs: [ + "foo1_test.go", + "foo2_test.go", + ], +} +` + depBp := ` +bootstrap_go_package { + name: "bar", +} +` + t.Parallel() + runGoTests(t, Bp2buildTestCase{ + Description: "Convert bootstrap_go_package to go_library", + ModuleTypeUnderTest: "bootrstap_go_package", + Blueprint: bp, + Filesystem: map[string]string{ + "bar/Android.bp": depBp, // Put dep in Android.bp to reduce boilerplate in ExpectedBazelTargets + }, + ExpectedBazelTargets: []string{makeBazelTargetHostOrDevice("go_library", "foo", + AttrNameToString{ + "deps": `["//bar:bar"]`, + "importpath": `"android/foo"`, + "srcs": `[ + ":foo1.go", + ":foo2.go", + ] + select({ + "//build/bazel/platforms/os:darwin": [":foo_darwin.go"], + "//build/bazel/platforms/os:linux_glibc": [":foo_linux.go"], + "//conditions:default": [], + })`, + }, + android.HostSupported, + )}, + }) +} From de623294fe87ca64c8afe3937bb75eabc6bf43cc Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 14 Jun 2023 21:30:38 +0000 Subject: [PATCH 2/4] Partial bp2build conversion of blueprint_go_binary This module type does not implement android.Module, and therefore we cannot write a conventional bp2build converter for this module type. Instead this has been special-cased inside bp2build/build_conversion.go. There is one major deviation between Soong and Bazel's go_binary/go_library. Soong collects the deps in the transitve closure and puts them on compile/link paths. Bazel OTOH, requires the direct imports to be listed in deps of the binary explicitly (AFAIK). Since bp2build cannot determine the list of direct imports from the list of transitive deps, put all the transitive deps in `deps` Test: unit tests Test: TH Bug: 284483729 Change-Id: I004aaf8607fef1697a0d9e7d018ad657b67778ac --- bp2build/build_conversion.go | 68 ++++++++++++++++++++++++++++++++-- bp2build/go_conversion_test.go | 37 ++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index e1252b2d8..265a368ce 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -356,12 +356,23 @@ func generateBazelTargetsGoPackage(ctx *android.Context, g *bootstrap.GoPackage, ca := android.CommonAttributes{ Name: g.Name(), } + + // For this bootstrap_go_package dep chain, + // A --> B --> C ( ---> depends on) + // Soong provides the convenience of only listing B as deps of A even if a src file of A imports C + // Bazel OTOH + // 1. requires C to be listed in `deps` expllicity. + // 2. does not require C to be listed if src of A does not import C + // + // bp2build does not have sufficient info on whether C is a direct dep of A or not, so for now collect all transitive deps and add them to deps + transitiveDeps := transitiveGoDeps(g.Deps(), goModulesMap) + ga := goAttributes{ Importpath: bazel.StringAttribute{ Value: proptools.StringPtr(g.GoPkgPath()), }, Srcs: goSrcLabels(g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), - Deps: goDepLabels(g.Deps(), goModulesMap), + Deps: goDepLabels(transitiveDeps, goModulesMap), Target_compatible_with: targetNotCompatibleWithAndroid(), } @@ -404,6 +415,55 @@ func createGoLibraryModuleMap(ctx *android.Context) nameToGoLibraryModule { return ret } +// Returns the deps in the transitive closure of a go target +func transitiveGoDeps(directDeps []string, goModulesMap nameToGoLibraryModule) []string { + allDeps := directDeps + i := 0 + for i < len(allDeps) { + curr := allDeps[i] + allDeps = append(allDeps, goModulesMap[curr].Deps...) + i += 1 + } + allDeps = android.SortedUniqueStrings(allDeps) + return allDeps +} + +func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, goModulesMap nameToGoLibraryModule) ([]BazelTarget, []error) { + ca := android.CommonAttributes{ + Name: g.Name(), + } + + // For this bootstrap_go_package dep chain, + // A --> B --> C ( ---> depends on) + // Soong provides the convenience of only listing B as deps of A even if a src file of A imports C + // Bazel OTOH + // 1. requires C to be listed in `deps` expllicity. + // 2. does not require C to be listed if src of A does not import C + // + // bp2build does not have sufficient info on whether C is a direct dep of A or not, so for now collect all transitive deps and add them to deps + transitiveDeps := transitiveGoDeps(g.Deps(), goModulesMap) + + ga := goAttributes{ + Srcs: goSrcLabels(g.Srcs(), g.LinuxSrcs(), g.DarwinSrcs()), + Deps: goDepLabels(transitiveDeps, goModulesMap), + Target_compatible_with: targetNotCompatibleWithAndroid(), + } + + bin := goBazelTarget{ + targetName: g.Name(), + targetPackage: ctx.ModuleDir(g), + bazelRuleClass: "go_binary", + bazelRuleLoadLocation: "@io_bazel_rules_go//go:def.bzl", + bazelAttributes: []interface{}{&ca, &ga}, + } + // TODO - b/284483729: Create go_test target from testSrcs + binTarget, err := generateBazelTarget(ctx, bin) + if err != nil { + return []BazelTarget{}, []error{err} + } + return []BazelTarget{binTarget}, nil +} + func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (conversionResults, []error) { buildFileToTargets := make(map[string]BazelTargets) @@ -496,8 +556,10 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers targets, targetErrs = generateBazelTargetsGoPackage(bpCtx, glib, nameToGoLibMap) errs = append(errs, targetErrs...) metrics.IncrementRuleClassCount("go_library") - } else if _, ok := m.(*bootstrap.GoBinary); ok { - // TODO - b/284483729: Create bazel targets for go binaries + } else if gbin, ok := m.(*bootstrap.GoBinary); ok { + targets, targetErrs = generateBazelTargetsGoBinary(bpCtx, gbin, nameToGoLibMap) + errs = append(errs, targetErrs...) + metrics.IncrementRuleClassCount("go_binary") } else { metrics.AddUnconvertedModule(m, moduleType, dir, android.UnconvertedReason{ ReasonType: int(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED), diff --git a/bp2build/go_conversion_test.go b/bp2build/go_conversion_test.go index 13d25137b..a872fc7db 100644 --- a/bp2build/go_conversion_test.go +++ b/bp2build/go_conversion_test.go @@ -87,3 +87,40 @@ bootstrap_go_package { )}, }) } + +func TestConvertGoBinaryWithTransitiveDeps(t *testing.T) { + bp := ` +blueprint_go_binary { + name: "foo", + srcs: ["main.go"], + deps: ["bar"], +} +` + depBp := ` +bootstrap_go_package { + name: "bar", + deps: ["baz"], +} +bootstrap_go_package { + name: "baz", +} +` + t.Parallel() + runGoTests(t, Bp2buildTestCase{ + Description: "Convert blueprint_go_binary to go_binary", + Blueprint: bp, + Filesystem: map[string]string{ + "bar/Android.bp": depBp, // Put dep in Android.bp to reduce boilerplate in ExpectedBazelTargets + }, + ExpectedBazelTargets: []string{makeBazelTargetHostOrDevice("go_binary", "foo", + AttrNameToString{ + "deps": `[ + "//bar:bar", + "//bar:baz", + ]`, + "srcs": `[":main.go"]`, + }, + android.HostSupported, + )}, + }) +} From 69afa98fbd19500786db96554bc6347dab07e557 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 11 Jul 2023 22:22:17 +0000 Subject: [PATCH 3/4] Create a temporary denylist for go binaries used in mixed builds This allows us to rollout building _some_ go targets using rules_go without affecting mixed builds. Test: Presubmit Bug: 284483729 Change-Id: I0ccb4c9b90614369147a380f44f7ae372ef9396e --- bp2build/build_conversion.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 265a368ce..9921a7f23 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -464,6 +464,17 @@ func generateBazelTargetsGoBinary(ctx *android.Context, g *bootstrap.GoBinary, g return []BazelTarget{binTarget}, nil } +var ( + // TODO - b/284483729: Remove this denyilst + // Temporary denylist of go binaries that are currently used in mixed builds + // This denylist allows us to rollout bp2build converters for go targets without affecting mixed builds + goBinaryDenylist = []string{ + "soong_zip", + "zip2zip", + "bazel_notice_gen", + } +) + func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (conversionResults, []error) { buildFileToTargets := make(map[string]BazelTargets) @@ -556,7 +567,7 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers targets, targetErrs = generateBazelTargetsGoPackage(bpCtx, glib, nameToGoLibMap) errs = append(errs, targetErrs...) metrics.IncrementRuleClassCount("go_library") - } else if gbin, ok := m.(*bootstrap.GoBinary); ok { + } else if gbin, ok := m.(*bootstrap.GoBinary); ok && !android.InList(m.Name(), goBinaryDenylist) { targets, targetErrs = generateBazelTargetsGoBinary(bpCtx, gbin, nameToGoLibMap) errs = append(errs, targetErrs...) metrics.IncrementRuleClassCount("go_binary") From 0a8a27500ef49c16a0f53963ce27eb90efe1b11d Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 21 Jun 2023 01:50:33 +0000 Subject: [PATCH 4/4] 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, )},