diff --git a/Android.bp b/Android.bp index afbae172a..1dfac875a 100644 --- a/Android.bp +++ b/Android.bp @@ -172,6 +172,7 @@ bootstrap_go_package { "cc/vndk_prebuilt.go", "cc/xom.go", + "cc/cflag_artifacts.go", "cc/cmakelists.go", "cc/compdb.go", "cc/compiler.go", @@ -304,7 +305,6 @@ bootstrap_go_package { "java/jdeps_test.go", "java/kotlin_test.go", "java/plugin_test.go", - "java/robolectric_test.go", "java/sdk_test.go", ], pluginFor: ["soong_build"], diff --git a/android/util.go b/android/util.go index 010244209..71ded5e07 100644 --- a/android/util.go +++ b/android/util.go @@ -319,3 +319,36 @@ func SplitFileExt(name string) (string, string, string) { return root, suffix, ext } + +// ShardPaths takes a Paths, and returns a slice of Paths where each one has at most shardSize paths. +func ShardPaths(paths Paths, shardSize int) []Paths { + if len(paths) == 0 { + return nil + } + ret := make([]Paths, 0, (len(paths)+shardSize-1)/shardSize) + for len(paths) > shardSize { + ret = append(ret, paths[0:shardSize]) + paths = paths[shardSize:] + } + if len(paths) > 0 { + ret = append(ret, paths) + } + return ret +} + +// ShardStrings takes a slice of strings, and returns a slice of slices of strings where each one has at most shardSize +// elements. +func ShardStrings(s []string, shardSize int) [][]string { + if len(s) == 0 { + return nil + } + ret := make([][]string, 0, (len(s)+shardSize-1)/shardSize) + for len(s) > shardSize { + ret = append(ret, s[0:shardSize]) + s = s[shardSize:] + } + if len(s) > 0 { + ret = append(ret, s) + } + return ret +} diff --git a/android/util_test.go b/android/util_test.go index 1df1c5af5..90fefeede 100644 --- a/android/util_test.go +++ b/android/util_test.go @@ -469,3 +469,102 @@ func TestSplitFileExt(t *testing.T) { } }) } + +func Test_Shard(t *testing.T) { + type args struct { + strings []string + shardSize int + } + tests := []struct { + name string + args args + want [][]string + }{ + { + name: "empty", + args: args{ + strings: nil, + shardSize: 1, + }, + want: [][]string(nil), + }, + { + name: "single shard", + args: args{ + strings: []string{"a", "b"}, + shardSize: 2, + }, + want: [][]string{{"a", "b"}}, + }, + { + name: "single short shard", + args: args{ + strings: []string{"a", "b"}, + shardSize: 3, + }, + want: [][]string{{"a", "b"}}, + }, + { + name: "shard per input", + args: args{ + strings: []string{"a", "b", "c"}, + shardSize: 1, + }, + want: [][]string{{"a"}, {"b"}, {"c"}}, + }, + { + name: "balanced shards", + args: args{ + strings: []string{"a", "b", "c", "d"}, + shardSize: 2, + }, + want: [][]string{{"a", "b"}, {"c", "d"}}, + }, + { + name: "unbalanced shards", + args: args{ + strings: []string{"a", "b", "c"}, + shardSize: 2, + }, + want: [][]string{{"a", "b"}, {"c"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("strings", func(t *testing.T) { + if got := ShardStrings(tt.args.strings, tt.args.shardSize); !reflect.DeepEqual(got, tt.want) { + t.Errorf("ShardStrings(%v, %v) = %v, want %v", + tt.args.strings, tt.args.shardSize, got, tt.want) + } + }) + + t.Run("paths", func(t *testing.T) { + stringsToPaths := func(strings []string) Paths { + if strings == nil { + return nil + } + paths := make(Paths, len(strings)) + for i, s := range strings { + paths[i] = PathForTesting(s) + } + return paths + } + + paths := stringsToPaths(tt.args.strings) + + var want []Paths + if sWant := tt.want; sWant != nil { + want = make([]Paths, len(sWant)) + for i, w := range sWant { + want[i] = stringsToPaths(w) + } + } + + if got := ShardPaths(paths, tt.args.shardSize); !reflect.DeepEqual(got, want) { + t.Errorf("ShardPaths(%v, %v) = %v, want %v", + paths, tt.args.shardSize, got, want) + } + }) + }) + } +} diff --git a/cc/cflag_artifacts.go b/cc/cflag_artifacts.go new file mode 100644 index 000000000..9ed3876ab --- /dev/null +++ b/cc/cflag_artifacts.go @@ -0,0 +1,188 @@ +package cc + +import ( + "fmt" + "sort" + "strings" + + "github.com/google/blueprint/proptools" + + "android/soong/android" +) + +func init() { + android.RegisterSingletonType("cflag_artifacts_text", cflagArtifactsTextFactory) +} + +var ( + TrackedCFlags = []string{ + "-Wall", + "-Werror", + "-Wextra", + "-Wthread-safety", + "-O3", + } + + TrackedCFlagsDir = []string{ + "device/google/", + "vendor/google/", + } +) + +const FileBP = 50 + +// Stores output files. +type cflagArtifactsText struct { + interOutputs map[string]android.WritablePaths + outputs android.WritablePaths +} + +// allowedDir verifies if the directory/project is part of the TrackedCFlagsDir +// filter. +func allowedDir(subdir string) bool { + subdir += "/" + for _, prefix := range TrackedCFlagsDir { + if strings.HasPrefix(subdir, prefix) { + return true + } + } + return false +} + +func (s *cflagArtifactsText) genFlagFilename(flag string) string { + return fmt.Sprintf("module_cflags%s.txt", flag) +} + +// incrementFile is used to generate an output path object with the passed in flag +// and part number. +// e.g. FLAG + part # -> out/soong/cflags/module_cflags-FLAG.txt.0 +func (s *cflagArtifactsText) incrementFile(ctx android.SingletonContext, + flag string, part int) (string, android.OutputPath) { + + filename := fmt.Sprintf("%s.%d", s.genFlagFilename(flag), part) + filepath := android.PathForOutput(ctx, "cflags", filename) + s.interOutputs[flag] = append(s.interOutputs[flag], filepath) + return filename, filepath +} + +// GenCFlagArtifactParts is used to generate the build rules which produce the +// intermediary files for each desired C Flag artifact +// e.g. module_cflags-FLAG.txt.0, module_cflags-FLAG.txt.1, ... +func (s *cflagArtifactsText) GenCFlagArtifactParts(ctx android.SingletonContext, + flag string, using bool, modules []string, part int) int { + + cleanedName := strings.Replace(flag, "=", "_", -1) + filename, filepath := s.incrementFile(ctx, cleanedName, part) + rule := android.NewRuleBuilder() + rule.Command().Textf("rm -f %s", filepath.String()) + + if using { + rule.Command(). + Textf("echo '# Modules using %s'", flag). + FlagWithOutput(">> ", filepath) + } else { + rule.Command(). + Textf("echo '# Modules not using %s'", flag). + FlagWithOutput(">> ", filepath) + } + + length := len(modules) + + if length == 0 { + rule.Build(pctx, ctx, filename, "gen "+filename) + part++ + } + + // Following loop splits the module list for each tracked C Flag into + // chunks of length FileBP (file breakpoint) and generates a partial artifact + // (intermediary file) build rule for each split. + moduleShards := android.ShardStrings(modules, FileBP) + for index, shard := range moduleShards { + rule.Command(). + Textf("for m in %s; do echo $m", + strings.Join(proptools.ShellEscapeList(shard), " ")). + FlagWithOutput(">> ", filepath). + Text("; done") + rule.Build(pctx, ctx, filename, "gen "+filename) + + if index+1 != len(moduleShards) { + filename, filepath = s.incrementFile(ctx, cleanedName, part+index+1) + rule = android.NewRuleBuilder() + rule.Command().Textf("rm -f %s", filepath.String()) + } + } + + return part + len(moduleShards) +} + +// GenCFlagArtifacts is used to generate build rules which combine the +// intermediary files of a specific tracked flag into a single C Flag artifact +// for each tracked flag. +// e.g. module_cflags-FLAG.txt.0 + module_cflags-FLAG.txt.1 = module_cflags-FLAG.txt +func (s *cflagArtifactsText) GenCFlagArtifacts(ctx android.SingletonContext) { + // Scans through s.interOutputs and creates a build rule for each tracked C + // Flag that concatenates the associated intermediary file into a single + // artifact. + for _, flag := range TrackedCFlags { + // Generate build rule to combine related intermediary files into a + // C Flag artifact + rule := android.NewRuleBuilder() + filename := s.genFlagFilename(flag) + outputpath := android.PathForOutput(ctx, "cflags", filename) + rule.Command(). + Text("cat"). + Inputs(s.interOutputs[flag].Paths()). + FlagWithOutput("> ", outputpath) + rule.Build(pctx, ctx, filename, "gen "+filename) + s.outputs = append(s.outputs, outputpath) + } +} + +func (s *cflagArtifactsText) GenerateBuildActions(ctx android.SingletonContext) { + modulesWithCFlag := make(map[string][]string) + + // Scan through all modules, selecting the ones that are part of the filter, + // and then storing into a map which tracks whether or not tracked C flag is + // used or not. + ctx.VisitAllModules(func(module android.Module) { + if ccModule, ok := module.(*Module); ok { + if allowedDir(ctx.ModuleDir(ccModule)) { + cflags := ccModule.flags.CFlags + cppflags := ccModule.flags.CppFlags + module := fmt.Sprintf("%s:%s (%s)", + ctx.BlueprintFile(ccModule), + ctx.ModuleName(ccModule), + ctx.ModuleSubDir(ccModule)) + for _, flag := range TrackedCFlags { + if inList(flag, cflags) || inList(flag, cppflags) { + modulesWithCFlag[flag] = append(modulesWithCFlag[flag], module) + } else { + modulesWithCFlag["!"+flag] = append(modulesWithCFlag["!"+flag], module) + } + } + } + } + }) + + // Traversing map and setting up rules to produce intermediary files which + // contain parts of each expected C Flag artifact. + for _, flag := range TrackedCFlags { + sort.Strings(modulesWithCFlag[flag]) + part := s.GenCFlagArtifactParts(ctx, flag, true, modulesWithCFlag[flag], 0) + sort.Strings(modulesWithCFlag["!"+flag]) + s.GenCFlagArtifactParts(ctx, flag, false, modulesWithCFlag["!"+flag], part) + } + + // Combine intermediary files into a single C Flag artifact. + s.GenCFlagArtifacts(ctx) +} + +func cflagArtifactsTextFactory() android.Singleton { + return &cflagArtifactsText{ + interOutputs: make(map[string]android.WritablePaths), + } +} + +func (s *cflagArtifactsText) MakeVars(ctx android.MakeVarsContext) { + ctx.Strict("SOONG_MODULES_CFLAG_ARTIFACTS", strings.Join(s.outputs.Strings(), " ")) +} diff --git a/java/aapt2.go b/java/aapt2.go index f0eb99cee..cfe0deab1 100644 --- a/java/aapt2.go +++ b/java/aapt2.go @@ -63,7 +63,7 @@ var aapt2CompileRule = pctx.AndroidStaticRule("aapt2Compile", func aapt2Compile(ctx android.ModuleContext, dir android.Path, paths android.Paths, flags []string) android.WritablePaths { - shards := shardPaths(paths, AAPT2_SHARD_SIZE) + shards := android.ShardPaths(paths, AAPT2_SHARD_SIZE) ret := make(android.WritablePaths, 0, len(paths)) diff --git a/java/java.go b/java/java.go index ce72c2760..f7b0f53ba 100644 --- a/java/java.go +++ b/java/java.go @@ -580,18 +580,6 @@ func hasSrcExt(srcs []string, ext string) bool { return false } -func shardPaths(paths android.Paths, shardSize int) []android.Paths { - ret := make([]android.Paths, 0, (len(paths)+shardSize-1)/shardSize) - for len(paths) > shardSize { - ret = append(ret, paths[0:shardSize]) - paths = paths[shardSize:] - } - if len(paths) > 0 { - ret = append(ret, paths) - } - return ret -} - func (j *Module) hasSrcExt(ext string) bool { return hasSrcExt(j.properties.Srcs, ext) } @@ -1156,7 +1144,7 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { shardSize := int(*(j.properties.Javac_shard_size)) var shardSrcs []android.Paths if len(uniqueSrcFiles) > 0 { - shardSrcs = shardPaths(uniqueSrcFiles, shardSize) + shardSrcs = android.ShardPaths(uniqueSrcFiles, shardSize) for idx, shardSrc := range shardSrcs { classes := j.compileJavaClasses(ctx, jarName, idx, shardSrc, nil, flags, extraJarDeps) diff --git a/java/robolectric.go b/java/robolectric.go index dbf54c9b3..b7646eb45 100644 --- a/java/robolectric.go +++ b/java/robolectric.go @@ -123,25 +123,6 @@ func (r *robolectricTest) GenerateAndroidBuildActions(ctx android.ModuleContext) } } -func shardTests(paths []string, shards int) [][]string { - if shards > len(paths) { - shards = len(paths) - } - if shards == 0 { - return nil - } - ret := make([][]string, 0, shards) - shardSize := (len(paths) + shards - 1) / shards - for len(paths) > shardSize { - ret = append(ret, paths[0:shardSize]) - paths = paths[shardSize:] - } - if len(paths) > 0 { - ret = append(ret, paths) - } - return ret -} - func generateRoboTestConfig(ctx android.ModuleContext, outputFile android.WritablePath, instrumentedApp *AndroidApp) { manifest := instrumentedApp.mergedManifestFile resourceApk := instrumentedApp.outputFile @@ -183,7 +164,9 @@ func (r *robolectricTest) AndroidMkEntries() android.AndroidMkEntries { entries.ExtraFooters = []android.AndroidMkExtraFootersFunc{ func(w io.Writer, name, prefix, moduleDir string, entries *android.AndroidMkEntries) { if s := r.robolectricProperties.Test_options.Shards; s != nil && *s > 1 { - shards := shardTests(r.tests, int(*s)) + numShards := int(*s) + shardSize := (len(r.tests) + numShards - 1) / numShards + shards := android.ShardStrings(r.tests, shardSize) for i, shard := range shards { r.writeTestRunner(w, name, "Run"+name+strconv.Itoa(i), shard) } diff --git a/java/robolectric_test.go b/java/robolectric_test.go deleted file mode 100644 index e89c6e74c..000000000 --- a/java/robolectric_test.go +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright 2019 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 java - -import ( - "reflect" - "testing" -) - -func Test_shardTests(t *testing.T) { - type args struct { - paths []string - shards int - } - tests := []struct { - name string - args args - want [][]string - }{ - { - name: "empty", - args: args{ - paths: nil, - shards: 1, - }, - want: [][]string(nil), - }, - { - name: "too many shards", - args: args{ - paths: []string{"a", "b"}, - shards: 3, - }, - want: [][]string{{"a"}, {"b"}}, - }, - { - name: "single shard", - args: args{ - paths: []string{"a", "b"}, - shards: 1, - }, - want: [][]string{{"a", "b"}}, - }, - { - name: "shard per input", - args: args{ - paths: []string{"a", "b", "c"}, - shards: 3, - }, - want: [][]string{{"a"}, {"b"}, {"c"}}, - }, - { - name: "balanced shards", - args: args{ - paths: []string{"a", "b", "c", "d"}, - shards: 2, - }, - want: [][]string{{"a", "b"}, {"c", "d"}}, - }, - { - name: "unbalanced shards", - args: args{ - paths: []string{"a", "b", "c"}, - shards: 2, - }, - want: [][]string{{"a", "b"}, {"c"}}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := shardTests(tt.args.paths, tt.args.shards); !reflect.DeepEqual(got, tt.want) { - t.Errorf("shardTests() = %v, want %v", got, tt.want) - } - }) - } -}