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
diff --git a/android/bazel.go b/android/bazel.go
index 9939bd5..09a2c3a 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 @@
}
// 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 cb47758..abdc107 100644
--- a/bazel/properties.go
+++ b/bazel/properties.go
@@ -19,21 +19,6 @@
"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 99d706c..c74f902 100644
--- a/bp2build/Android.bp
+++ b/bp2build/Android.bp
@@ -11,6 +11,7 @@
"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 7169d7e..97a5137 100644
--- a/bp2build/bp2build.go
+++ b/bp2build/bp2build.go
@@ -22,7 +22,7 @@
// 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 7fa4996..962f278 100644
--- a/bp2build/build_conversion.go
+++ b/bp2build/build_conversion.go
@@ -99,9 +99,10 @@
}
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 (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 @@
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 @@
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 @@
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 aa4fc1d..89acbe9 100644
--- a/bp2build/build_conversion_test.go
+++ b/bp2build/build_conversion_test.go
@@ -1221,3 +1221,157 @@
}
}
}
+
+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 0000000..23bca83
--- /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 1225f2b..7877bb8 100644
--- a/bp2build/conversion.go
+++ b/bp2build/conversion.go
@@ -24,9 +24,9 @@
// 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 @@
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 @@
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 916129f..65b06c6 100644
--- a/bp2build/metrics.go
+++ b/bp2build/metrics.go
@@ -13,6 +13,9 @@
// 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 @@
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 bd75a8f..a15a4a5 100644
--- a/bp2build/testing.go
+++ b/bp2build/testing.go
@@ -175,7 +175,7 @@
}
// 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 4586f44..8322fbe 100644
--- a/cmd/soong_build/main.go
+++ b/cmd/soong_build/main.go
@@ -198,7 +198,6 @@
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 @@
// 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 0a77d67..edc8a42 100644
--- a/cmd/soong_build/queryview.go
+++ b/cmd/soong_build/queryview.go
@@ -22,7 +22,7 @@
"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