From a3ae007641d85106e3cfda196bc0b15d49f923a3 Mon Sep 17 00:00:00 2001
From: Chris Parsons <cparsons@google.com>
Date: Wed, 10 May 2023 21:10:08 +0000
Subject: [PATCH] Touch soong_build output at the end of main

This fixes an incrementality bug that resulted in superfluous
build.ninja regeneration after a new glob definition is added.

Fixes: 279674820
Test: Treehugger
Test; New integration test in this CL

Change-Id: Ifefe66a0eb1c125e9ad5373d60437a1cb1e6fdec
---
 cmd/soong_build/main.go | 15 ++++++++-------
 tests/bootstrap_test.sh | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go
index 5de232664..10b09d3eb 100644
--- a/cmd/soong_build/main.go
+++ b/cmd/soong_build/main.go
@@ -540,10 +540,16 @@ func main() {
 		}
 		writeMetrics(configuration, ctx.EventHandler, metricsDir)
 	}
-	writeUsedEnvironmentFile(configuration, finalOutputFile)
+	writeUsedEnvironmentFile(configuration)
+
+	// Touch the output file so that it's the newest file created by soong_build.
+	// This is necessary because, if soong_build generated any files which
+	// are ninja inputs to the main output file, then ninja would superfluously
+	// rebuild this output file on the next build invocation.
+	touch(shared.JoinPath(topDir, finalOutputFile))
 }
 
-func writeUsedEnvironmentFile(configuration android.Config, finalOutputFile string) {
+func writeUsedEnvironmentFile(configuration android.Config) {
 	if usedEnvFile == "" {
 		return
 	}
@@ -562,11 +568,6 @@ func writeUsedEnvironmentFile(configuration android.Config, finalOutputFile stri
 	}
 	err = os.WriteFile(path, data, 0666)
 	maybeQuit(err, "error writing used environment file '%s'", usedEnvFile)
-
-	// Touch the output file so that it's not older than the file we just
-	// wrote. We can't write the environment file earlier because one an access
-	// new environment variables while writing it.
-	touch(shared.JoinPath(topDir, finalOutputFile))
 }
 
 func touch(path string) {
diff --git a/tests/bootstrap_test.sh b/tests/bootstrap_test.sh
index fda5ca086..59352477c 100755
--- a/tests/bootstrap_test.sh
+++ b/tests/bootstrap_test.sh
@@ -885,4 +885,37 @@ function test_queryview_null_build() {
   fi
 }
 
+# This test verifies that adding a new glob to a blueprint file only
+# causes build.ninja to be regenerated on the *next* build, and *not*
+# the build after. (This is a regression test for a bug where globs
+# resulted in two successive regenerations.)
+function test_new_glob_incrementality {
+  setup
+
+  run_soong nothing
+  local -r mtime1=$(stat -c "%y" out/soong/build.ninja)
+
+  mkdir -p globdefpkg/
+  cat > globdefpkg/Android.bp <<'EOF'
+filegroup {
+  name: "fg_with_glob",
+  srcs: ["*.txt"],
+}
+EOF
+
+  run_soong nothing
+  local -r mtime2=$(stat -c "%y" out/soong/build.ninja)
+
+  if [[ "$mtime1" == "$mtime2" ]]; then
+    fail "Ninja file was not regenerated, despite a new bp file"
+  fi
+
+  run_soong nothing
+  local -r mtime3=$(stat -c "%y" out/soong/build.ninja)
+
+  if [[ "$mtime2" != "$mtime3" ]]; then
+    fail "Ninja file was regenerated despite no previous bp changes"
+  fi
+}
+
 scan_and_run_tests