Revert "Modify symlink_forest to rerun when soong_build has changed."
This reverts commit 23a4120c57ada5dba31669db16281cd1e37045c1.
Reason for revert: broke soong_integration
Change-Id: I4d51841756675b3745244d23e13aefda0916614b
diff --git a/bp2build/symlink_forest.go b/bp2build/symlink_forest.go
index 06e63ca..966b94a 100644
--- a/bp2build/symlink_forest.go
+++ b/bp2build/symlink_forest.go
@@ -22,6 +22,7 @@
"regexp"
"sort"
"strconv"
+ "strings"
"sync"
"sync/atomic"
@@ -31,12 +32,19 @@
)
// A tree structure that describes what to do at each directory in the created
-// symlink tree. Currently, it is used to enumerate which files/directories
+// symlink tree. Currently it is used to enumerate which files/directories
// should be excluded from symlinking. Each instance of "node" represents a file
// or a directory. If excluded is true, then that file/directory should be
// excluded from symlinking. Otherwise, the node is not excluded, but one of its
// descendants is (otherwise the node in question would not exist)
+// This is a version int written to a file called symlink_forest_version at the root of the
+// symlink forest. If the version here does not match the version in the file, then we'll
+// clean the whole symlink forest and recreate it. This number can be bumped whenever there's
+// an incompatible change to the forest layout or a bug in incrementality that needs to be fixed
+// on machines that may still have the bug present in their forest.
+const symlinkForestVersion = 2
+
type instructionsNode struct {
name string
excluded bool // If false, this is just an intermediate node
@@ -185,7 +193,7 @@
srcPath := shared.JoinPath(topdir, src)
dstPath := shared.JoinPath(topdir, dst)
- // Check whether a symlink already exists.
+ // Check if a symlink already exists.
if dstInfo, err := os.Lstat(dstPath); err != nil {
if !os.IsNotExist(err) {
fmt.Fprintf(os.Stderr, "Failed to lstat '%s': %s", dst, err)
@@ -232,49 +240,44 @@
return false
}
-// Returns the hash of the soong_build binary to determine whether we should
-// force symlink_forest to re-execute
-// This is similar to a version number increment - but that shouldn't be required
-// for every update to this file
-func getSoongBuildMTime() int64 {
- binaryPath, err := os.Executable()
- if err != nil {
- fmt.Fprintf(os.Stderr, "error finding executable path %s\n", err)
- os.Exit(1)
- }
-
- info, err := os.Stat(binaryPath)
- if err != nil {
- fmt.Fprintf(os.Stderr, "error stating executable path %s\n", err)
- }
-
- return info.ModTime().UnixMilli()
-}
-
-// maybeCleanSymlinkForest will remove the whole symlink forest directory if the soong_build
-// binary has changed since the last execution.
-func maybeCleanSymlinkForest(topdir, forest string, verbose bool, soongBuildMTime int64) error {
- mtimeFilePath := shared.JoinPath(topdir, forest, "soong_build_mtime")
- mtimeFileContents, err := os.ReadFile(mtimeFilePath)
+// maybeCleanSymlinkForest will remove the whole symlink forest directory if the version recorded
+// in the symlink_forest_version file is not equal to symlinkForestVersion.
+func maybeCleanSymlinkForest(topdir, forest string, verbose bool) error {
+ versionFilePath := shared.JoinPath(topdir, forest, "symlink_forest_version")
+ versionFileContents, err := os.ReadFile(versionFilePath)
if err != nil && !os.IsNotExist(err) {
return err
}
-
- if string(soongBuildMTime) != string(mtimeFileContents) {
+ versionFileString := strings.TrimSpace(string(versionFileContents))
+ symlinkForestVersionString := strconv.Itoa(symlinkForestVersion)
+ if err != nil || versionFileString != symlinkForestVersionString {
+ if verbose {
+ fmt.Fprintf(os.Stderr, "Old symlink_forest_version was %q, current is %q. Cleaning symlink forest before recreating...\n", versionFileString, symlinkForestVersionString)
+ }
err = os.RemoveAll(shared.JoinPath(topdir, forest))
if err != nil {
return err
}
}
-
return nil
}
-func writeSoongBuildMTimeFile(topdir, forest string, mtime int64) error {
- hashFilePath := shared.JoinPath(topdir, forest, "soong_build_mtime")
- contents := []byte(strconv.FormatInt(mtime, 10))
-
- return os.WriteFile(hashFilePath, contents, 0666)
+// maybeWriteVersionFile will write the symlink_forest_version file containing symlinkForestVersion
+// if it doesn't exist already. If it exists we know it must contain symlinkForestVersion because
+// we checked for that already in maybeCleanSymlinkForest
+func maybeWriteVersionFile(topdir, forest string) error {
+ versionFilePath := shared.JoinPath(topdir, forest, "symlink_forest_version")
+ _, err := os.Stat(versionFilePath)
+ if err != nil {
+ if !os.IsNotExist(err) {
+ return err
+ }
+ err = os.WriteFile(versionFilePath, []byte(strconv.Itoa(symlinkForestVersion)+"\n"), 0666)
+ if err != nil {
+ return err
+ }
+ }
+ return nil
}
// Recursively plants a symlink forest at forestDir. The symlink tree will
@@ -470,10 +473,7 @@
symlinkCount: atomic.Uint64{},
}
- // Check whether soong_build has been modified since the last run
- soongBuildMTime := getSoongBuildMTime()
-
- err := maybeCleanSymlinkForest(topdir, forest, verbose, soongBuildMTime)
+ err := maybeCleanSymlinkForest(topdir, forest, verbose)
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
@@ -491,10 +491,11 @@
deps = append(deps, dep)
}
- err = writeSoongBuildMTimeFile(topdir, forest, soongBuildMTime)
+ err = maybeWriteVersionFile(topdir, forest)
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
+
return deps, context.mkdirCount.Load(), context.symlinkCount.Load()
}
diff --git a/tests/run_integration_tests.sh b/tests/run_integration_tests.sh
index 2349993..6b9ff8b 100755
--- a/tests/run_integration_tests.sh
+++ b/tests/run_integration_tests.sh
@@ -10,7 +10,6 @@
"$TOP/build/soong/tests/persistent_bazel_test.sh"
"$TOP/build/soong/tests/soong_test.sh"
"$TOP/build/soong/tests/stale_metrics_files_test.sh"
-"$TOP/build/soong/tests/symlink_forest_rerun_test.sh"
"$TOP/prebuilts/build-tools/linux-x86/bin/py3-cmd" "$TOP/build/bazel/ci/rbc_dashboard.py" aosp_arm64-userdebug
# The following tests build against the full source tree and don't rely on the
diff --git a/tests/symlink_forest_rerun_test.sh b/tests/symlink_forest_rerun_test.sh
deleted file mode 100755
index b704222..0000000
--- a/tests/symlink_forest_rerun_test.sh
+++ /dev/null
@@ -1,42 +0,0 @@
-#!/bin/bash -eu
-
-set -o pipefail
-
-# Tests that symlink_Forest will rerun if soong_build has schanged
-
-source "$(dirname "$0")/lib.sh"
-
-function test_symlink_forest_reruns {
- setup
-
- mkdir -p a
- touch a/g.txt
- cat > a/Android.bp <<'EOF'
-filegroup {
- name: "g",
- srcs: ["g.txt"],
- }
-EOF
-
- run_soong g
-
- mtime=`cat out/soong/workspace/soong_build_mtime`
- # rerun with no changes - ensure that it hasn't changed
- run_soong g
- newmtime=`cat out/soong/workspace/soong_build_mtime`
- if [[ ! "$mtime" == "$mtime" ]]; then
- fail "symlink forest reran when it shouldn't have"
- fi
-
- # change exit codes to force a soong_build rebuild.
- sed -i 's/os.Exit(1)/os.Exit(2)/g' build/soong/bp2build/symlink_forest.go
-
- run_soong g
- newmtime=`cat out/soong/workspace/soong_build_mtime`
- if [[ "$mtime" == "$newmtime" ]]; then
- fail "symlink forest did not rerun when it should have"
- fi
-
-}
-
-scan_and_run_tests