Merge "bp2build: export some cc toolchain flags into Starlark."

android/bazel.go

@@ -207,7 +207,7 @@ var (
 		"libc_jemalloc_wrapper", // http://b/187012490, cc_library_static, depends on //external/jemalloc_new:libjemalloc5 (http://b/186828626)
 		"libc_ndk",              // http://b/187013218, cc_library_static, depends on //bionic/libm:libm (http://b/183064661)
 		"libc",                  // http://b/183064430, cc_library, depends on //external/jemalloc_new:libjemalloc5 (http://b/186828626)
-		"libc_bionic_ndk",       // http://b/186822256, cc_library_static, signal.cpp:186:52: error: ISO C++ requires field designators to be specified in declaration order
+		"libc_bionic_ndk",       // http://b/186822256, cc_library_static, fatal error: 'generated_android_ids.h' file not found
 		"libc_malloc_hooks",     // http://b/187016307, cc_library, ld.lld: error: undefined symbol: __malloc_hook
 		"libm",                  // http://b/183064661, cc_library, math.h:25:16: error: unexpected token in argument list
 

bp2build/Android.bp

@@ -20,6 +20,7 @@ bootstrap_go_package {
         "soong-android",
         "soong-bazel",
         "soong-cc",
+        "soong-cc-config",
         "soong-genrule",
         "soong-python",
         "soong-sh",

bp2build/bp2build.go

@@ -24,22 +24,16 @@ import (
 // writing .bzl files that are equivalent to Android.bp files that are capable
 // of being built with Bazel.
 func Codegen(ctx *CodegenContext) CodegenMetrics {
-	outputDir := android.PathForOutput(ctx, "bp2build")
+	// This directory stores BUILD files that could be eventually checked-in.
-	android.RemoveAllOutputDir(outputDir)
+	bp2buildDir := android.PathForOutput(ctx, "bp2build")
+	android.RemoveAllOutputDir(bp2buildDir)
 
 	buildToTargets, metrics := GenerateBazelTargets(ctx, true)
+	bp2buildFiles := CreateBazelFiles(nil, buildToTargets, ctx.mode)
+	writeFiles(ctx, bp2buildDir, bp2buildFiles)
 
-	filesToWrite := CreateBazelFiles(nil, buildToTargets, ctx.mode)
+	soongInjectionDir := android.PathForOutput(ctx, "soong_injection")
-
+	writeFiles(ctx, soongInjectionDir, CreateSoongInjectionFiles())
-	generatedBuildFiles := []string{}
-	for _, f := range filesToWrite {
-		p := getOrCreateOutputDir(outputDir, ctx, f.Dir).Join(ctx, f.Basename)
-		if err := writeFile(ctx, p, f.Contents); err != nil {
-			panic(fmt.Errorf("Failed to write %q (dir %q) due to %q", f.Basename, f.Dir, err))
-		}
-		// if these generated files are modified, regenerate on next run.
-		generatedBuildFiles = append(generatedBuildFiles, p.String())
-	}
 
 	return metrics
 }
@@ -51,6 +45,16 @@ func getOrCreateOutputDir(outputDir android.OutputPath, ctx android.PathContext,
 	return dirPath
 }
 
+// writeFiles materializes a list of BazelFile rooted at outputDir.
+func writeFiles(ctx android.PathContext, outputDir android.OutputPath, files []BazelFile) {
+	for _, f := range files {
+		p := getOrCreateOutputDir(outputDir, ctx, f.Dir).Join(ctx, f.Basename)
+		if err := writeFile(ctx, p, f.Contents); err != nil {
+			panic(fmt.Errorf("Failed to write %q (dir %q) due to %q", f.Basename, f.Dir, err))
+		}
+	}
+}
+
 func writeFile(ctx android.PathContext, pathToFile android.OutputPath, content string) error {
 	// These files are made editable to allow users to modify and iterate on them
 	// in the source tree.

bp2build/conversion.go

@@ -2,6 +2,7 @@ package bp2build
 
 import (
 	"android/soong/android"
+	"android/soong/cc/config"
 	"fmt"
 	"reflect"
 	"sort"
@@ -16,6 +17,15 @@ type BazelFile struct {
 	Contents string
 }
 
+func CreateSoongInjectionFiles() []BazelFile {
+	var files []BazelFile
+
+	files = append(files, newFile("cc_toolchain", "BUILD", "")) // Creates a //cc_toolchain package.
+	files = append(files, newFile("cc_toolchain", "constants.bzl", config.BazelCcToolchainVars()))
+
+	return files
+}
+
 func CreateBazelFiles(
 	ruleShims map[string]RuleShim,
 	buildToTargets map[string]BazelTargets,

bp2build/conversion_test.go

@@ -79,9 +79,33 @@ func TestCreateBazelFiles_QueryView_AddsTopLevelFiles(t *testing.T) {
 	}
 }
 
-func TestCreateBazelFiles_Bp2Build_CreatesNoFilesWithNoTargets(t *testing.T) {
+func TestCreateBazelFiles_Bp2Build_CreatesDefaultFiles(t *testing.T) {
-	files := CreateBazelFiles(map[string]RuleShim{}, map[string]BazelTargets{}, Bp2Build)
+	files := CreateSoongInjectionFiles()
-	if len(files) != 0 {
+
-		t.Errorf("Expected no files, got %d", len(files))
+	expectedFilePaths := []bazelFilepath{
+		{
+			dir:      "cc_toolchain",
+			basename: "BUILD",
+		},
+		{
+			dir:      "cc_toolchain",
+			basename: "constants.bzl",
+		},
+	}
+
+	if len(files) != len(expectedFilePaths) {
+		t.Errorf("Expected %d file, got %d", len(expectedFilePaths), len(files))
+	}
+
+	for i := range files {
+		actualFile, expectedFile := files[i], expectedFilePaths[i]
+
+		if actualFile.Dir != expectedFile.dir || actualFile.Basename != expectedFile.basename {
+			t.Errorf("Did not find expected file %s/%s", actualFile.Dir, actualFile.Basename)
+		}
+
+		if expectedFile.basename != "BUILD" && actualFile.Contents == "" {
+			t.Errorf("Contents of %s unexpected empty.", actualFile)
+		}
 	}
 }

cc/config/Android.bp

@@ -10,6 +10,7 @@ bootstrap_go_package {
         "soong-remoteexec",
     ],
     srcs: [
+        "bp2build.go",
         "clang.go",
         "global.go",
         "tidy.go",
@@ -31,6 +32,7 @@ bootstrap_go_package {
         "arm64_linux_host.go",
     ],
     testSrcs: [
+        "bp2build_test.go",
         "tidy_test.go",
     ],
 }

cc/config/bp2build.go

@@ -0,0 +1,148 @@
+// Copyright 2021 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 config
+
+import (
+	"android/soong/android"
+	"fmt"
+	"regexp"
+	"strings"
+)
+
+// Helpers for exporting cc configuration information to Bazel.
+
+var (
+	// Map containing toolchain variables that are independent of the
+	// environment variables of the build.
+	exportedVars = exportedVariablesMap{}
+)
+
+// variableValue is a string slice because the exported variables are all lists
+// of string, and it's simpler to manipulate string lists before joining them
+// into their final string representation.
+type variableValue []string
+
+// envDependentVariable is a toolchain variable computed based on an environment variable.
+type exportedVariablesMap map[string]variableValue
+
+func (m exportedVariablesMap) Set(k string, v variableValue) {
+	m[k] = v
+}
+
+// Convenience function to declare a static variable and export it to Bazel's cc_toolchain.
+func staticVariableExportedToBazel(name string, value []string) {
+	pctx.StaticVariable(name, strings.Join(value, " "))
+	exportedVars.Set(name, variableValue(value))
+}
+
+// BazelCcToolchainVars generates bzl file content containing variables for
+// Bazel's cc_toolchain configuration.
+func BazelCcToolchainVars() string {
+	ret := "# GENERATED FOR BAZEL FROM SOONG. DO NOT EDIT.\n\n"
+
+	// Ensure that string s has no invalid characters to be generated into the bzl file.
+	validateCharacters := func(s string) string {
+		for _, c := range []string{`\n`, `"`, `\`} {
+			if strings.Contains(s, c) {
+				panic(fmt.Errorf("%s contains illegal character %s", s, c))
+			}
+		}
+		return s
+	}
+
+	// For each exported variable, recursively expand elements in the variableValue
+	// list to ensure that interpolated variables are expanded according to their values
+	// in the exportedVars scope.
+	for _, k := range android.SortedStringKeys(exportedVars) {
+		variableValue := exportedVars[k]
+		var expandedVars []string
+		for _, v := range variableValue {
+			expandedVars = append(expandedVars, expandVar(v, exportedVars)...)
+		}
+		// Build the list for this variable.
+		list := "["
+		for _, flag := range expandedVars {
+			list += fmt.Sprintf("\n    \"%s\",", validateCharacters(flag))
+		}
+		list += "\n]"
+		// Assign the list as a bzl-private variable; this variable will be exported
+		// out through a constants struct later.
+		ret += fmt.Sprintf("_%s = %s\n", k, list)
+		ret += "\n"
+	}
+
+	// Build the exported constants struct.
+	ret += "constants = struct(\n"
+	for _, k := range android.SortedStringKeys(exportedVars) {
+		ret += fmt.Sprintf("    %s = _%s,\n", k, k)
+	}
+	ret += ")"
+	return ret
+}
+
+// expandVar recursively expand interpolated variables in the exportedVars scope.
+//
+// We're using a string slice to track the seen variables to avoid
+// stackoverflow errors with infinite recursion. it's simpler to use a
+// string slice than to handle a pass-by-referenced map, which would make it
+// quite complex to track depth-first interpolations. It's also unlikely the
+// interpolation stacks are deep (n > 1).
+func expandVar(toExpand string, exportedVars map[string]variableValue) []string {
+	// e.g. "${ClangExternalCflags}"
+	r := regexp.MustCompile(`\${([a-zA-Z0-9_]+)}`)
+
+	// Internal recursive function.
+	var expandVarInternal func(string, map[string]bool) []string
+	expandVarInternal = func(toExpand string, seenVars map[string]bool) []string {
+		var ret []string
+		for _, v := range strings.Split(toExpand, " ") {
+			matches := r.FindStringSubmatch(v)
+			if len(matches) == 0 {
+				return []string{v}
+			}
+
+			if len(matches) != 2 {
+				panic(fmt.Errorf(
+					"Expected to only match 1 subexpression in %s, got %d",
+					v,
+					len(matches)-1))
+			}
+
+			// Index 1 of FindStringSubmatch contains the subexpression match
+			// (variable name) of the capture group.
+			variable := matches[1]
+			// toExpand contains a variable.
+			if _, ok := seenVars[variable]; ok {
+				panic(fmt.Errorf(
+					"Unbounded recursive interpolation of variable: %s", variable))
+			}
+			// A map is passed-by-reference. Create a new map for
+			// this scope to prevent variables seen in one depth-first expansion
+			// to be also treated as "seen" in other depth-first traversals.
+			newSeenVars := map[string]bool{}
+			for k := range seenVars {
+				newSeenVars[k] = true
+			}
+			newSeenVars[variable] = true
+			unexpandedVars := exportedVars[variable]
+			for _, unexpandedVar := range unexpandedVars {
+				ret = append(ret, expandVarInternal(unexpandedVar, newSeenVars)...)
+			}
+		}
+		return ret
+	}
+
+	return expandVarInternal(toExpand, map[string]bool{})
+}

cc/config/bp2build_test.go

@@ -0,0 +1,91 @@
+// Copyright 2021 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 config
+
+import (
+	"testing"
+)
+
+func TestExpandVars(t *testing.T) {
+	testCases := []struct {
+		description    string
+		exportedVars   map[string]variableValue
+		toExpand       string
+		expectedValues []string
+	}{
+		{
+			description: "single level expansion",
+			exportedVars: map[string]variableValue{
+				"foo": variableValue([]string{"bar"}),
+			},
+			toExpand:       "${foo}",
+			expectedValues: []string{"bar"},
+		},
+		{
+			description: "double level expansion",
+			exportedVars: map[string]variableValue{
+				"foo": variableValue([]string{"${bar}"}),
+				"bar": variableValue([]string{"baz"}),
+			},
+			toExpand:       "${foo}",
+			expectedValues: []string{"baz"},
+		},
+		{
+			description: "double level expansion with a literal",
+			exportedVars: map[string]variableValue{
+				"a": variableValue([]string{"${b}", "c"}),
+				"b": variableValue([]string{"d"}),
+			},
+			toExpand:       "${a}",
+			expectedValues: []string{"d", "c"},
+		},
+		{
+			description: "double level expansion, with two variables in a string",
+			exportedVars: map[string]variableValue{
+				"a": variableValue([]string{"${b} ${c}"}),
+				"b": variableValue([]string{"d"}),
+				"c": variableValue([]string{"e"}),
+			},
+			toExpand:       "${a}",
+			expectedValues: []string{"d", "e"},
+		},
+		{
+			description: "triple level expansion with two variables in a string",
+			exportedVars: map[string]variableValue{
+				"a": variableValue([]string{"${b} ${c}"}),
+				"b": variableValue([]string{"${c}", "${d}"}),
+				"c": variableValue([]string{"${d}"}),
+				"d": variableValue([]string{"foo"}),
+			},
+			toExpand:       "${a}",
+			expectedValues: []string{"foo", "foo", "foo"},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			output := expandVar(testCase.toExpand, testCase.exportedVars)
+			if len(output) != len(testCase.expectedValues) {
+				t.Errorf("Expected %d values, got %d", len(testCase.expectedValues), len(output))
+			}
+			for i, actual := range output {
+				expectedValue := testCase.expectedValues[i]
+				if actual != expectedValue {
+					t.Errorf("Actual value '%s' doesn't match expected value '%s'", actual, expectedValue)
+				}
+			}
+		})
+	}
+}

cc/config/clang.go

@@ -98,7 +98,7 @@ var ClangTidyDisableChecks = []string{
 }
 
 func init() {
-	pctx.StaticVariable("ClangExtraCflags", strings.Join([]string{
+	staticVariableExportedToBazel("ClangExtraCflags", []string{
 		"-D__compiler_offsetof=__builtin_offsetof",
 
 		// Emit address-significance table which allows linker to perform safe ICF. Clang does
@@ -151,9 +151,9 @@ func init() {
 		// This macro allows the bionic versioning.h to indirectly determine whether the
 		// option -Wunguarded-availability is on or not.
 		"-D__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__",
-	}, " "))
+	})
 
-	pctx.StaticVariable("ClangExtraCppflags", strings.Join([]string{
+	staticVariableExportedToBazel("ClangExtraCppflags", []string{
 		// -Wimplicit-fallthrough is not enabled by -Wall.
 		"-Wimplicit-fallthrough",
 
@@ -162,13 +162,11 @@ func init() {
 
 		// libc++'s math.h has an #include_next outside of system_headers.
 		"-Wno-gnu-include-next",
-	}, " "))
+	})
 
-	pctx.StaticVariable("ClangExtraTargetCflags", strings.Join([]string{
+	staticVariableExportedToBazel("ClangExtraTargetCflags", []string{"-nostdlibinc"})
-		"-nostdlibinc",
-	}, " "))
 
-	pctx.StaticVariable("ClangExtraNoOverrideCflags", strings.Join([]string{
+	staticVariableExportedToBazel("ClangExtraNoOverrideCflags", []string{
 		"-Werror=address-of-temporary",
 		// Bug: http://b/29823425 Disable -Wnull-dereference until the
 		// new cases detected by this warning in Clang r271374 are
@@ -203,11 +201,11 @@ func init() {
 		"-Wno-non-c-typedef-for-linkage", // http://b/161304145
 		// New warnings to be fixed after clang-r407598
 		"-Wno-string-concatenation", // http://b/175068488
-	}, " "))
+	})
 
 	// Extra cflags for external third-party projects to disable warnings that
 	// are infeasible to fix in all the external projects and their upstream repos.
-	pctx.StaticVariable("ClangExtraExternalCflags", strings.Join([]string{
+	staticVariableExportedToBazel("ClangExtraExternalCflags", []string{
 		"-Wno-enum-compare",
 		"-Wno-enum-compare-switch",
 
@@ -228,7 +226,7 @@ func init() {
 
 		// http://b/165945989
 		"-Wno-psabi",
-	}, " "))
+	})
 }
 
 func ClangFilterUnknownCflags(cflags []string) []string {

cc/config/global.go

@@ -165,13 +165,25 @@ func init() {
 		commonGlobalCflags = append(commonGlobalCflags, "-fdebug-prefix-map=/proc/self/cwd=")
 	}
 
-	pctx.StaticVariable("CommonGlobalConlyflags", strings.Join(commonGlobalConlyflags, " "))
+	staticVariableExportedToBazel("CommonGlobalConlyflags", commonGlobalConlyflags)
-	pctx.StaticVariable("DeviceGlobalCppflags", strings.Join(deviceGlobalCppflags, " "))
+	staticVariableExportedToBazel("DeviceGlobalCppflags", deviceGlobalCppflags)
-	pctx.StaticVariable("DeviceGlobalLdflags", strings.Join(deviceGlobalLdflags, " "))
+	staticVariableExportedToBazel("DeviceGlobalLdflags", deviceGlobalLdflags)
-	pctx.StaticVariable("DeviceGlobalLldflags", strings.Join(deviceGlobalLldflags, " "))
+	staticVariableExportedToBazel("DeviceGlobalLldflags", deviceGlobalLldflags)
-	pctx.StaticVariable("HostGlobalCppflags", strings.Join(hostGlobalCppflags, " "))
+	staticVariableExportedToBazel("HostGlobalCppflags", hostGlobalCppflags)
-	pctx.StaticVariable("HostGlobalLdflags", strings.Join(hostGlobalLdflags, " "))
+	staticVariableExportedToBazel("HostGlobalLdflags", hostGlobalLdflags)
-	pctx.StaticVariable("HostGlobalLldflags", strings.Join(hostGlobalLldflags, " "))
+	staticVariableExportedToBazel("HostGlobalLldflags", hostGlobalLldflags)
+
+	// Export the static default CommonClangGlobalCflags to Bazel.
+	// TODO(187086342): handle cflags that are set in VariableFuncs.
+	commonClangGlobalCFlags := append(
+		ClangFilterUnknownCflags(commonGlobalCflags),
+		[]string{
+			"${ClangExtraCflags}",
+			// Default to zero initialization.
+			"-ftrivial-auto-var-init=zero",
+			"-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang",
+		}...)
+	exportedVars.Set("CommonClangGlobalCflags", variableValue(commonClangGlobalCFlags))
 
 	pctx.VariableFunc("CommonClangGlobalCflags", func(ctx android.PackageVarContext) string {
 		flags := ClangFilterUnknownCflags(commonGlobalCflags)
@@ -190,26 +202,26 @@ func init() {
 			// Default to zero initialization.
 			flags = append(flags, "-ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang")
 		}
-
 		return strings.Join(flags, " ")
 	})
 
+	// Export the static default DeviceClangGlobalCflags to Bazel.
+	// TODO(187086342): handle cflags that are set in VariableFuncs.
+	deviceClangGlobalCflags := append(ClangFilterUnknownCflags(deviceGlobalCflags), "${ClangExtraTargetCflags}")
+	exportedVars.Set("DeviceClangGlobalCflags", variableValue(deviceClangGlobalCflags))
+
 	pctx.VariableFunc("DeviceClangGlobalCflags", func(ctx android.PackageVarContext) string {
 		if ctx.Config().Fuchsia() {
 			return strings.Join(ClangFilterUnknownCflags(deviceGlobalCflags), " ")
 		} else {
-			return strings.Join(append(ClangFilterUnknownCflags(deviceGlobalCflags), "${ClangExtraTargetCflags}"), " ")
+			return strings.Join(deviceClangGlobalCflags, " ")
 		}
 	})
-	pctx.StaticVariable("HostClangGlobalCflags",
-		strings.Join(ClangFilterUnknownCflags(hostGlobalCflags), " "))
-	pctx.StaticVariable("NoOverrideClangGlobalCflags",
-		strings.Join(append(ClangFilterUnknownCflags(noOverrideGlobalCflags), "${ClangExtraNoOverrideCflags}"), " "))
 
-	pctx.StaticVariable("CommonClangGlobalCppflags",
+	staticVariableExportedToBazel("HostClangGlobalCflags", ClangFilterUnknownCflags(hostGlobalCflags))
-		strings.Join(append(ClangFilterUnknownCflags(commonGlobalCppflags), "${ClangExtraCppflags}"), " "))
+	staticVariableExportedToBazel("NoOverrideClangGlobalCflags", append(ClangFilterUnknownCflags(noOverrideGlobalCflags), "${ClangExtraNoOverrideCflags}"))
-
+	staticVariableExportedToBazel("CommonClangGlobalCppflags", append(ClangFilterUnknownCflags(commonGlobalCppflags), "${ClangExtraCppflags}"))
-	pctx.StaticVariable("ClangExternalCflags", "${ClangExtraExternalCflags}")
+	staticVariableExportedToBazel("ClangExternalCflags", []string{"${ClangExtraExternalCflags}"})
 
 	// Everything in these lists is a crime against abstraction and dependency tracking.
 	// Do not add anything to this list.