From 787fb3618948fa866f01762f624a38a3f7b299fc Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Thu, 14 Oct 2021 18:43:51 -0400 Subject: [PATCH] Add OS to configuration key in mixed builds This also removes the special-case filegroup from mixed builds buildroot; no special handling is required. Since we're currently hardcoding linux_x86_64 as our host platform, it should be fine to continue assumping that hardcoded host for now. Test: USE_BAZEL_ANALYSIS=1 m adbd Change-Id: I35509f4eb33ba7a243fab4c34b35958f6f2fceab --- android/bazel_handler.go | 118 +++++++++++++++++++--------------- android/bazel_handler_test.go | 8 +-- android/filegroup.go | 4 +- cc/library.go | 2 +- cc/library_headers.go | 2 +- cc/object.go | 2 +- cc/prebuilt.go | 2 +- genrule/genrule.go | 2 +- 8 files changed, 79 insertions(+), 61 deletions(-) diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 06712a1ea..80e127c29 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -48,11 +48,17 @@ type cqueryRequest interface { StarlarkFunctionBody() string } +// Portion of cquery map key to describe target configuration. +type configKey struct { + archType ArchType + osType OsType +} + // Map key to describe bazel cquery requests. type cqueryKey struct { label string requestType cqueryRequest - archType ArchType + configKey configKey } // bazelHandler is the interface for a helper object related to deferring to Bazel for @@ -72,14 +78,14 @@ type BazelContext interface { // has been queued to be run later. // Returns result files built by building the given bazel target label. - GetOutputFiles(label string, archType ArchType) ([]string, bool) + GetOutputFiles(label string, cfgKey configKey) ([]string, bool) // TODO(cparsons): Other cquery-related methods should be added here. // Returns the results of GetOutputFiles and GetCcObjectFiles in a single query (in that order). - GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) + GetCcInfo(label string, cfgKey configKey) (cquery.CcInfo, bool, error) // Returns the executable binary resultant from building together the python sources - GetPythonBinary(label string, archType ArchType) (string, bool) + GetPythonBinary(label string, cfgKey configKey) (string, bool) // ** End cquery methods @@ -140,17 +146,17 @@ type MockBazelContext struct { LabelToPythonBinary map[string]string } -func (m MockBazelContext) GetOutputFiles(label string, archType ArchType) ([]string, bool) { +func (m MockBazelContext) GetOutputFiles(label string, cfgKey configKey) ([]string, bool) { result, ok := m.LabelToOutputFiles[label] return result, ok } -func (m MockBazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) { +func (m MockBazelContext) GetCcInfo(label string, cfgKey configKey) (cquery.CcInfo, bool, error) { result, ok := m.LabelToCcInfo[label] return result, ok, nil } -func (m MockBazelContext) GetPythonBinary(label string, archType ArchType) (string, bool) { +func (m MockBazelContext) GetPythonBinary(label string, cfgKey configKey) (string, bool) { result, ok := m.LabelToPythonBinary[label] return result, ok } @@ -171,8 +177,8 @@ func (m MockBazelContext) BuildStatementsToRegister() []bazel.BuildStatement { var _ BazelContext = MockBazelContext{} -func (bazelCtx *bazelContext) GetOutputFiles(label string, archType ArchType) ([]string, bool) { - rawString, ok := bazelCtx.cquery(label, cquery.GetOutputFiles, archType) +func (bazelCtx *bazelContext) GetOutputFiles(label string, cfgKey configKey) ([]string, bool) { + rawString, ok := bazelCtx.cquery(label, cquery.GetOutputFiles, cfgKey) var ret []string if ok { bazelOutput := strings.TrimSpace(rawString) @@ -181,8 +187,8 @@ func (bazelCtx *bazelContext) GetOutputFiles(label string, archType ArchType) ([ return ret, ok } -func (bazelCtx *bazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) { - result, ok := bazelCtx.cquery(label, cquery.GetCcInfo, archType) +func (bazelCtx *bazelContext) GetCcInfo(label string, cfgKey configKey) (cquery.CcInfo, bool, error) { + result, ok := bazelCtx.cquery(label, cquery.GetCcInfo, cfgKey) if !ok { return cquery.CcInfo{}, ok, nil } @@ -192,8 +198,8 @@ func (bazelCtx *bazelContext) GetCcInfo(label string, archType ArchType) (cquery return ret, ok, err } -func (bazelCtx *bazelContext) GetPythonBinary(label string, archType ArchType) (string, bool) { - rawString, ok := bazelCtx.cquery(label, cquery.GetPythonBinary, archType) +func (bazelCtx *bazelContext) GetPythonBinary(label string, cfgKey configKey) (string, bool) { + rawString, ok := bazelCtx.cquery(label, cquery.GetPythonBinary, cfgKey) var ret string if ok { bazelOutput := strings.TrimSpace(rawString) @@ -202,15 +208,15 @@ func (bazelCtx *bazelContext) GetPythonBinary(label string, archType ArchType) ( return ret, ok } -func (n noopBazelContext) GetOutputFiles(label string, archType ArchType) ([]string, bool) { +func (n noopBazelContext) GetOutputFiles(label string, cfgKey configKey) ([]string, bool) { panic("unimplemented") } -func (n noopBazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) { +func (n noopBazelContext) GetCcInfo(label string, cfgKey configKey) (cquery.CcInfo, bool, error) { panic("unimplemented") } -func (n noopBazelContext) GetPythonBinary(label string, archType ArchType) (string, bool) { +func (n noopBazelContext) GetPythonBinary(label string, cfgKey configKey) (string, bool) { panic("unimplemented") } @@ -303,8 +309,8 @@ func (context *bazelContext) BazelEnabled() bool { // returns (result, true). If the request is queued but no results are available, // then returns ("", false). func (context *bazelContext) cquery(label string, requestType cqueryRequest, - archType ArchType) (string, bool) { - key := cqueryKey{label, requestType, archType} + cfgKey configKey) (string, bool) { + key := cqueryKey{label, requestType, cfgKey} if result, ok := context.results[key]; ok { return result, true } else { @@ -419,7 +425,7 @@ func (context *bazelContext) mainBzlFileContents() []byte { def _config_node_transition_impl(settings, attr): return { - "//command_line_option:platforms": "@//build/bazel/platforms:android_%s" % attr.arch, + "//command_line_option:platforms": "@//build/bazel/platforms:%s_%s" % (attr.os, attr.arch), } _config_node_transition = transition( @@ -437,7 +443,8 @@ config_node = rule( implementation = _passthrough_rule_impl, attrs = { "arch" : attr.string(mandatory = True), - "deps" : attr.label_list(cfg = _config_node_transition), + "os" : attr.string(mandatory = True), + "deps" : attr.label_list(cfg = _config_node_transition, allow_files = True), "_allowlist_function_transition": attr.label(default = "@bazel_tools//tools/allowlists/function_transition_allowlist"), }, ) @@ -488,38 +495,32 @@ phony_root(name = "phonyroot", configNodeFormatString := ` config_node(name = "%s", arch = "%s", + os = "%s", deps = [%s], ) -` - - commonArchFilegroupString := ` -filegroup(name = "common", - srcs = [%s], -) ` configNodesSection := "" - labelsByArch := map[string][]string{} + labelsByConfig := map[string][]string{} for val, _ := range context.requests { labelString := fmt.Sprintf("\"@%s\"", val.label) - archString := getArchString(val) - labelsByArch[archString] = append(labelsByArch[archString], labelString) + configString := getConfigString(val) + labelsByConfig[configString] = append(labelsByConfig[configString], labelString) } allLabels := []string{} - for archString, labels := range labelsByArch { - if archString == "common" { - // arch-less labels (e.g. filegroups) don't need a config_node - allLabels = append(allLabels, "\":common\"") - labelsString := strings.Join(labels, ",\n ") - configNodesSection += fmt.Sprintf(commonArchFilegroupString, labelsString) - } else { - // Create a config_node, and add the config_node's label to allLabels - allLabels = append(allLabels, fmt.Sprintf("\":%s\"", archString)) - labelsString := strings.Join(labels, ",\n ") - configNodesSection += fmt.Sprintf(configNodeFormatString, archString, archString, labelsString) + for configString, labels := range labelsByConfig { + configTokens := strings.Split(configString, "|") + if len(configTokens) != 2 { + panic(fmt.Errorf("Unexpected config string format: %s", configString)) } + archString := configTokens[0] + osString := configTokens[1] + targetString := fmt.Sprintf("%s_%s", osString, archString) + allLabels = append(allLabels, fmt.Sprintf("\":%s\"", targetString)) + labelsString := strings.Join(labels, ",\n ") + configNodesSection += fmt.Sprintf(configNodeFormatString, targetString, archString, osString, labelsString) } return []byte(fmt.Sprintf(formatString, configNodesSection, strings.Join(allLabels, ",\n "))) @@ -587,11 +588,15 @@ def %s(target): %s def get_arch(target): + # TODO(b/199363072): filegroups and file targets aren't associated with any + # specific platform architecture in mixed builds. This is consistent with how + # Soong treats filegroups, but it may not be the case with manually-written + # filegroup BUILD targets. buildoptions = build_options(target) if buildoptions == None: # File targets do not have buildoptions. File targets aren't associated with - # any specific platform architecture in mixed builds. - return "common" + # any specific platform architecture in mixed builds, so use the host. + return "x86_64|linux" platforms = build_options(target)["//command_line_option:platforms"] if len(platforms) != 1: # An individual configured target should have only one platform architecture. @@ -601,10 +606,12 @@ def get_arch(target): platform_name = build_options(target)["//command_line_option:platforms"][0].name if platform_name == "host": return "HOST" + elif platform_name.startswith("linux_glibc_"): + return platform_name[len("linux_glibc_"):] + "|" + platform_name[:len("linux_glibc_")-1] elif platform_name.startswith("android_"): - return platform_name[len("android_"):] + return platform_name[len("android_"):] + "|" + platform_name[:len("android_")-1] elif platform_name.startswith("linux_"): - return platform_name[len("linux_"):] + return platform_name[len("linux_"):] + "|" + platform_name[:len("linux_")-1] else: fail("expected platform name of the form 'android_' or 'linux_', but was " + str(platforms)) return "UNKNOWN" @@ -852,14 +859,23 @@ func (c *bazelSingleton) GenerateBuildActions(ctx SingletonContext) { } func getCqueryId(key cqueryKey) string { - return key.label + "|" + getArchString(key) + return key.label + "|" + getConfigString(key) } -func getArchString(key cqueryKey) string { - arch := key.archType.Name - if len(arch) > 0 { - return arch - } else { - return "x86_64" +func getConfigString(key cqueryKey) string { + arch := key.configKey.archType.Name + if len(arch) == 0 || arch == "common" { + // Use host platform, which is currently hardcoded to be x86_64. + arch = "x86_64" } + os := key.configKey.osType.Name + if len(os) == 0 || os == "common_os" { + // Use host OS, which is currently hardcoded to be linux. + os = "linux" + } + return arch + "|" + os +} + +func GetConfigKey(ctx ModuleContext) configKey { + return configKey{archType: ctx.Arch().ArchType, osType: ctx.Os()} } diff --git a/android/bazel_handler_test.go b/android/bazel_handler_test.go index fe66a9092..ad5b63bae 100644 --- a/android/bazel_handler_test.go +++ b/android/bazel_handler_test.go @@ -9,11 +9,11 @@ import ( func TestRequestResultsAfterInvokeBazel(t *testing.T) { label := "//foo:bar" - arch := Arm64 + cfg := configKey{Arm64, Android} bazelContext, _ := testBazelContext(t, map[bazelCommand]string{ - bazelCommand{command: "cquery", expression: "deps(@soong_injection//mixed_builds:buildroot, 2)"}: `//foo:bar|arm64>>out/foo/bar.txt`, + bazelCommand{command: "cquery", expression: "deps(@soong_injection//mixed_builds:buildroot, 2)"}: `//foo:bar|arm64|android>>out/foo/bar.txt`, }) - g, ok := bazelContext.GetOutputFiles(label, arch) + g, ok := bazelContext.GetOutputFiles(label, cfg) if ok { t.Errorf("Did not expect cquery results prior to running InvokeBazel(), but got %s", g) } @@ -21,7 +21,7 @@ func TestRequestResultsAfterInvokeBazel(t *testing.T) { if err != nil { t.Fatalf("Did not expect error invoking Bazel, but got %s", err) } - g, ok = bazelContext.GetOutputFiles(label, arch) + g, ok = bazelContext.GetOutputFiles(label, cfg) if !ok { t.Errorf("Expected cquery results after running InvokeBazel(), but got none") } else if w := []string{"out/foo/bar.txt"}; !reflect.DeepEqual(w, g) { diff --git a/android/filegroup.go b/android/filegroup.go index f8f095586..a79374d1f 100644 --- a/android/filegroup.go +++ b/android/filegroup.go @@ -118,14 +118,16 @@ func (fg *fileGroup) maybeGenerateBazelBuildActions(ctx ModuleContext) { } archVariant := ctx.Arch().ArchType + osVariant := ctx.Os() if len(fg.Srcs()) == 1 && fg.Srcs()[0].Base() == fg.Name() { // This will be a regular file target, not filegroup, in Bazel. // See FilegroupBp2Build for more information. archVariant = Common + osVariant = CommonOS } bazelCtx := ctx.Config().BazelContext - filePaths, ok := bazelCtx.GetOutputFiles(fg.GetBazelLabel(ctx, fg), archVariant) + filePaths, ok := bazelCtx.GetOutputFiles(fg.GetBazelLabel(ctx, fg), configKey{archVariant, osVariant}) if !ok { return } diff --git a/cc/library.go b/cc/library.go index ed4d3d22e..d63acfbf7 100644 --- a/cc/library.go +++ b/cc/library.go @@ -639,7 +639,7 @@ func (handler *ccLibraryBazelHandler) generateSharedBazelBuildActions(ctx androi func (handler *ccLibraryBazelHandler) GenerateBazelBuildActions(ctx android.ModuleContext, label string) bool { bazelCtx := ctx.Config().BazelContext - ccInfo, ok, err := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType) + ccInfo, ok, err := bazelCtx.GetCcInfo(label, android.GetConfigKey(ctx)) if err != nil { ctx.ModuleErrorf("Error getting Bazel CcInfo: %s", err) return false diff --git a/cc/library_headers.go b/cc/library_headers.go index 51c1eb828..b2b3dbce9 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -57,7 +57,7 @@ type libraryHeaderBazelHander struct { func (h *libraryHeaderBazelHander) GenerateBazelBuildActions(ctx android.ModuleContext, label string) bool { bazelCtx := ctx.Config().BazelContext - ccInfo, ok, err := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType) + ccInfo, ok, err := bazelCtx.GetCcInfo(label, android.GetConfigKey(ctx)) if err != nil { ctx.ModuleErrorf("Error getting Bazel CcInfo: %s", err) return false diff --git a/cc/object.go b/cc/object.go index d8bb08fac..43abb3a3c 100644 --- a/cc/object.go +++ b/cc/object.go @@ -54,7 +54,7 @@ type objectBazelHandler struct { func (handler *objectBazelHandler) GenerateBazelBuildActions(ctx android.ModuleContext, label string) bool { bazelCtx := ctx.Config().BazelContext - objPaths, ok := bazelCtx.GetOutputFiles(label, ctx.Arch().ArchType) + objPaths, ok := bazelCtx.GetOutputFiles(label, android.GetConfigKey(ctx)) if ok { if len(objPaths) != 1 { ctx.ModuleErrorf("expected exactly one object file for '%s', but got %s", label, objPaths) diff --git a/cc/prebuilt.go b/cc/prebuilt.go index d324241e0..62f393824 100644 --- a/cc/prebuilt.go +++ b/cc/prebuilt.go @@ -330,7 +330,7 @@ type prebuiltStaticLibraryBazelHandler struct { func (h *prebuiltStaticLibraryBazelHandler) GenerateBazelBuildActions(ctx android.ModuleContext, label string) bool { bazelCtx := ctx.Config().BazelContext - ccInfo, ok, err := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType) + ccInfo, ok, err := bazelCtx.GetCcInfo(label, android.GetConfigKey(ctx)) if err != nil { ctx.ModuleErrorf("Error getting Bazel CcInfo: %s", err) } diff --git a/genrule/genrule.go b/genrule/genrule.go index 4dd21351a..b9c2b109d 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -248,7 +248,7 @@ func toolDepsMutator(ctx android.BottomUpMutatorContext) { // Returns true if information was available from Bazel, false if bazel invocation still needs to occur. func (c *Module) GenerateBazelBuildActions(ctx android.ModuleContext, label string) bool { bazelCtx := ctx.Config().BazelContext - filePaths, ok := bazelCtx.GetOutputFiles(label, ctx.Arch().ArchType) + filePaths, ok := bazelCtx.GetOutputFiles(label, android.GetConfigKey(ctx)) if ok { var bazelOutputFiles android.Paths exportIncludeDirs := map[string]bool{}