From 3a8d0fbedecf6e09cd2b210504084021f29089aa Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Thu, 2 Feb 2023 18:16:29 -0500 Subject: [PATCH] Only write soong_injection files if changed This also fixes determinism of these files (by ensuring that bazel requests and their configurations are sorted, as these are used in the soong_injection file output) Bug: 266983462 Test: Manually verified soong_injection files are the same among multiple runs Test: Unit test Test: m nothing Change-Id: I1568930549cff0bc5676825434cc448d16ebdd4e --- android/bazel_handler.go | 68 +++++++++++++++++++++++++++-------- android/bazel_handler_test.go | 27 +++++++++++++- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 8d4504175..7ba0023a5 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -183,9 +183,11 @@ type bazelPaths struct { // and their results after the requests have been made. type mixedBuildBazelContext struct { bazelRunner - paths *bazelPaths - requests map[cqueryKey]bool // cquery requests that have not yet been issued to Bazel - requestMutex sync.Mutex // requests can be written in parallel + paths *bazelPaths + // cquery requests that have not yet been issued to Bazel. This list is maintained + // in a sorted state, and is guaranteed to have no duplicates. + requests []cqueryKey + requestMutex sync.Mutex // requests can be written in parallel results map[cqueryKey]string // Results of cquery requests after Bazel invocations @@ -281,7 +283,29 @@ func (bazelCtx *mixedBuildBazelContext) QueueBazelRequest(label string, requestT key := makeCqueryKey(label, requestType, cfgKey) bazelCtx.requestMutex.Lock() defer bazelCtx.requestMutex.Unlock() - bazelCtx.requests[key] = true + + // Insert key into requests, maintaining the sort, and only if it's not duplicate. + keyString := key.String() + foundEqual := false + notLessThanKeyString := func(i int) bool { + s := bazelCtx.requests[i].String() + v := strings.Compare(s, keyString) + if v == 0 { + foundEqual = true + } + return v >= 0 + } + targetIndex := sort.Search(len(bazelCtx.requests), notLessThanKeyString) + if foundEqual { + return + } + + if targetIndex == len(bazelCtx.requests) { + bazelCtx.requests = append(bazelCtx.requests, key) + } else { + bazelCtx.requests = append(bazelCtx.requests[:targetIndex+1], bazelCtx.requests[targetIndex:]...) + bazelCtx.requests[targetIndex] = key + } } func (bazelCtx *mixedBuildBazelContext) GetOutputFiles(label string, cfgKey configKey) ([]string, error) { @@ -472,7 +496,6 @@ func NewBazelContext(c *config) (BazelContext, error) { return &mixedBuildBazelContext{ bazelRunner: &builtinBazelRunner{}, paths: &paths, - requests: make(map[cqueryKey]bool), modulesDefaultToBazel: c.BuildMode == BazelDevMode, bazelEnabledModules: enabledModules, bazelDisabledModules: disabledModules, @@ -712,14 +735,23 @@ config_node(name = "%s", configNodesSection := "" labelsByConfig := map[string][]string{} - for val := range context.requests { + + for _, val := range context.requests { labelString := fmt.Sprintf("\"@%s\"", val.label) configString := getConfigString(val) labelsByConfig[configString] = append(labelsByConfig[configString], labelString) } + // Configs need to be sorted to maintain determinism of the BUILD file. + sortedConfigs := make([]string, 0, len(labelsByConfig)) + for val := range labelsByConfig { + sortedConfigs = append(sortedConfigs, val) + } + sort.Slice(sortedConfigs, func(i, j int) bool { return sortedConfigs[i] < sortedConfigs[j] }) + allLabels := []string{} - for configString, labels := range labelsByConfig { + for _, configString := range sortedConfigs { + labels := labelsByConfig[configString] configTokens := strings.Split(configString, "|") if len(configTokens) != 2 { panic(fmt.Errorf("Unexpected config string format: %s", configString)) @@ -750,7 +782,7 @@ func indent(original string) string { // request type. func (context *mixedBuildBazelContext) cqueryStarlarkFileContents() []byte { requestTypeToCqueryIdEntries := map[cqueryRequest][]string{} - for val := range context.requests { + for _, val := range context.requests { cqueryId := getCqueryId(val) mapEntryString := fmt.Sprintf("%q : True", cqueryId) requestTypeToCqueryIdEntries[val.requestType] = @@ -929,7 +961,7 @@ func (context *mixedBuildBazelContext) InvokeBazel(config Config, ctx *Context) } // Clear requests. - context.requests = map[cqueryKey]bool{} + context.requests = []cqueryKey{} return nil } @@ -946,17 +978,17 @@ func (context *mixedBuildBazelContext) runCquery(ctx *Context) error { return err } } - if err := os.WriteFile(filepath.Join(soongInjectionPath, "WORKSPACE.bazel"), []byte{}, 0666); err != nil { + if err := writeFileBytesIfChanged(filepath.Join(soongInjectionPath, "WORKSPACE.bazel"), []byte{}, 0666); err != nil { return err } - if err := os.WriteFile(filepath.Join(mixedBuildsPath, "main.bzl"), context.mainBzlFileContents(), 0666); err != nil { + if err := writeFileBytesIfChanged(filepath.Join(mixedBuildsPath, "main.bzl"), context.mainBzlFileContents(), 0666); err != nil { return err } - if err := os.WriteFile(filepath.Join(mixedBuildsPath, "BUILD.bazel"), context.mainBuildFileContents(), 0666); err != nil { + if err := writeFileBytesIfChanged(filepath.Join(mixedBuildsPath, "BUILD.bazel"), context.mainBuildFileContents(), 0666); err != nil { return err } cqueryFileRelpath := filepath.Join(context.paths.injectedFilesDir(), "buildroot.cquery") - if err := os.WriteFile(absolutePath(cqueryFileRelpath), context.cqueryStarlarkFileContents(), 0666); err != nil { + if err := writeFileBytesIfChanged(absolutePath(cqueryFileRelpath), context.cqueryStarlarkFileContents(), 0666); err != nil { return err } @@ -977,7 +1009,7 @@ func (context *mixedBuildBazelContext) runCquery(ctx *Context) error { cqueryResults[splitLine[0]] = splitLine[1] } } - for val := range context.requests { + for _, val := range context.requests { if cqueryResult, ok := cqueryResults[getCqueryId(val)]; ok { context.results[val] = cqueryResult } else { @@ -988,6 +1020,14 @@ func (context *mixedBuildBazelContext) runCquery(ctx *Context) error { return nil } +func writeFileBytesIfChanged(path string, contents []byte, perm os.FileMode) error { + oldContents, err := os.ReadFile(path) + if err != nil || !bytes.Equal(contents, oldContents) { + err = os.WriteFile(path, contents, perm) + } + return nil +} + func (context *mixedBuildBazelContext) runAquery(config Config, ctx *Context) error { if ctx != nil { ctx.EventHandler.Begin("aquery") diff --git a/android/bazel_handler_test.go b/android/bazel_handler_test.go index 013e19c96..d97180205 100644 --- a/android/bazel_handler_test.go +++ b/android/bazel_handler_test.go @@ -168,6 +168,32 @@ func TestCoverageFlagsAfterInvokeBazel(t *testing.T) { } } +func TestBazelRequestsSorted(t *testing.T) { + bazelContext, _ := testBazelContext(t, map[bazelCommand]string{}) + + bazelContext.QueueBazelRequest("zzz", cquery.GetOutputFiles, configKey{"arm64_armv8-a", Android}) + bazelContext.QueueBazelRequest("ccc", cquery.GetApexInfo, configKey{"arm64_armv8-a", Android}) + bazelContext.QueueBazelRequest("duplicate", cquery.GetOutputFiles, configKey{"arm64_armv8-a", Android}) + bazelContext.QueueBazelRequest("duplicate", cquery.GetOutputFiles, configKey{"arm64_armv8-a", Android}) + bazelContext.QueueBazelRequest("xxx", cquery.GetOutputFiles, configKey{"arm64_armv8-a", Linux}) + bazelContext.QueueBazelRequest("aaa", cquery.GetOutputFiles, configKey{"arm64_armv8-a", Android}) + bazelContext.QueueBazelRequest("aaa", cquery.GetOutputFiles, configKey{"otherarch", Android}) + bazelContext.QueueBazelRequest("bbb", cquery.GetOutputFiles, configKey{"otherarch", Android}) + + if len(bazelContext.requests) != 7 { + t.Error("Expected 7 request elements, but got", len(bazelContext.requests)) + } + + lastString := "" + for _, val := range bazelContext.requests { + thisString := val.String() + if thisString <= lastString { + t.Errorf("Requests are not ordered correctly. '%s' came before '%s'", lastString, thisString) + } + lastString = thisString + } +} + func verifyExtraFlags(t *testing.T, config Config, expected string) string { bazelContext, _ := testBazelContext(t, map[bazelCommand]string{}) @@ -204,7 +230,6 @@ func testBazelContext(t *testing.T, bazelCommandResults map[bazelCommand]string) return &mixedBuildBazelContext{ bazelRunner: runner, paths: &p, - requests: map[cqueryKey]bool{}, }, p.soongOutDir }