Fix middleman conflicts in external repositories by appending the package path

Fixes #341.

--
MOS_MIGRATED_REVID=99390495

4 files changed, 47 insertions, 9 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
index 85a1450344..d67850a3e7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java
@@ -168,9 +168,12 @@ public final class MiddlemanFactory {
    * <p>Note: there's no need to synchronize this method; the only use of a field is via a call to
    * another synchronized method (getArtifact()).
    */
-  public Artifact createMiddlemanAllowMultiple(ActionRegistry registry,
-      ActionOwner owner, String purpose, Iterable<Artifact> inputs, Root middlemanDir) {
-    PathFragment stampName = new PathFragment("_middlemen/" + purpose);
+  public Artifact createMiddlemanAllowMultiple(ActionRegistry registry, ActionOwner owner,
+      PathFragment packageDirectory, String purpose, Iterable<Artifact> inputs, Root middlemanDir) {
+    String escapedPackageDirectory = Actions.escapedPath(packageDirectory.getPathString());
+    PathFragment stampName =
+        new PathFragment("_middlemen/" + (purpose.startsWith(escapedPackageDirectory)
+                                 ? purpose : (escapedPackageDirectory + purpose)));
     Artifact stampFile = artifactFactory.getDerivedArtifact(stampName, middlemanDir,
         actionRegistry.getOwner());
     MiddlemanAction.create(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java
index 23dd8d4ab7..2a8ff0652c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java
@@ -54,7 +54,7 @@ public final class CompilationHelper {
     }
     MiddlemanFactory factory = env.getMiddlemanFactory();
     return ImmutableList.of(factory.createMiddlemanAllowMultiple(
-        env, actionOwner, purpose, filesToBuild,
+        env, actionOwner, ruleContext.getPackageDirectory(), purpose, filesToBuild,
         ruleContext.getConfiguration().getMiddlemanDirectory()));
   }
 
@@ -85,8 +85,8 @@ public final class CompilationHelper {
     }
     MiddlemanFactory factory = env.getMiddlemanFactory();
     Iterable<Artifact> artifacts = dep.getProvider(FileProvider.class).getFilesToBuild();
-    return ImmutableList.of(factory.createMiddlemanAllowMultiple(
-        env, actionOwner, purpose, artifacts,
-        ruleContext.getConfiguration().getMiddlemanDirectory()));
+    return ImmutableList.of(
+        factory.createMiddlemanAllowMultiple(env, actionOwner, ruleContext.getPackageDirectory(),
+            purpose, artifacts, ruleContext.getConfiguration().getMiddlemanDirectory()));
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 751be081f5..5250665375 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -458,8 +458,9 @@ public class CppHelper {
       artifacts = symlinkedArtifacts;
       purpose += "_with_solib";
     }
-    return ImmutableList.of(factory.createMiddlemanAllowMultiple(
-        env, actionOwner, purpose, artifacts, configuration.getMiddlemanDirectory()));
+    return ImmutableList.of(
+        factory.createMiddlemanAllowMultiple(env, actionOwner, ruleContext.getPackageDirectory(),
+            purpose, artifacts, configuration.getMiddlemanDirectory()));
   }
 
   /**
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index ec04ffc63f..738d9e6640 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -72,4 +72,38 @@ function test_path_with_spaces() {
   bazel help &> $TEST_log || fail "Help failed"
 }
 
+# Tests for middleman conflict when using workspace repository
+function test_middleman_conflict() {
+  local test_repo1=$TEST_TMPDIR/repo1
+  local test_repo2=$TEST_TMPDIR/repo2
+
+  mkdir -p $test_repo1
+  mkdir -p $test_repo2
+  echo "1" >$test_repo1/test.in
+  echo "2" >$test_repo2/test.in
+  echo 'filegroup(name="test", srcs=["test.in"], visibility=["//visibility:public"])' \
+    >$test_repo1/BUILD
+  echo 'filegroup(name="test", srcs=["test.in"], visibility=["//visibility:public"])' \
+    >$test_repo2/BUILD
+  touch $test_repo1/WORKSPACE
+  touch $test_repo2/WORKSPACE
+
+  cat > WORKSPACE <<EOF
+local_repository(name = 'repo1', path='$test_repo1')
+local_repository(name = 'repo2', path='$test_repo2')
+EOF
+
+  cat > BUILD <<'EOF'
+genrule(
+  name = "test",
+  srcs = ["@repo1//:test", "@repo2//:test"],
+  outs = ["test.out"],
+  cmd = "cat $(SRCS) >$@"
+)
+EOF
+  bazel fetch //:test || fail "Fetch failed"
+  bazel build //:test || echo "Expected build to succeed"
+  check_eq "12" "$(cat bazel-genfiles/test.out | tr -d '[[:space:]]')"
+}
+
 run_suite "workspace tests"