From dbcb1ff46935e4fa848dbe5285a71479492d9ca4 Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Thu, 10 Dec 2020 17:19:18 -0500 Subject: [PATCH] Use aquery to declare bazel actions in the ninja file. This effectively moves execution of Bazel actions outside of soong_build and alongside ninja execution of the actual ninja files, whether that be by ninja or by Bazel itself. This almost allows for mixed builds and Bazel-as-Ninja-executor to coexist, but requires hacks explained in b/175307058. Test: Treehugger Test: lunch aosp_flame && USE_BAZEL_ANALYSIS=1 m libc Test: lunch aosp_flame && USE_BAZEL=1 USE_BAZEL_ANALYSIS=1 m libc, though this requires a hack of the main BUILD file. See b/175307058 Change-Id: Ia2f6b0f1057e8cea3809de66d8287f13d84b510c --- android/bazel_handler.go | 133 ++++++++++++++++++++++++++++++++++----- android/paths.go | 29 +++++++++ bazel/Android.bp | 4 ++ bazel/aquery.go | 116 ++++++++++++++++++++++++++++++++++ genrule/genrule.go | 2 +- genrule/genrule_test.go | 3 +- 6 files changed, 268 insertions(+), 19 deletions(-) create mode 100644 bazel/aquery.go diff --git a/android/bazel_handler.go b/android/bazel_handler.go index d810726eb..81ca475ef 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -26,9 +26,10 @@ import ( "strings" "sync" + "github.com/google/blueprint/bootstrap" + "android/soong/bazel" "android/soong/shared" - "github.com/google/blueprint/bootstrap" ) type CqueryRequestType int @@ -60,6 +61,12 @@ type BazelContext interface { // Returns true if bazel is enabled for the given configuration. BazelEnabled() bool + + // Returns the bazel output base (the root directory for all bazel intermediate outputs). + OutputBase() string + + // Returns build statements which should get registered to reflect Bazel's outputs. + BuildStatementsToRegister() []bazel.BuildStatement } // A context object which tracks queued requests that need to be made to Bazel, @@ -76,6 +83,9 @@ type bazelContext struct { requestMutex sync.Mutex // requests can be written in parallel results map[cqueryKey]string // Results of cquery requests after Bazel invocations + + // Build statements which should get registered to reflect Bazel's outputs. + buildStatements []bazel.BuildStatement } var _ BazelContext = &bazelContext{} @@ -103,6 +113,14 @@ func (m MockBazelContext) BazelEnabled() bool { return true } +func (m MockBazelContext) OutputBase() string { + return "outputbase" +} + +func (m MockBazelContext) BuildStatementsToRegister() []bazel.BuildStatement { + return []bazel.BuildStatement{} +} + var _ BazelContext = MockBazelContext{} func (bazelCtx *bazelContext) GetAllFiles(label string) ([]string, bool) { @@ -123,10 +141,18 @@ func (n noopBazelContext) InvokeBazel() error { panic("unimplemented") } +func (m noopBazelContext) OutputBase() string { + return "" +} + func (n noopBazelContext) BazelEnabled() bool { return false } +func (m noopBazelContext) BuildStatementsToRegister() []bazel.BuildStatement { + return []bazel.BuildStatement{} +} + func NewBazelContext(c *config) (BazelContext, error) { // TODO(cparsons): Assess USE_BAZEL=1 instead once "mixed Soong/Bazel builds" // are production ready. @@ -241,14 +267,32 @@ local_repository( func (context *bazelContext) mainBzlFileContents() []byte { contents := ` +##################################################### # This file is generated by soong_build. Do not edit. +##################################################### + def _mixed_build_root_impl(ctx): return [DefaultInfo(files = depset(ctx.files.deps))] +# Rule representing the root of the build, to depend on all Bazel targets that +# are required for the build. Building this target will build the entire Bazel +# build tree. mixed_build_root = rule( implementation = _mixed_build_root_impl, attrs = {"deps" : attr.label_list()}, ) + +def _phony_root_impl(ctx): + return [] + +# Rule to depend on other targets but build nothing. +# This is useful as follows: building a target of this rule will generate +# symlink forests for all dependencies of the target, without executing any +# actions of the build. +phony_root = rule( + implementation = _phony_root_impl, + attrs = {"deps" : attr.label_list()}, +) ` return []byte(contents) } @@ -268,11 +312,15 @@ func canonicalizeLabel(label string) string { func (context *bazelContext) mainBuildFileContents() []byte { formatString := ` # This file is generated by soong_build. Do not edit. -load(":main.bzl", "mixed_build_root") +load(":main.bzl", "mixed_build_root", "phony_root") mixed_build_root(name = "buildroot", deps = [%s], ) + +phony_root(name = "phonyroot", + deps = [":buildroot"], +) ` var buildRootDeps []string = nil for val, _ := range context.requests { @@ -379,22 +427,46 @@ func (context *bazelContext) InvokeBazel() error { } } - // Issue a build command. - // TODO(cparsons): Invoking bazel execution during soong_build should be avoided; - // bazel actions should either be added to the Ninja file and executed later, - // or bazel should handle execution. + // Issue an aquery command to retrieve action information about the bazel build tree. + // // TODO(cparsons): Use --target_pattern_file to avoid command line limits. - _, err = context.issueBazelCommand(bazel.BazelBuildPhonyRootRunName, "build", []string{buildroot_label}) + var aqueryOutput string + aqueryOutput, err = context.issueBazelCommand(bazel.AqueryBuildRootRunName, "aquery", + []string{fmt.Sprintf("deps(%s)", buildroot_label), + // Use jsonproto instead of proto; actual proto parsing would require a dependency on Bazel's + // proto sources, which would add a number of unnecessary dependencies. + "--output=jsonproto"}) if err != nil { return err } + context.buildStatements = bazel.AqueryBuildStatements([]byte(aqueryOutput)) + + // Issue a build command of the phony root to generate symlink forests for dependencies of the + // Bazel build. This is necessary because aquery invocations do not generate this symlink forest, + // but some of symlinks may be required to resolve source dependencies of the build. + _, err = context.issueBazelCommand(bazel.BazelBuildPhonyRootRunName, "build", + []string{"//:phonyroot"}) + + if err != nil { + return err + } + + fmt.Printf("Build statements %s", context.buildStatements) // Clear requests. context.requests = map[cqueryKey]bool{} return nil } +func (context *bazelContext) BuildStatementsToRegister() []bazel.BuildStatement { + return context.buildStatements +} + +func (context *bazelContext) OutputBase() string { + return context.outputBase +} + // Singleton used for registering BUILD file ninja dependencies (needed // for correctness of builds which use Bazel. func BazelSingleton() Singleton { @@ -404,18 +476,45 @@ func BazelSingleton() Singleton { type bazelSingleton struct{} func (c *bazelSingleton) GenerateBuildActions(ctx SingletonContext) { - if ctx.Config().BazelContext.BazelEnabled() { - bazelBuildList := absolutePath(filepath.Join( - filepath.Dir(bootstrap.ModuleListFile), "bazel.list")) - ctx.AddNinjaFileDeps(bazelBuildList) + // bazelSingleton is a no-op if mixed-soong-bazel-builds are disabled. + if !ctx.Config().BazelContext.BazelEnabled() { + return + } - data, err := ioutil.ReadFile(bazelBuildList) - if err != nil { - ctx.Errorf(err.Error()) + // Add ninja file dependencies for files which all bazel invocations require. + bazelBuildList := absolutePath(filepath.Join( + filepath.Dir(bootstrap.ModuleListFile), "bazel.list")) + ctx.AddNinjaFileDeps(bazelBuildList) + + data, err := ioutil.ReadFile(bazelBuildList) + if err != nil { + ctx.Errorf(err.Error()) + } + files := strings.Split(strings.TrimSpace(string(data)), "\n") + for _, file := range files { + ctx.AddNinjaFileDeps(file) + } + + // Register bazel-owned build statements (obtained from the aquery invocation). + for index, buildStatement := range ctx.Config().BazelContext.BuildStatementsToRegister() { + rule := NewRuleBuilder(pctx, ctx) + cmd := rule.Command() + cmd.Text(fmt.Sprintf("cd %s/execroot/__main__ && %s", + ctx.Config().BazelContext.OutputBase(), buildStatement.Command)) + + for _, outputPath := range buildStatement.OutputPaths { + cmd.ImplicitOutput(PathForBazelOut(ctx, outputPath)) } - files := strings.Split(strings.TrimSpace(string(data)), "\n") - for _, file := range files { - ctx.AddNinjaFileDeps(file) + for _, inputPath := range buildStatement.InputPaths { + cmd.Implicit(PathForBazelOut(ctx, inputPath)) } + + // This is required to silence warnings pertaining to unexpected timestamps. Particularly, + // some Bazel builtins (such as files in the bazel_tools directory) have far-future + // timestamps. Without restat, Ninja would emit warnings that the input files of a + // build statement have later timestamps than the outputs. + rule.Restat() + + rule.Build(fmt.Sprintf("bazel %s", index), buildStatement.Mnemonic) } } diff --git a/android/paths.go b/android/paths.go index 0238a3fcf..10d8d0df5 100644 --- a/android/paths.go +++ b/android/paths.go @@ -1154,6 +1154,17 @@ func pathForModule(ctx ModuleContext) OutputPath { return PathForOutput(ctx, ".intermediates", ctx.ModuleDir(), ctx.ModuleName(), ctx.ModuleSubDir()) } +type BazelOutPath struct { + OutputPath +} + +var _ Path = BazelOutPath{} +var _ objPathProvider = BazelOutPath{} + +func (p BazelOutPath) objPathWithExt(ctx ModuleContext, subdir, ext string) ModuleObjPath { + return PathForModuleObj(ctx, subdir, pathtools.ReplaceExtension(p.path, ext)) +} + // PathForVndkRefAbiDump returns an OptionalPath representing the path of the // reference abi dump for the given module. This is not guaranteed to be valid. func PathForVndkRefAbiDump(ctx ModuleContext, version, fileName string, @@ -1192,6 +1203,24 @@ func PathForVndkRefAbiDump(ctx ModuleContext, version, fileName string, fileName+ext) } +// PathForBazelOut returns a Path representing the paths... under an output directory dedicated to +// bazel-owned outputs. +func PathForBazelOut(ctx PathContext, paths ...string) BazelOutPath { + execRootPathComponents := append([]string{"execroot", "__main__"}, paths...) + execRootPath := filepath.Join(execRootPathComponents...) + validatedExecRootPath, err := validatePath(execRootPath) + if err != nil { + reportPathError(ctx, err) + } + + outputPath := OutputPath{basePath{"", ctx.Config(), ""}, + ctx.Config().BazelContext.OutputBase()} + + return BazelOutPath{ + OutputPath: outputPath.withRel(validatedExecRootPath), + } +} + // PathForModuleOut returns a Path representing the paths... under the module's // output directory. func PathForModuleOut(ctx ModuleContext, paths ...string) ModuleOutPath { diff --git a/bazel/Android.bp b/bazel/Android.bp index d557be557..05eddc1b1 100644 --- a/bazel/Android.bp +++ b/bazel/Android.bp @@ -2,10 +2,14 @@ bootstrap_go_package { name: "soong-bazel", pkgPath: "android/soong/bazel", srcs: [ + "aquery.go", "constants.go", "properties.go", ], pluginFor: [ "soong_build", ], + deps: [ + "blueprint", + ], } diff --git a/bazel/aquery.go b/bazel/aquery.go new file mode 100644 index 000000000..69d4fde14 --- /dev/null +++ b/bazel/aquery.go @@ -0,0 +1,116 @@ +// 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 bazel + +import ( + "encoding/json" + "strings" + + "github.com/google/blueprint/proptools" +) + +// artifact contains relevant portions of Bazel's aquery proto, Artifact. +// Represents a single artifact, whether it's a source file or a derived output file. +type artifact struct { + Id string + ExecPath string +} + +// KeyValuePair represents Bazel's aquery proto, KeyValuePair. +type KeyValuePair struct { + Key string + Value string +} + +// depSetOfFiles contains relevant portions of Bazel's aquery proto, DepSetOfFiles. +// Represents a data structure containing one or more files. Depsets in Bazel are an efficient +// data structure for storing large numbers of file paths. +type depSetOfFiles struct { + Id string + // TODO(cparsons): Handle non-flat depsets. + DirectArtifactIds []string +} + +// action contains relevant portions of Bazel's aquery proto, Action. +// Represents a single command line invocation in the Bazel build graph. +type action struct { + Arguments []string + EnvironmentVariables []KeyValuePair + InputDepSetIds []string + Mnemonic string + OutputIds []string +} + +// actionGraphContainer contains relevant portions of Bazel's aquery proto, ActionGraphContainer. +// An aquery response from Bazel contains a single ActionGraphContainer proto. +type actionGraphContainer struct { + Artifacts []artifact + Actions []action + DepSetOfFiles []depSetOfFiles +} + +// BuildStatement contains information to register a build statement corresponding (one to one) +// with a Bazel action from Bazel's action graph. +type BuildStatement struct { + Command string + OutputPaths []string + InputPaths []string + Env []KeyValuePair + Mnemonic string +} + +// AqueryBuildStatements returns an array of BuildStatements which should be registered (and output +// to a ninja file) to correspond one-to-one with the given action graph json proto (from a bazel +// aquery invocation). +func AqueryBuildStatements(aqueryJsonProto []byte) []BuildStatement { + buildStatements := []BuildStatement{} + + var aqueryResult actionGraphContainer + json.Unmarshal(aqueryJsonProto, &aqueryResult) + + artifactIdToPath := map[string]string{} + for _, artifact := range aqueryResult.Artifacts { + artifactIdToPath[artifact.Id] = artifact.ExecPath + } + depsetIdToArtifactIds := map[string][]string{} + for _, depset := range aqueryResult.DepSetOfFiles { + depsetIdToArtifactIds[depset.Id] = depset.DirectArtifactIds + } + + for _, actionEntry := range aqueryResult.Actions { + outputPaths := []string{} + for _, outputId := range actionEntry.OutputIds { + // TODO(cparsons): Validate the id is present. + outputPaths = append(outputPaths, artifactIdToPath[outputId]) + } + inputPaths := []string{} + for _, inputDepSetId := range actionEntry.InputDepSetIds { + // TODO(cparsons): Validate the id is present. + for _, inputId := range depsetIdToArtifactIds[inputDepSetId] { + // TODO(cparsons): Validate the id is present. + inputPaths = append(inputPaths, artifactIdToPath[inputId]) + } + } + buildStatement := BuildStatement{ + Command: strings.Join(proptools.ShellEscapeList(actionEntry.Arguments), " "), + OutputPaths: outputPaths, + InputPaths: inputPaths, + Env: actionEntry.EnvironmentVariables, + Mnemonic: actionEntry.Mnemonic} + buildStatements = append(buildStatements, buildStatement) + } + + return buildStatements +} diff --git a/genrule/genrule.go b/genrule/genrule.go index 93938c92b..905401796 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -206,7 +206,7 @@ func (c *Module) generateBazelBuildActions(ctx android.ModuleContext, label stri if ok { var bazelOutputFiles android.Paths for _, bazelOutputFile := range filePaths { - bazelOutputFiles = append(bazelOutputFiles, android.PathForSource(ctx, bazelOutputFile)) + bazelOutputFiles = append(bazelOutputFiles, android.PathForBazelOut(ctx, bazelOutputFile)) } c.outputFiles = bazelOutputFiles c.outputDeps = bazelOutputFiles diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 8d3cfcbba..6ae9a18dd 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -736,7 +736,8 @@ func TestGenruleWithBazel(t *testing.T) { } gen := ctx.ModuleForTests("foo", "").Module().(*Module) - expectedOutputFiles := []string{"bazelone.txt", "bazeltwo.txt"} + expectedOutputFiles := []string{"outputbase/execroot/__main__/bazelone.txt", + "outputbase/execroot/__main__/bazeltwo.txt"} if !reflect.DeepEqual(gen.outputFiles.Strings(), expectedOutputFiles) { t.Errorf("Expected output files: %q, actual: %q", expectedOutputFiles, gen.outputFiles) }