From b21166e236e757fb1cd3db648fbc034a51817fef Mon Sep 17 00:00:00 2001
From: "Lukacs T. Berki" <lberki@google.com>
Date: Wed, 21 Apr 2021 11:58:36 +0200
Subject: [PATCH] Make symlink_forest.go prefer generated files.

Now, if the same file exists in the generated tree and the source tree,
it symlinks in the generated file instead of failing outright.

Drive-by fix: print errors for all conflicts instead of bailing out on
the first one.

Test: Presubmits (including the two new tests)
Change-Id: Ifb5b3fc89b5454d231966bfa4e61c22cd69834f3
---
 bp2build/symlink_forest.go | 24 ++++++++++++-----
 tests/bootstrap_test.sh    | 53 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/bp2build/symlink_forest.go b/bp2build/symlink_forest.go
index db7bef877..15a63356e 100644
--- a/bp2build/symlink_forest.go
+++ b/bp2build/symlink_forest.go
@@ -94,7 +94,7 @@ func symlinkIntoForest(topdir, dst, src string) {
 // contain every file in buildFilesDir and srcDir excluding the files in
 // exclude. Collects every directory encountered during the traversal of srcDir
 // into acc.
-func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir string, srcDir string, exclude *node, acc *[]string) {
+func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir string, srcDir string, exclude *node, acc *[]string, okay *bool) {
 	if exclude != nil && exclude.excluded {
 		// This directory is not needed, bail out
 		return
@@ -149,7 +149,7 @@ func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir
 			if buildFilesChildEntry.IsDir() && excludeChild != nil {
 				// Not in the source tree, but we have to exclude something from under
 				// this subtree, so descend
-				plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc)
+				plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc, okay)
 			} else {
 				// Not in the source tree, symlink BUILD file
 				symlinkIntoForest(topdir, forestChild, buildFilesChild)
@@ -158,20 +158,26 @@ func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir
 			if srcChildEntry.IsDir() && excludeChild != nil {
 				// Not in the build file tree, but we have to exclude something from
 				// under this subtree, so descend
-				plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc)
+				plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc, okay)
 			} else {
 				// Not in the build file tree, symlink source tree, carry on
 				symlinkIntoForest(topdir, forestChild, srcChild)
 			}
 		} else if srcChildEntry.IsDir() && buildFilesChildEntry.IsDir() {
 			// Both are directories. Descend.
-			plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc)
+			plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc, okay)
+		} else if !srcChildEntry.IsDir() && !buildFilesChildEntry.IsDir() {
+			// Neither is a directory. Prioritize BUILD files generated by bp2build
+			// over any BUILD file imported into external/.
+			fmt.Fprintf(os.Stderr, "Both '%s' and '%s' exist, symlinking the former to '%s'\n",
+				buildFilesChild, srcChild, forestChild)
+			symlinkIntoForest(topdir, forestChild, buildFilesChild)
 		} else {
 			// Both exist and one is a file. This is an error.
 			fmt.Fprintf(os.Stderr,
-				"Conflict in workspace symlink tree creation: both '%s' and '%s' exist and at least one of them is a file\n",
+				"Conflict in workspace symlink tree creation: both '%s' and '%s' exist and exactly one is a directory\n",
 				srcChild, buildFilesChild)
-			os.Exit(1)
+			*okay = false
 		}
 	}
 }
@@ -184,6 +190,10 @@ func PlantSymlinkForest(topdir string, forest string, buildFiles string, srcDir
 	deps := make([]string, 0)
 	os.RemoveAll(shared.JoinPath(topdir, forest))
 	excludeTree := treeFromExcludePathList(exclude)
-	plantSymlinkForestRecursive(topdir, forest, buildFiles, srcDir, excludeTree, &deps)
+	okay := true
+	plantSymlinkForestRecursive(topdir, forest, buildFiles, srcDir, excludeTree, &deps, &okay)
+	if !okay {
+		os.Exit(1)
+	}
 	return deps
 }
diff --git a/tests/bootstrap_test.sh b/tests/bootstrap_test.sh
index 1a39464bd..a3429ddca 100755
--- a/tests/bootstrap_test.sh
+++ b/tests/bootstrap_test.sh
@@ -609,6 +609,57 @@ EOF
   [[ -L out/soong/workspace/a/a2.txt ]] || fail "a/a2.txt not symlinked"
 }
 
+function test_bp2build_build_file_precedence {
+  setup
+
+  mkdir -p a
+  touch a/a.txt
+  touch a/BUILD
+  cat > a/Android.bp <<EOF
+filegroup {
+  name: "a",
+  srcs: ["a.txt"],
+  bazel_module: { bp2build_available: true },
+}
+EOF
+
+  GENERATE_BAZEL_FILES=1 run_soong
+  [[ -L out/soong/workspace/a/BUILD ]] || fail "BUILD file not symlinked"
+  [[ "$(readlink -f out/soong/workspace/a/BUILD)" =~ bp2build/a/BUILD$ ]] \
+    || fail "BUILD files symlinked to the wrong place"
+}
+
+function test_bp2build_reports_multiple_errors {
+  setup
+
+  mkdir -p a/BUILD
+  touch a/a.txt
+  cat > a/Android.bp <<EOF
+filegroup {
+  name: "a",
+  srcs: ["a.txt"],
+  bazel_module: { bp2build_available: true },
+}
+EOF
+
+  mkdir -p b/BUILD
+  touch b/b.txt
+  cat > b/Android.bp <<EOF
+filegroup {
+  name: "b",
+  srcs: ["b.txt"],
+  bazel_module: { bp2build_available: true },
+}
+EOF
+
+  if GENERATE_BAZEL_FILES=1 run_soong >& "$MOCK_TOP/errors"; then
+    fail "Build should have failed"
+  fi
+
+  grep -q "a/BUILD' exist" "$MOCK_TOP/errors" || fail "Error for a/BUILD not found"
+  grep -q "b/BUILD' exist" "$MOCK_TOP/errors" || fail "Error for b/BUILD not found"
+}
+
 test_smoke
 test_null_build
 test_null_build_after_docs
@@ -628,3 +679,5 @@ test_bp2build_add_android_bp
 test_bp2build_add_to_glob
 test_bp2build_bazel_workspace_structure
 test_bp2build_bazel_workspace_add_file
+test_bp2build_build_file_precedence
+test_bp2build_reports_multiple_errors