From ba3ea16f14eb1017fd75aead089ca34b6f32406b Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 17 Feb 2021 13:22:03 -0500 Subject: [PATCH 1/2] Use handcrafted build targets in bp2build If both bp2build_available and label are specified, label will be preferred. Initially, we copy the entire BUILD.bazel file. Eventually we may move this to use bazel query for a more accurate result. Test: go test * Test: build/bazel/scripts/milestone-2/demo.sh full Test: GENERATE_BAZEL_FILES=true m nothing edit bionic/libc/tools/BUILD.bazel GENERATE_BAZEL_FILES=true m nothing and verify changes picked up Bug: 180516554 Change-Id: I43025583300e6b10d2c18032cd4a76237b578d59 --- android/bazel.go | 69 ++++++++++++- bazel/properties.go | 15 --- bp2build/Android.bp | 1 + bp2build/bp2build.go | 2 +- bp2build/build_conversion.go | 98 ++++++++++++++----- bp2build/build_conversion_test.go | 154 ++++++++++++++++++++++++++++++ bp2build/constants.go | 25 +++++ bp2build/conversion.go | 15 ++- bp2build/metrics.go | 6 +- bp2build/testing.go | 2 +- cmd/soong_build/main.go | 4 +- cmd/soong_build/queryview.go | 2 +- 12 files changed, 339 insertions(+), 54 deletions(-) create mode 100644 bp2build/constants.go diff --git a/android/bazel.go b/android/bazel.go index 9939bd5e2..09a2c3af9 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -14,19 +14,46 @@ package android -import "android/soong/bazel" +import ( + "fmt" + "io/ioutil" + "path/filepath" + "strings" + + "github.com/google/blueprint/proptools" +) + +type bazelModuleProperties struct { + // The label of the Bazel target replacing this Soong module. When run in conversion mode, this + // will import the handcrafted build target into the autogenerated file. Note: this may result in + // a conflict due to duplicate targets if bp2build_available is also set. + Label *string + + // If true, bp2build will generate the converted Bazel target for this module. Note: this may + // cause a conflict due to the duplicate targets if label is also set. + Bp2build_available bool +} + +// Properties contains common module properties for Bazel migration purposes. +type properties struct { + // In USE_BAZEL_ANALYSIS=1 mode, this represents the Bazel target replacing + // this Soong module. + Bazel_module bazelModuleProperties +} // BazelModuleBase contains the property structs with metadata for modules which can be converted to // Bazel. type BazelModuleBase struct { - bazelProperties bazel.Properties + bazelProperties properties } // Bazelable is specifies the interface for modules that can be converted to Bazel. type Bazelable interface { - bazelProps() *bazel.Properties + bazelProps() *properties + HasHandcraftedLabel() bool GetBazelLabel() string ConvertWithBp2build() bool + GetBazelBuildFileContents(c Config, path, name string) (string, error) } // BazelModule is a lightweight wrapper interface around Module for Bazel-convertible modules. @@ -42,16 +69,48 @@ func InitBazelModule(module BazelModule) { } // bazelProps returns the Bazel properties for the given BazelModuleBase. -func (b *BazelModuleBase) bazelProps() *bazel.Properties { +func (b *BazelModuleBase) bazelProps() *properties { return &b.bazelProperties } +// HasHandcraftedLabel returns whether this module has a handcrafted Bazel label. +func (b *BazelModuleBase) HasHandcraftedLabel() bool { + return b.bazelProperties.Bazel_module.Label != nil +} + +// HandcraftedLabel returns the handcrafted label for this module, or empty string if there is none +func (b *BazelModuleBase) HandcraftedLabel() string { + return proptools.String(b.bazelProperties.Bazel_module.Label) +} + // GetBazelLabel returns the Bazel label for the given BazelModuleBase. func (b *BazelModuleBase) GetBazelLabel() string { - return b.bazelProperties.Bazel_module.Label + return proptools.String(b.bazelProperties.Bazel_module.Label) } // ConvertWithBp2build returns whether the given BazelModuleBase should be converted with bp2build. func (b *BazelModuleBase) ConvertWithBp2build() bool { return b.bazelProperties.Bazel_module.Bp2build_available } + +// GetBazelBuildFileContents returns the file contents of a hand-crafted BUILD file if available or +// an error if there are errors reading the file. +// TODO(b/181575318): currently we append the whole BUILD file, let's change that to do +// something more targeted based on the rule type and target. +func (b *BazelModuleBase) GetBazelBuildFileContents(c Config, path, name string) (string, error) { + if !strings.Contains(b.GetBazelLabel(), path) { + return "", fmt.Errorf("%q not found in bazel_module.label %q", path, b.GetBazelLabel()) + } + name = filepath.Join(path, name) + f, err := c.fs.Open(name) + if err != nil { + return "", err + } + defer f.Close() + + data, err := ioutil.ReadAll(f) + if err != nil { + return "", err + } + return string(data[:]), nil +} diff --git a/bazel/properties.go b/bazel/properties.go index cb47758f9..abdc1077d 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -19,21 +19,6 @@ import ( "sort" ) -type bazelModuleProperties struct { - // The label of the Bazel target replacing this Soong module. - Label string - - // If true, bp2build will generate the converted Bazel target for this module. - Bp2build_available bool -} - -// Properties contains common module properties for Bazel migration purposes. -type Properties struct { - // In USE_BAZEL_ANALYSIS=1 mode, this represents the Bazel target replacing - // this Soong module. - Bazel_module bazelModuleProperties -} - // BazelTargetModuleProperties contain properties and metadata used for // Blueprint to BUILD file conversion. type BazelTargetModuleProperties struct { diff --git a/bp2build/Android.bp b/bp2build/Android.bp index 99d706cc5..c74f90232 100644 --- a/bp2build/Android.bp +++ b/bp2build/Android.bp @@ -11,6 +11,7 @@ bootstrap_go_package { "build_conversion.go", "bzl_conversion.go", "configurability.go", + "constants.go", "conversion.go", "metrics.go", ], diff --git a/bp2build/bp2build.go b/bp2build/bp2build.go index 7169d7e97..97a5137ee 100644 --- a/bp2build/bp2build.go +++ b/bp2build/bp2build.go @@ -22,7 +22,7 @@ import ( // The Bazel bp2build code generator is responsible for writing .bzl files that are equivalent to // Android.bp files that are capable of being built with Bazel. -func Codegen(ctx CodegenContext) CodegenMetrics { +func Codegen(ctx *CodegenContext) CodegenMetrics { outputDir := android.PathForOutput(ctx, "bp2build") android.RemoveAllOutputDir(outputDir) diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 7fa499683..962f278b8 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -99,9 +99,10 @@ type bpToBuildContext interface { } type CodegenContext struct { - config android.Config - context android.Context - mode CodegenMode + config android.Config + context android.Context + mode CodegenMode + additionalDeps []string } func (c *CodegenContext) Mode() CodegenMode { @@ -137,14 +138,26 @@ func (mode CodegenMode) String() string { } } -func (ctx CodegenContext) AddNinjaFileDeps(...string) {} -func (ctx CodegenContext) Config() android.Config { return ctx.config } -func (ctx CodegenContext) Context() android.Context { return ctx.context } +// AddNinjaFileDeps adds dependencies on the specified files to be added to the ninja manifest. The +// primary builder will be rerun whenever the specified files are modified. Allows us to fulfill the +// PathContext interface in order to add dependencies on hand-crafted BUILD files. Note: must also +// call AdditionalNinjaDeps and add them manually to the ninja file. +func (ctx *CodegenContext) AddNinjaFileDeps(deps ...string) { + ctx.additionalDeps = append(ctx.additionalDeps, deps...) +} + +// AdditionalNinjaDeps returns additional ninja deps added by CodegenContext +func (ctx *CodegenContext) AdditionalNinjaDeps() []string { + return ctx.additionalDeps +} + +func (ctx *CodegenContext) Config() android.Config { return ctx.config } +func (ctx *CodegenContext) Context() android.Context { return ctx.context } // NewCodegenContext creates a wrapper context that conforms to PathContext for // writing BUILD files in the output directory. -func NewCodegenContext(config android.Config, context android.Context, mode CodegenMode) CodegenContext { - return CodegenContext{ +func NewCodegenContext(config android.Config, context android.Context, mode CodegenMode) *CodegenContext { + return &CodegenContext{ context: context, config: config, mode: mode, @@ -163,12 +176,14 @@ func propsToAttributes(props map[string]string) string { return attributes } -func GenerateBazelTargets(ctx CodegenContext) (map[string]BazelTargets, CodegenMetrics) { +func GenerateBazelTargets(ctx *CodegenContext) (map[string]BazelTargets, CodegenMetrics) { buildFileToTargets := make(map[string]BazelTargets) + buildFileToAppend := make(map[string]bool) // Simple metrics tracking for bp2build - totalModuleCount := 0 - ruleClassCount := make(map[string]int) + metrics := CodegenMetrics{ + RuleClassCount: make(map[string]int), + } bpCtx := ctx.Context() bpCtx.VisitAllModules(func(m blueprint.Module) { @@ -177,13 +192,29 @@ func GenerateBazelTargets(ctx CodegenContext) (map[string]BazelTargets, CodegenM switch ctx.Mode() { case Bp2Build: - if b, ok := m.(android.BazelTargetModule); !ok { - // Only include regular Soong modules (non-BazelTargetModules) into the total count. - totalModuleCount += 1 - return + if b, ok := m.(android.Bazelable); ok && b.HasHandcraftedLabel() { + metrics.handCraftedTargetCount += 1 + metrics.TotalModuleCount += 1 + pathToBuildFile := getBazelPackagePath(b) + // We are using the entire contents of handcrafted build file, so if multiple targets within + // a package have handcrafted targets, we only want to include the contents one time. + if _, exists := buildFileToAppend[pathToBuildFile]; exists { + return + } + var err error + t, err = getHandcraftedBuildContent(ctx, b, pathToBuildFile) + if err != nil { + panic(fmt.Errorf("Error converting %s: %s", bpCtx.ModuleName(m), err)) + } + // TODO(b/181575318): currently we append the whole BUILD file, let's change that to do + // something more targeted based on the rule type and target + buildFileToAppend[pathToBuildFile] = true + } else if btm, ok := m.(android.BazelTargetModule); ok { + t = generateBazelTarget(bpCtx, m, btm) + metrics.RuleClassCount[t.ruleClass] += 1 } else { - t = generateBazelTarget(bpCtx, m, b) - ruleClassCount[t.ruleClass] += 1 + metrics.TotalModuleCount += 1 + return } case QueryView: // Blocklist certain module types from being generated. @@ -200,17 +231,34 @@ func GenerateBazelTargets(ctx CodegenContext) (map[string]BazelTargets, CodegenM buildFileToTargets[dir] = append(buildFileToTargets[dir], t) }) - metrics := CodegenMetrics{ - TotalModuleCount: totalModuleCount, - RuleClassCount: ruleClassCount, - } - return buildFileToTargets, metrics } -func generateBazelTarget(ctx bpToBuildContext, m blueprint.Module, b android.BazelTargetModule) BazelTarget { - ruleClass := b.RuleClass() - bzlLoadLocation := b.BzlLoadLocation() +func getBazelPackagePath(b android.Bazelable) string { + label := b.GetBazelLabel() + pathToBuildFile := strings.TrimPrefix(label, "//") + pathToBuildFile = strings.Split(pathToBuildFile, ":")[0] + return pathToBuildFile +} + +func getHandcraftedBuildContent(ctx *CodegenContext, b android.Bazelable, pathToBuildFile string) (BazelTarget, error) { + p := android.ExistentPathForSource(ctx, pathToBuildFile, HandcraftedBuildFileName) + if !p.Valid() { + return BazelTarget{}, fmt.Errorf("Could not find file %q for handcrafted target.", pathToBuildFile) + } + c, err := b.GetBazelBuildFileContents(ctx.Config(), pathToBuildFile, HandcraftedBuildFileName) + if err != nil { + return BazelTarget{}, err + } + // TODO(b/181575318): once this is more targeted, we need to include name, rule class, etc + return BazelTarget{ + content: c, + }, nil +} + +func generateBazelTarget(ctx bpToBuildContext, m blueprint.Module, btm android.BazelTargetModule) BazelTarget { + ruleClass := btm.RuleClass() + bzlLoadLocation := btm.BzlLoadLocation() // extract the bazel attributes from the module. props := getBuildProperties(ctx, m) diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index aa4fc1d97..89acbe96a 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -1221,3 +1221,157 @@ func TestAllowlistingBp2buildTargets(t *testing.T) { } } } + +func TestCombineBuildFilesBp2buildTargets(t *testing.T) { + testCases := []struct { + description string + moduleTypeUnderTest string + moduleTypeUnderTestFactory android.ModuleFactory + moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext) + preArchMutators []android.RegisterMutatorFunc + depsMutators []android.RegisterMutatorFunc + bp string + expectedBazelTargets []string + fs map[string]string + dir string + }{ + { + description: "filegroup bazel_module.label", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, + bp: `filegroup { + name: "fg_foo", + bazel_module: { label: "//other:fg_foo" }, +}`, + expectedBazelTargets: []string{ + `// BUILD file`, + }, + fs: map[string]string{ + "other/BUILD.bazel": `// BUILD file`, + }, + }, + { + description: "multiple bazel_module.label same BUILD", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, + bp: `filegroup { + name: "fg_foo", + bazel_module: { label: "//other:fg_foo" }, +} + +filegroup { + name: "foo", + bazel_module: { label: "//other:foo" }, +}`, + expectedBazelTargets: []string{ + `// BUILD file`, + }, + fs: map[string]string{ + "other/BUILD.bazel": `// BUILD file`, + }, + }, + { + description: "filegroup bazel_module.label and bp2build", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, + bp: `filegroup { + name: "fg_foo", + bazel_module: { + label: "//other:fg_foo", + bp2build_available: true, + }, +}`, + expectedBazelTargets: []string{ + `filegroup( + name = "fg_foo", +)`, + `// BUILD file`, + }, + fs: map[string]string{ + "other/BUILD.bazel": `// BUILD file`, + }, + }, + { + description: "filegroup bazel_module.label and filegroup bp2build", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, + bp: `filegroup { + name: "fg_foo", + bazel_module: { + label: "//other:fg_foo", + }, +} + +filegroup { + name: "fg_bar", + bazel_module: { + bp2build_available: true, + }, +}`, + expectedBazelTargets: []string{ + `filegroup( + name = "fg_bar", +)`, + `// BUILD file`, + }, + fs: map[string]string{ + "other/BUILD.bazel": `// BUILD file`, + }, + }, + } + + dir := "." + for _, testCase := range testCases { + fs := make(map[string][]byte) + toParse := []string{ + "Android.bp", + } + for f, content := range testCase.fs { + if strings.HasSuffix(f, "Android.bp") { + toParse = append(toParse, f) + } + fs[f] = []byte(content) + } + config := android.TestConfig(buildDir, nil, testCase.bp, fs) + ctx := android.NewTestContext(config) + ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory) + for _, m := range testCase.depsMutators { + ctx.DepsBp2BuildMutators(m) + } + ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator) + ctx.RegisterForBazelConversion() + + _, errs := ctx.ParseFileList(dir, toParse) + if Errored(t, testCase.description, errs) { + continue + } + _, errs = ctx.ResolveDependencies(config) + if Errored(t, testCase.description, errs) { + continue + } + + checkDir := dir + if testCase.dir != "" { + checkDir = testCase.dir + } + bazelTargets := generateBazelTargetsForDir(NewCodegenContext(config, *ctx.Context, Bp2Build), checkDir) + if actualCount, expectedCount := len(bazelTargets), len(testCase.expectedBazelTargets); actualCount != expectedCount { + t.Errorf("%s: Expected %d bazel target, got %d\n%s", testCase.description, expectedCount, actualCount, bazelTargets) + } else { + for i, target := range bazelTargets { + if w, g := testCase.expectedBazelTargets[i], target.content; w != g { + t.Errorf( + "%s: Expected generated Bazel target to be '%s', got '%s'", + testCase.description, + w, + g, + ) + } + } + } + } +} diff --git a/bp2build/constants.go b/bp2build/constants.go new file mode 100644 index 000000000..23bca83ad --- /dev/null +++ b/bp2build/constants.go @@ -0,0 +1,25 @@ +// Copyright 2020 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 + +var ( + // When both a BUILD and BUILD.bazel file are exist in the same package, the BUILD.bazel file will + // be preferred for use within a Bazel build. + + // The file name used for automatically generated files. Files with this name are ignored by git. + GeneratedBuildFileName = "BUILD" + // The file name used for hand-crafted build targets. + HandcraftedBuildFileName = "BUILD.bazel" +) diff --git a/bp2build/conversion.go b/bp2build/conversion.go index 1225f2bed..7877bb859 100644 --- a/bp2build/conversion.go +++ b/bp2build/conversion.go @@ -24,9 +24,9 @@ func CreateBazelFiles( // Write top level files: WORKSPACE and BUILD. These files are empty. files = append(files, newFile("", "WORKSPACE", "")) // Used to denote that the top level directory is a package. - files = append(files, newFile("", "BUILD", "")) + files = append(files, newFile("", GeneratedBuildFileName, "")) - files = append(files, newFile(bazelRulesSubDir, "BUILD", "")) + files = append(files, newFile(bazelRulesSubDir, GeneratedBuildFileName, "")) if mode == QueryView { // These files are only used for queryview. @@ -47,7 +47,14 @@ func createBuildFiles(buildToTargets map[string]BazelTargets, mode CodegenMode) files := make([]BazelFile, 0, len(buildToTargets)) for _, dir := range android.SortedStringKeys(buildToTargets) { targets := buildToTargets[dir] - sort.Slice(targets, func(i, j int) bool { return targets[i].name < targets[j].name }) + sort.Slice(targets, func(i, j int) bool { + // this will cover all bp2build generated targets + if targets[i].name < targets[j].name { + return true + } + // give a strict ordering to content from hand-crafted targets + return targets[i].content < targets[j].content + }) content := soongModuleLoad if mode == Bp2Build { content = `# This file was automatically generated by bp2build for the Bazel migration project. @@ -62,7 +69,7 @@ func createBuildFiles(buildToTargets map[string]BazelTargets, mode CodegenMode) content += "\n\n" } content += targets.String() - files = append(files, newFile(dir, "BUILD", content)) + files = append(files, newFile(dir, GeneratedBuildFileName, content)) } return files } diff --git a/bp2build/metrics.go b/bp2build/metrics.go index 916129fba..65b06c636 100644 --- a/bp2build/metrics.go +++ b/bp2build/metrics.go @@ -13,6 +13,9 @@ type CodegenMetrics struct { // Counts of generated Bazel targets per Bazel rule class RuleClassCount map[string]int + + // Total number of handcrafted targets + handCraftedTargetCount int } // Print the codegen metrics to stdout. @@ -24,7 +27,8 @@ func (metrics CodegenMetrics) Print() { generatedTargetCount += count } fmt.Printf( - "[bp2build] Generated %d total BUILD targets from %d Android.bp modules.\n", + "[bp2build] Generated %d total BUILD targets and included %d handcrafted BUILD targets from %d Android.bp modules.\n", generatedTargetCount, + metrics.handCraftedTargetCount, metrics.TotalModuleCount) } diff --git a/bp2build/testing.go b/bp2build/testing.go index bd75a8fc8..a15a4a552 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -175,7 +175,7 @@ func customBp2BuildMutatorFromStarlark(ctx android.TopDownMutatorContext) { } // Helper method for tests to easily access the targets in a dir. -func generateBazelTargetsForDir(codegenCtx CodegenContext, dir string) BazelTargets { +func generateBazelTargetsForDir(codegenCtx *CodegenContext, dir string) BazelTargets { buildFileToTargets, _ := GenerateBazelTargets(codegenCtx) return buildFileToTargets[dir] } diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 4586f44a6..8322fbe72 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -198,7 +198,6 @@ func runBp2Build(srcDir string, configuration android.Config) { if err != nil { panic(err) } - extraNinjaDepsString := strings.Join(extraNinjaDeps, " \\\n ") // Run the loading and analysis pipeline to prepare the graph of regular // Modules parsed from Android.bp files, and the BazelTargetModules mapped @@ -215,6 +214,9 @@ func runBp2Build(srcDir string, configuration android.Config) { // 1:1 mapping for each module. metrics.Print() + extraNinjaDeps = append(extraNinjaDeps, codegenContext.AdditionalNinjaDeps()...) + extraNinjaDepsString := strings.Join(extraNinjaDeps, " \\\n ") + // Workarounds to support running bp2build in a clean AOSP checkout with no // prior builds, and exiting early as soon as the BUILD files get generated, // therefore not creating build.ninja files that soong_ui and callers of diff --git a/cmd/soong_build/queryview.go b/cmd/soong_build/queryview.go index 0a77d67a9..edc8a422d 100644 --- a/cmd/soong_build/queryview.go +++ b/cmd/soong_build/queryview.go @@ -22,7 +22,7 @@ import ( "path/filepath" ) -func createBazelQueryView(ctx bp2build.CodegenContext, bazelQueryViewDir string) error { +func createBazelQueryView(ctx *bp2build.CodegenContext, bazelQueryViewDir string) error { ruleShims := bp2build.CreateRuleShims(android.ModuleTypeFactories()) // Ignore metrics reporting for queryview, since queryview is already a full-repo From bdc609972cbbe9480d14f4cbf87523b81ca35a99 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 24 Feb 2021 16:55:11 -0500 Subject: [PATCH 2/2] Support autoconverted modules in mixed builds modules converted with bp2build_available are will also be available to be used in mixed builds. Test: build/bazel/scripts/milestone-2/demo.sh full Test: go tests Change-Id: I49f16ec3ba5bb11dfed8066af069c27eb04371fb --- android/bazel.go | 25 +++++++++++++++---- android/paths.go | 47 ++++++++++++++++++++++++++++++------ bp2build/build_conversion.go | 2 +- cc/cc.go | 2 +- genrule/genrule.go | 2 +- 5 files changed, 62 insertions(+), 16 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 09a2c3af9..683495bd6 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -20,6 +20,7 @@ import ( "path/filepath" "strings" + "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -51,9 +52,11 @@ type BazelModuleBase struct { type Bazelable interface { bazelProps() *properties HasHandcraftedLabel() bool - GetBazelLabel() string + HandcraftedLabel() string + GetBazelLabel(ctx BazelConversionPathContext, module blueprint.Module) string ConvertWithBp2build() bool GetBazelBuildFileContents(c Config, path, name string) (string, error) + ConvertedToBazel() bool } // BazelModule is a lightweight wrapper interface around Module for Bazel-convertible modules. @@ -84,8 +87,14 @@ func (b *BazelModuleBase) HandcraftedLabel() string { } // GetBazelLabel returns the Bazel label for the given BazelModuleBase. -func (b *BazelModuleBase) GetBazelLabel() string { - return proptools.String(b.bazelProperties.Bazel_module.Label) +func (b *BazelModuleBase) GetBazelLabel(ctx BazelConversionPathContext, module blueprint.Module) string { + if b.HasHandcraftedLabel() { + return b.HandcraftedLabel() + } + if b.ConvertWithBp2build() { + return bp2buildModuleLabel(ctx, module) + } + return "" // no label for unconverted module } // ConvertWithBp2build returns whether the given BazelModuleBase should be converted with bp2build. @@ -98,8 +107,8 @@ func (b *BazelModuleBase) ConvertWithBp2build() bool { // TODO(b/181575318): currently we append the whole BUILD file, let's change that to do // something more targeted based on the rule type and target. func (b *BazelModuleBase) GetBazelBuildFileContents(c Config, path, name string) (string, error) { - if !strings.Contains(b.GetBazelLabel(), path) { - return "", fmt.Errorf("%q not found in bazel_module.label %q", path, b.GetBazelLabel()) + if !strings.Contains(b.HandcraftedLabel(), path) { + return "", fmt.Errorf("%q not found in bazel_module.label %q", path, b.HandcraftedLabel()) } name = filepath.Join(path, name) f, err := c.fs.Open(name) @@ -114,3 +123,9 @@ func (b *BazelModuleBase) GetBazelBuildFileContents(c Config, path, name string) } return string(data[:]), nil } + +// ConvertedToBazel returns whether this module has been converted to Bazel, whether automatically +// or manually +func (b *BazelModuleBase) ConvertedToBazel() bool { + return b.ConvertWithBp2build() || b.HasHandcraftedLabel() +} diff --git a/android/paths.go b/android/paths.go index 3f4d3f281..f648c557b 100644 --- a/android/paths.go +++ b/android/paths.go @@ -339,6 +339,7 @@ type BazelConversionPathContext interface { EarlyModulePathContext GetDirectDep(name string) (blueprint.Module, blueprint.DependencyTag) + Module() Module OtherModuleName(m blueprint.Module) string OtherModuleDir(m blueprint.Module) string } @@ -434,15 +435,45 @@ func expandSrcsForBazel(ctx BazelConversionPathContext, paths, expandedExcludes // already be resolved by either deps mutator or path deps mutator. func getOtherModuleLabel(ctx BazelConversionPathContext, dep, tag string) bazel.Label { m, _ := ctx.GetDirectDep(dep) - // TODO(b/165114590): Convert tag (":name{.tag}") to corresponding Bazel implicit output targets. - otherModuleName := ctx.OtherModuleName(m) - var label bazel.Label - if otherDir, dir := ctx.OtherModuleDir(m), ctx.ModuleDir(); otherDir != dir { - label.Label = fmt.Sprintf("//%s:%s", otherDir, otherModuleName) - } else { - label.Label = fmt.Sprintf(":%s", otherModuleName) + otherLabel := bazelModuleLabel(ctx, m, tag) + label := bazelModuleLabel(ctx, ctx.Module(), "") + if samePackage(label, otherLabel) { + otherLabel = bazelShortLabel(otherLabel) } - return label + + return bazel.Label{ + Label: otherLabel, + } +} + +func bazelModuleLabel(ctx BazelConversionPathContext, module blueprint.Module, tag string) string { + // TODO(b/165114590): Convert tag (":name{.tag}") to corresponding Bazel implicit output targets. + b, ok := module.(Bazelable) + // TODO(b/181155349): perhaps return an error here if the module can't be/isn't being converted + if !ok || !b.ConvertedToBazel() { + return bp2buildModuleLabel(ctx, module) + } + return b.GetBazelLabel(ctx, module) +} + +func bazelShortLabel(label string) string { + i := strings.Index(label, ":") + return label[i:] +} + +func bazelPackage(label string) string { + i := strings.Index(label, ":") + return label[0:i] +} + +func samePackage(label1, label2 string) bool { + return bazelPackage(label1) == bazelPackage(label2) +} + +func bp2buildModuleLabel(ctx BazelConversionPathContext, module blueprint.Module) string { + moduleName := ctx.OtherModuleName(module) + moduleDir := ctx.OtherModuleDir(module) + return fmt.Sprintf("//%s:%s", moduleDir, moduleName) } // OutputPaths is a slice of OutputPath objects, with helpers to operate on the collection. diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 962f278b8..9c98c7661 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -235,7 +235,7 @@ func GenerateBazelTargets(ctx *CodegenContext) (map[string]BazelTargets, Codegen } func getBazelPackagePath(b android.Bazelable) string { - label := b.GetBazelLabel() + label := b.HandcraftedLabel() pathToBuildFile := strings.TrimPrefix(label, "//") pathToBuildFile = strings.Split(pathToBuildFile, ":")[0] return pathToBuildFile diff --git a/cc/cc.go b/cc/cc.go index c335dac36..cab7459ed 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1605,7 +1605,7 @@ func (c *Module) setSubnameProperty(actx android.ModuleContext) { // Returns true if Bazel was successfully used for the analysis of this module. func (c *Module) maybeGenerateBazelActions(actx android.ModuleContext) bool { - bazelModuleLabel := c.GetBazelLabel() + bazelModuleLabel := c.GetBazelLabel(actx, c) bazelActionsUsed := false if c.bazelHandler != nil && actx.Config().BazelContext.BazelEnabled() && len(bazelModuleLabel) > 0 { bazelActionsUsed = c.bazelHandler.generateBazelBuildActions(actx, bazelModuleLabel) diff --git a/genrule/genrule.go b/genrule/genrule.go index 53499068c..4d90433aa 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -538,7 +538,7 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { g.outputFiles = outputFiles.Paths() - bazelModuleLabel := g.GetBazelLabel() + bazelModuleLabel := g.GetBazelLabel(ctx, g) bazelActionsUsed := false if ctx.Config().BazelContext.BazelEnabled() && len(bazelModuleLabel) > 0 { bazelActionsUsed = g.generateBazelBuildActions(ctx, bazelModuleLabel)