commit 0ad729f79213e7d0d80260c092b557751d757b6b
author Laszlo Csomor <laszlocsomor@google.com> Wed Dec 02 15:20:35 2015 +0000
committer Kristina Chodorow <kchodorow@google.com> Wed Dec 02 22:26:34 2015 +0000
tree fdec4af039557e71173376bacccfcf3c6932c1b8
parent 07b0ba6079022d9a9643d46b28aa4d2a75334b35

Fix a bug in the Google-internal rule "Fileset" (not in Bazel).

--
MOS_MIGRATED_REVID=109195426

src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
index f227182..d2e0132 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
@@ -75,7 +75,9 @@
       }
 
       for (FilesetOutputSymlink s : nested.getSymlinks()) {
-        maybeStoreSymlink(s, t.getDestPath(), exclusions, outputSymlinks);
+        if (!exclusions.contains(s.name.getPathString())) {
+          maybeStoreSymlink(s, t.getDestPath(), outputSymlinks);
+        }
       }
     } else {
       // The "nested" traversal params are absent if and only if the "direct" traversal params are
@@ -163,6 +165,15 @@
 
       // Create one output symlink for each entry in the results.
       for (ResolvedFile f : results) {
+        // The linkName has to be under the traversal's root, which is also the prefix to remove.
+        PathFragment linkName = f.getNameInSymlinkTree().relativeTo(prefixToRemove);
+
+        // Check whether the symlink is excluded before attempting to resolve it.
+        // It may be dangling, but excluding it is still fine.
+        if (exclusions.contains(linkName.getPathString())) {
+          continue;
+        }
+
         PathFragment targetName;
         try {
           targetName = f.getTargetInSymlinkTree(direct.isFollowingSymlinks());
@@ -172,11 +183,7 @@
 
         // Metadata field must be present. It can only be absent when stripped by tests.
         String metadata = Integer.toHexString(f.metadata.get().hashCode());
-
-        // The linkName has to be under the traversal's root, which is also the prefix to remove.
-        PathFragment linkName = f.getNameInSymlinkTree().relativeTo(prefixToRemove);
-        maybeStoreSymlink(linkName, targetName, metadata, t.getDestPath(), exclusions,
-            outputSymlinks);
+        maybeStoreSymlink(linkName, targetName, metadata, t.getDestPath(), outputSymlinks);
       }
     }
 
@@ -185,20 +192,16 @@
 
   /** Stores an output symlink unless it's excluded or would overwrite an existing one. */
   private static void maybeStoreSymlink(FilesetOutputSymlink nestedLink, PathFragment destPath,
-      Set<String> exclusions, Map<PathFragment, FilesetOutputSymlink> result) {
-    maybeStoreSymlink(nestedLink.name, nestedLink.target, nestedLink.metadata, destPath,
-        exclusions, result);
+      Map<PathFragment, FilesetOutputSymlink> result) {
+    maybeStoreSymlink(nestedLink.name, nestedLink.target, nestedLink.metadata, destPath, result);
   }
 
   /** Stores an output symlink unless it's excluded or would overwrite an existing one. */
   private static void maybeStoreSymlink(PathFragment linkName, PathFragment linkTarget,
-      String metadata, PathFragment destPath, Set<String> exclusions,
-      Map<PathFragment, FilesetOutputSymlink> result) {
-    if (!exclusions.contains(linkName.getPathString())) {
-      linkName = destPath.getRelative(linkName);
-      if (!result.containsKey(linkName)) {
-        result.put(linkName, new FilesetOutputSymlink(linkName, linkTarget, metadata));
-      }
+      String metadata, PathFragment destPath, Map<PathFragment, FilesetOutputSymlink> result) {
+    linkName = destPath.getRelative(linkName);
+    if (!result.containsKey(linkName)) {
+      result.put(linkName, new FilesetOutputSymlink(linkName, linkTarget, metadata));
     }
   }