From 932c01cf9efab7e8dc00b7faad470b5499ad04c0 Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Fri, 25 Mar 2022 16:33:26 +0000 Subject: [PATCH] export Java variables to Bazel Test: build/bazel/bp2build.sh Change-Id: Ia06f9265c9f96e6add6edbd0cee925fe37b430d3 --- android/config_bp2build.go | 54 ++++++++++++- android/config_bp2build_test.go | 131 ++++++++++++++++++++++++++++++++ bp2build/build_conversion.go | 4 +- bp2build/conversion.go | 8 +- bp2build/conversion_test.go | 8 ++ java/config/config.go | 46 +++++++---- java/config/error_prone.go | 24 +++--- starlark_fmt/format.go | 13 +++- starlark_fmt/format_test.go | 12 ++- 9 files changed, 262 insertions(+), 38 deletions(-) diff --git a/android/config_bp2build.go b/android/config_bp2build.go index 80b09fc31..748be62cc 100644 --- a/android/config_bp2build.go +++ b/android/config_bp2build.go @@ -101,6 +101,17 @@ func (ev ExportedVariables) ExportSourcePathVariable(name string, value string) ev.exportedStringVars.set(name, value) } +// ExportVariableFuncVariable declares a variable whose value is evaluated at +// runtime via a function and exports it to Bazel's toolchain. +func (ev ExportedVariables) ExportVariableFuncVariable(name string, f func() string) { + ev.exportedConfigDependingVars.set(name, func(config Config) string { + return f() + }) + ev.pctx.VariableFunc(name, func(PackageVarContext) string { + return f() + }) +} + // ExportString only exports a variable to Bazel, but does not declare it in Soong func (ev ExportedVariables) ExportString(name string, value string) { ev.exportedStringVars.set(name, value) @@ -403,7 +414,8 @@ func expandVar(config Config, toExpand string, stringScope ExportedStringVariabl return ret, nil } var ret []string - for _, v := range strings.Split(toExpand, " ") { + stringFields := splitStringKeepingQuotedSubstring(toExpand, ' ') + for _, v := range stringFields { val, err := expandVarInternal(v, map[string]bool{}) if err != nil { return ret, err @@ -414,6 +426,46 @@ func expandVar(config Config, toExpand string, stringScope ExportedStringVariabl return ret, nil } +// splitStringKeepingQuotedSubstring splits a string on a provided separator, +// but it will not split substrings inside unescaped double quotes. If the double +// quotes are escaped, then the returned string will only include the quote, and +// not the escape. +func splitStringKeepingQuotedSubstring(s string, delimiter byte) []string { + var ret []string + quote := byte('"') + + var substring []byte + quoted := false + escaped := false + + for i := range s { + if !quoted && s[i] == delimiter { + ret = append(ret, string(substring)) + substring = []byte{} + continue + } + + characterIsEscape := i < len(s)-1 && s[i] == '\\' && s[i+1] == quote + if characterIsEscape { + escaped = true + continue + } + + if s[i] == quote { + if !escaped { + quoted = !quoted + } + escaped = false + } + + substring = append(substring, s[i]) + } + + ret = append(ret, string(substring)) + + return ret +} + func validateVariableMethod(name string, methodValue reflect.Value) { methodType := methodValue.Type() if methodType.Kind() != reflect.Func { diff --git a/android/config_bp2build_test.go b/android/config_bp2build_test.go index 05a1798c6..1a0ba7b93 100644 --- a/android/config_bp2build_test.go +++ b/android/config_bp2build_test.go @@ -321,3 +321,134 @@ constants = struct( }) } } + +func TestSplitStringKeepingQuotedSubstring(t *testing.T) { + testCases := []struct { + description string + s string + delimiter byte + split []string + }{ + { + description: "empty string returns single empty string", + s: "", + delimiter: ' ', + split: []string{ + "", + }, + }, + { + description: "string with single space returns two empty strings", + s: " ", + delimiter: ' ', + split: []string{ + "", + "", + }, + }, + { + description: "string with two spaces returns three empty strings", + s: " ", + delimiter: ' ', + split: []string{ + "", + "", + "", + }, + }, + { + description: "string with four words returns four word string", + s: "hello world with words", + delimiter: ' ', + split: []string{ + "hello", + "world", + "with", + "words", + }, + }, + { + description: "string with words and nested quote returns word strings and quote string", + s: `hello "world with" words`, + delimiter: ' ', + split: []string{ + "hello", + `"world with"`, + "words", + }, + }, + { + description: "string with escaped quote inside real quotes", + s: `hello \"world "with\" words"`, + delimiter: ' ', + split: []string{ + "hello", + `"world`, + `"with" words"`, + }, + }, + { + description: "string with words and escaped quotes returns word strings", + s: `hello \"world with\" words`, + delimiter: ' ', + split: []string{ + "hello", + `"world`, + `with"`, + "words", + }, + }, + { + description: "string which is single quoted substring returns only substring", + s: `"hello world with words"`, + delimiter: ' ', + split: []string{ + `"hello world with words"`, + }, + }, + { + description: "string starting with quote returns quoted string", + s: `"hello world with" words`, + delimiter: ' ', + split: []string{ + `"hello world with"`, + "words", + }, + }, + { + description: "string with starting quote and no ending quote returns quote to end of string", + s: `hello "world with words`, + delimiter: ' ', + split: []string{ + "hello", + `"world with words`, + }, + }, + { + description: "quoted string is treated as a single \"word\" unless separated by delimiter", + s: `hello "world"with words`, + delimiter: ' ', + split: []string{ + "hello", + `"world"with`, + "words", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + split := splitStringKeepingQuotedSubstring(tc.s, tc.delimiter) + if len(split) != len(tc.split) { + t.Fatalf("number of split string elements (%d) differs from expected (%d): split string (%v), expected (%v)", + len(split), len(tc.split), split, tc.split, + ) + } + for i := range split { + if split[i] != tc.split[i] { + t.Errorf("split string element (%d), %v, differs from expected, %v", i, split[i], tc.split[i]) + } + } + }) + } +} diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 1d3b10550..a96a3fc7b 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -580,7 +580,9 @@ func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) ( elements = append(elements, val) } } - return starlark_fmt.PrintList(elements, indent, "%s"), nil + return starlark_fmt.PrintList(elements, indent, func(s string) string { + return "%s" + }), nil case reflect.Struct: // Special cases where the bp2build sends additional information to the codegenerator diff --git a/bp2build/conversion.go b/bp2build/conversion.go index 91e614d23..1790dd7b8 100644 --- a/bp2build/conversion.go +++ b/bp2build/conversion.go @@ -7,7 +7,8 @@ import ( "strings" "android/soong/android" - "android/soong/cc/config" + cc_config "android/soong/cc/config" + java_config "android/soong/java/config" "github.com/google/blueprint/proptools" ) @@ -22,7 +23,10 @@ func CreateSoongInjectionFiles(cfg android.Config, metrics CodegenMetrics) []Baz var files []BazelFile files = append(files, newFile("cc_toolchain", GeneratedBuildFileName, "")) // Creates a //cc_toolchain package. - files = append(files, newFile("cc_toolchain", "constants.bzl", config.BazelCcToolchainVars(cfg))) + files = append(files, newFile("cc_toolchain", "constants.bzl", cc_config.BazelCcToolchainVars(cfg))) + + files = append(files, newFile("java_toolchain", GeneratedBuildFileName, "")) // Creates a //java_toolchain package. + files = append(files, newFile("java_toolchain", "constants.bzl", java_config.BazelJavaToolchainVars(cfg))) files = append(files, newFile("metrics", "converted_modules.txt", strings.Join(metrics.convertedModules, "\n"))) diff --git a/bp2build/conversion_test.go b/bp2build/conversion_test.go index d65ece8c7..e49d8553b 100644 --- a/bp2build/conversion_test.go +++ b/bp2build/conversion_test.go @@ -94,6 +94,14 @@ func TestCreateBazelFiles_Bp2Build_CreatesDefaultFiles(t *testing.T) { dir: "cc_toolchain", basename: "constants.bzl", }, + { + dir: "java_toolchain", + basename: GeneratedBuildFileName, + }, + { + dir: "java_toolchain", + basename: "constants.bzl", + }, { dir: "metrics", basename: "converted_modules.txt", diff --git a/java/config/config.go b/java/config/config.go index 262c53196..95b841fa7 100644 --- a/java/config/config.go +++ b/java/config/config.go @@ -26,7 +26,8 @@ import ( ) var ( - pctx = android.NewPackageContext("android/soong/java/config") + pctx = android.NewPackageContext("android/soong/java/config") + exportedVars = android.NewExportedVariables(pctx) LegacyCorePlatformBootclasspathLibraries = []string{"legacy.core.platform.api.stubs", "core-lambda-stubs"} LegacyCorePlatformSystemModules = "legacy-core-platform-api-stubs-system-modules" @@ -53,25 +54,40 @@ var ( } ) -const ( - JavaVmFlags = `-XX:OnError="cat hs_err_pid%p.log" -XX:CICompilerCount=6 -XX:+UseDynamicNumberOfGCThreads` - JavacVmFlags = `-J-XX:OnError="cat hs_err_pid%p.log" -J-XX:CICompilerCount=6 -J-XX:+UseDynamicNumberOfGCThreads -J-XX:+TieredCompilation -J-XX:TieredStopAtLevel=1` +var ( + JavacVmFlags = strings.Join(javacVmFlagsList, " ") + javaVmFlagsList = []string{ + `-XX:OnError="cat hs_err_pid%p.log"`, + "-XX:CICompilerCount=6", + "-XX:+UseDynamicNumberOfGCThreads", + } + javacVmFlagsList = []string{ + `-J-XX:OnError="cat hs_err_pid%p.log"`, + "-J-XX:CICompilerCount=6", + "-J-XX:+UseDynamicNumberOfGCThreads", + "-J-XX:+TieredCompilation", + "-J-XX:TieredStopAtLevel=1", + } ) func init() { pctx.Import("github.com/google/blueprint/bootstrap") - pctx.StaticVariable("JavacHeapSize", "2048M") - pctx.StaticVariable("JavacHeapFlags", "-J-Xmx${JavacHeapSize}") + exportedVars.ExportStringStaticVariable("JavacHeapSize", "2048M") + exportedVars.ExportStringStaticVariable("JavacHeapFlags", "-J-Xmx${JavacHeapSize}") // ErrorProne can use significantly more memory than javac alone, give it a higher heap // size (b/221480398). - pctx.StaticVariable("ErrorProneHeapSize", "4096M") - pctx.StaticVariable("ErrorProneHeapFlags", "-J-Xmx${ErrorProneHeapSize}") + exportedVars.ExportStringStaticVariable("ErrorProneHeapSize", "4096M") + exportedVars.ExportStringStaticVariable("ErrorProneHeapFlags", "-J-Xmx${ErrorProneHeapSize}") - pctx.StaticVariable("DexFlags", "-JXX:OnError='cat hs_err_pid%p.log' -JXX:CICompilerCount=6 -JXX:+UseDynamicNumberOfGCThreads") + exportedVars.ExportStringListStaticVariable("DexFlags", []string{ + `-JXX:OnError="cat hs_err_pid%p.log"`, + "-JXX:CICompilerCount=6", + "-JXX:+UseDynamicNumberOfGCThreads", + }) - pctx.StaticVariable("CommonJdkFlags", strings.Join([]string{ + exportedVars.ExportStringListStaticVariable("CommonJdkFlags", []string{ `-Xmaxerrs 9999999`, `-encoding UTF-8`, `-sourcepath ""`, @@ -85,10 +101,10 @@ func init() { // b/65004097: prevent using java.lang.invoke.StringConcatFactory when using -target 1.9 `-XDstringConcat=inline`, - }, " ")) + }) - pctx.StaticVariable("JavaVmFlags", JavaVmFlags) - pctx.StaticVariable("JavacVmFlags", JavacVmFlags) + exportedVars.ExportStringListStaticVariable("JavaVmFlags", javaVmFlagsList) + exportedVars.ExportStringListStaticVariable("JavacVmFlags", javacVmFlagsList) pctx.VariableConfigMethod("hostPrebuiltTag", android.Config.PrebuiltOS) @@ -184,6 +200,10 @@ func init() { hostJNIToolVariableWithSdkToolsPrebuilt("SignapkJniLibrary", "libconscrypt_openjdk_jni") } +func BazelJavaToolchainVars(config android.Config) string { + return android.BazelToolchainVars(config, exportedVars) +} + func hostBinToolVariableWithSdkToolsPrebuilt(name, tool string) { pctx.VariableFunc(name, func(ctx android.PackageVarContext) string { if ctx.Config().AlwaysUsePrebuiltSdks() { diff --git a/java/config/error_prone.go b/java/config/error_prone.go index 48681b5c9..5f853c812 100644 --- a/java/config/error_prone.go +++ b/java/config/error_prone.go @@ -16,8 +16,6 @@ package config import ( "strings" - - "android/soong/android" ) var ( @@ -31,23 +29,23 @@ var ( ) // Wrapper that grabs value of val late so it can be initialized by a later module's init function -func errorProneVar(name string, val *[]string, sep string) { - pctx.VariableFunc(name, func(android.PackageVarContext) string { +func errorProneVar(val *[]string, sep string) func() string { + return func() string { return strings.Join(*val, sep) - }) + } } func init() { - errorProneVar("ErrorProneClasspath", &ErrorProneClasspath, ":") - errorProneVar("ErrorProneChecksError", &ErrorProneChecksError, " ") - errorProneVar("ErrorProneChecksWarning", &ErrorProneChecksWarning, " ") - errorProneVar("ErrorProneChecksDefaultDisabled", &ErrorProneChecksDefaultDisabled, " ") - errorProneVar("ErrorProneChecksOff", &ErrorProneChecksOff, " ") - errorProneVar("ErrorProneFlags", &ErrorProneFlags, " ") - pctx.StaticVariable("ErrorProneChecks", strings.Join([]string{ + exportedVars.ExportVariableFuncVariable("ErrorProneClasspath", errorProneVar(&ErrorProneClasspath, ":")) + exportedVars.ExportVariableFuncVariable("ErrorProneChecksError", errorProneVar(&ErrorProneChecksError, " ")) + exportedVars.ExportVariableFuncVariable("ErrorProneChecksWarning", errorProneVar(&ErrorProneChecksWarning, " ")) + exportedVars.ExportVariableFuncVariable("ErrorProneChecksDefaultDisabled", errorProneVar(&ErrorProneChecksDefaultDisabled, " ")) + exportedVars.ExportVariableFuncVariable("ErrorProneChecksOff", errorProneVar(&ErrorProneChecksOff, " ")) + exportedVars.ExportVariableFuncVariable("ErrorProneFlags", errorProneVar(&ErrorProneFlags, " ")) + exportedVars.ExportStringListStaticVariable("ErrorProneChecks", []string{ "${ErrorProneChecksOff}", "${ErrorProneChecksError}", "${ErrorProneChecksWarning}", "${ErrorProneChecksDefaultDisabled}", - }, " ")) + }) } diff --git a/starlark_fmt/format.go b/starlark_fmt/format.go index 23eee59b3..3e51fa14c 100644 --- a/starlark_fmt/format.go +++ b/starlark_fmt/format.go @@ -39,21 +39,26 @@ func PrintBool(item bool) string { // PrintsStringList returns a Starlark-compatible string of a list of Strings/Labels. func PrintStringList(items []string, indentLevel int) string { - return PrintList(items, indentLevel, `"%s"`) + return PrintList(items, indentLevel, func(s string) string { + if strings.Contains(s, "\"") { + return `'''%s'''` + } + return `"%s"` + }) } // PrintList returns a Starlark-compatible string of list formmated as requested. -func PrintList(items []string, indentLevel int, formatString string) string { +func PrintList(items []string, indentLevel int, formatString func(string) string) string { if len(items) == 0 { return "[]" } else if len(items) == 1 { - return fmt.Sprintf("["+formatString+"]", items[0]) + return fmt.Sprintf("["+formatString(items[0])+"]", items[0]) } list := make([]string, 0, len(items)+2) list = append(list, "[") innerIndent := Indention(indentLevel + 1) for _, item := range items { - list = append(list, fmt.Sprintf(`%s`+formatString+`,`, innerIndent, item)) + list = append(list, fmt.Sprintf(`%s`+formatString(item)+`,`, innerIndent, item)) } list = append(list, Indention(indentLevel)+"]") return strings.Join(list, "\n") diff --git a/starlark_fmt/format_test.go b/starlark_fmt/format_test.go index 90f78ef7a..9450a31b3 100644 --- a/starlark_fmt/format_test.go +++ b/starlark_fmt/format_test.go @@ -18,6 +18,10 @@ import ( "testing" ) +func simpleFormat(s string) string { + return "%s" +} + func TestPrintEmptyStringList(t *testing.T) { in := []string{} indentLevel := 0 @@ -54,7 +58,7 @@ func TestPrintMultiElementStringList(t *testing.T) { func TestPrintEmptyList(t *testing.T) { in := []string{} indentLevel := 0 - out := PrintList(in, indentLevel, "%s") + out := PrintList(in, indentLevel, simpleFormat) expectedOut := "[]" if out != expectedOut { t.Errorf("Expected %q, got %q", expectedOut, out) @@ -64,7 +68,7 @@ func TestPrintEmptyList(t *testing.T) { func TestPrintSingleElementList(t *testing.T) { in := []string{"1"} indentLevel := 0 - out := PrintList(in, indentLevel, "%s") + out := PrintList(in, indentLevel, simpleFormat) expectedOut := `[1]` if out != expectedOut { t.Errorf("Expected %q, got %q", expectedOut, out) @@ -74,7 +78,7 @@ func TestPrintSingleElementList(t *testing.T) { func TestPrintMultiElementList(t *testing.T) { in := []string{"1", "2"} indentLevel := 0 - out := PrintList(in, indentLevel, "%s") + out := PrintList(in, indentLevel, simpleFormat) expectedOut := `[ 1, 2, @@ -87,7 +91,7 @@ func TestPrintMultiElementList(t *testing.T) { func TestListWithNonZeroIndent(t *testing.T) { in := []string{"1", "2"} indentLevel := 1 - out := PrintList(in, indentLevel, "%s") + out := PrintList(in, indentLevel, simpleFormat) expectedOut := `[ 1, 2,