commit d3a726cdb6fbb0483c653264bcd05757f1f04824
author Damien Martin-Guillerez <dmarting@google.com> Wed Jul 29 17:51:37 2015 +0000
committer Damien Martin-Guillerez <dmarting@google.com> Thu Jul 30 11:31:14 2015 +0000
tree ac8d7e7497b5909a5cff523e6291ad7efa70a825
parent 8ff5b3c00216392cd13dba61093c20501b377329

Fix middleman conflicts in external repositories by appending the package path

Fixes #341.

--
MOS_MIGRATED_REVID=99390495

src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java

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 85a1450..d67850a 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 @@
    * <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(

src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java

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 23dd8d4..2a8ff06 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 @@
     }
     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 @@
     }
     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()));
   }
 }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java

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 751be08..5250665 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 @@
       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()));
   }
 
   /**

src/test/shell/bazel/workspace_test.sh

diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index ec04ffc..738d9e6 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -72,4 +72,38 @@
   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"