rdar://105156287 ([ownership-analysis] Restrict mustScanAfterPaths to user scripts)

Author: johari

include/llbuild/BuildSystem/Command.h

@@ -55,7 +55,7 @@ class Command : public basic::JobDescriptor {
 
   std::vector<BuildNode*> inputs;
   std::vector<BuildNode*> outputs;
-  bool excludeFromOwnershipAnalysis = false;
+  bool repairViaOwnershipAnalysis = false;
 
   StringRef getName() const { return name; }
 

lib/BuildSystem/BuildFile.cpp

@@ -109,7 +109,7 @@ class OwnershipAnalysis {
 public:
   std::vector<std::pair<BuildNode*, Command*>> outputNodesAndCommands;
 
-  std::vector<BuildNode*> allDirectoryInputNodes;
+  std::vector<std::pair<BuildNode*, Command*>> directoryInputNodesAndCommands;
 
   OwnershipAnalysis(const BuildDescription::command_set& commands, BuildFileDelegate& fileDelegate): fileDelegate(fileDelegate) {
     // Extract outputs and directory inputs of all commands
@@ -123,7 +123,7 @@ class OwnershipAnalysis {
 
       for (auto input: command->getInputs()) {
         if (input->isDirectory()) {
-          allDirectoryInputNodes.push_back(input);
+          directoryInputNodesAndCommands.push_back(std::pair<BuildNode*, Command*>(input, command));
         }
       }
     }
@@ -140,7 +140,7 @@ class OwnershipAnalysis {
   /// Establish ownerships
   bool establishOwnerships() {
     for (auto outputNodeAndCommand: outputNodesAndCommands) {
-        if (outputNodeAndCommand.second->isExternalCommand() && outputNodeAndCommand.second->excludeFromOwnershipAnalysis == false) {
+        if (outputNodeAndCommand.second->isExternalCommand() && outputNodeAndCommand.second->repairViaOwnershipAnalysis == true) {
           Command *owner = includedOwnerOf(outputNodeAndCommand.first->getName());
           if (owner == nullptr) {
             setOwner(outputNodeAndCommand.first, outputNodeAndCommand.second);
@@ -224,12 +224,14 @@ class OwnershipAnalysis {
   //
   // This ensures TaskC will wait until TaskB is finished.
   void amendOutputOfOwnersWithConsumedSubpaths() {
-    for (auto inputNode: allDirectoryInputNodes) {
-      Command *owner = includedOwnerOf(inputNode->getName());
+    for (auto directoryInputNodeAndCommand: directoryInputNodesAndCommands) {
+      Command *owner = includedOwnerOf(directoryInputNodeAndCommand.first->getName());
       if (owner != nullptr) {
         auto ownerOutputs = owner->getOutputs();
-        if (std::find(ownerOutputs.begin(), ownerOutputs.end(), inputNode) == ownerOutputs.end()) {
-          owner->addOutput(inputNode);
+        if (std::find(ownerOutputs.begin(), ownerOutputs.end(), directoryInputNodeAndCommand.first) == ownerOutputs.end()) {
+          if (owner->repairViaOwnershipAnalysis) {
+            owner->addOutput(directoryInputNodeAndCommand.first);
+          }
         }
       }
     }
@@ -245,25 +247,27 @@ class OwnershipAnalysis {
   // We should add "a.txt" and "b.txt" to mustScanAfterPaths of "unowned-directory/".
   // This ensures TaskC will wait until TaskA and TaskB are finished.
   void deferScanningUnownedInputsUntilSubpathsAvailable() {
-    auto unownedDirectoryInputNodes = std::vector<BuildNode*>();
-    std::copy_if(allDirectoryInputNodes.begin(),
-                 allDirectoryInputNodes.end(),
-                 std::back_inserter(unownedDirectoryInputNodes),
-                 [this](const BuildNode* node) -> bool {
-      return isIncludedUnownedNode(node);
+    auto unownedDirectoryInputNodesAndConsumingCommands = std::vector<std::pair<BuildNode*, Command*>>();
+    std::copy_if(directoryInputNodesAndCommands.begin(),
+                 directoryInputNodesAndCommands.end(),
+                 std::back_inserter(unownedDirectoryInputNodesAndConsumingCommands),
+                 [this](const std::pair<BuildNode*, Command*> directoryInputNodeAndCommand) -> bool {
+      return isIncludedUnownedNode(directoryInputNodeAndCommand.first) && directoryInputNodeAndCommand.second->repairViaOwnershipAnalysis;
     });
 
+    // For each output node and its producing command (e.g. "unowned-directory/a.txt" and "TaskA"),
+    // check if there exists an unowned node (e.g. "unowned-directory/" used by "TaskC") that is a parent of the produced node.
+    // Only add "a.txt" to mustScanAfterPaths of "unowned-directory/" if TaskC is marked as "repairViaOwnershipAnalysis".
     for (auto outputNodeAndCommand: outputNodesAndCommands) {
-      auto unownedNode = std::find_if(unownedDirectoryInputNodes.begin(),
-                                      unownedDirectoryInputNodes.end(),
-                                      [=](BuildNode* unownedDirectory) -> bool {
-        return outputNodeAndCommand.first->getName().startswith(unownedDirectory->getName()) && outputNodeAndCommand.second->excludeFromOwnershipAnalysis == false;
+      auto repairableUnownedNode =
+        std::find_if(unownedDirectoryInputNodesAndConsumingCommands.begin(),
+                     unownedDirectoryInputNodesAndConsumingCommands.end(),
+                     [=](std::pair<BuildNode*, Command*> unownedDirectoryAndCommand) -> bool {
+          return outputNodeAndCommand.first->getName().startswith(unownedDirectoryAndCommand.first->getName()) && outputNodeAndCommand.second->repairViaOwnershipAnalysis == true;
       });
 
-      if (unownedNode != unownedDirectoryInputNodes.end()) {
-        auto mustScanAfterPath = outputNodeAndCommand.first->getName();
-        std::string filename = llvm::sys::path::filename(mustScanAfterPath);
-        (*unownedNode)->mustScanAfterPaths.push_back(outputNodeAndCommand.first->getName());
+      if (repairableUnownedNode != unownedDirectoryInputNodesAndConsumingCommands.end()) {
+        (*repairableUnownedNode).first->mustScanAfterPaths.push_back(outputNodeAndCommand.first->getName());
       }
     }
   }

lib/BuildSystem/BuildNode.cpp

@@ -104,9 +104,6 @@ bool BuildNode::configureAttribute(const ConfigureContext& ctx, StringRef name,
   } else if (name == "content-exclusion-patterns") {
     exclusionPatterns = basic::StringList(value);
     return true;
-  } else if (name == "must-scan-after-paths") {
-    mustScanAfterPaths = basic::StringList(value).getValues();
-    return true;
   }
 
   // We don't support any other custom attributes.

lib/BuildSystem/BuildSystem.cpp

@@ -3097,28 +3097,14 @@ class MkdirCommand : public ExternalCommand {
 };
 
 class MkdirTool : public Tool {
-  bool excludeFromOwnershipAnalysis = false;
 
 public:
   using Tool::Tool;
 
   virtual bool configureAttribute(const ConfigureContext& ctx, StringRef name,
                                   StringRef value) override {
-    if (name == "exclude-from-ownership-analysis") {
-      if (value == "true") {
-        excludeFromOwnershipAnalysis = true;
-        return true;
-      } else if (value == "false") {
-        excludeFromOwnershipAnalysis = false;
-        return true;
-      } else {
-        ctx.error("invalid value for attribute: '" + name + "'");
-        return false;
-      }
-    } else {
-      ctx.error("unexpected attribute: '" + name + "'");
-      return false;
-    }
+    ctx.error("unexpected attribute: '" + name + "'");
+    return false;
   }
 
   virtual bool configureAttribute(const ConfigureContext& ctx, StringRef name,
@@ -3137,7 +3123,6 @@ class MkdirTool : public Tool {
 
   virtual std::unique_ptr<Command> createCommand(StringRef name) override {
     auto res = llvm::make_unique<MkdirCommand>(name);
-    res->excludeFromOwnershipAnalysis = excludeFromOwnershipAnalysis;
     return res;
   }
 };
@@ -3229,12 +3214,12 @@ class SymlinkCommand : public Command {
     } else if (name == "link-output-path") {
       linkOutputPath = value;
       return true;
-    } else if (name == "exclude-from-ownership-analysis") {
+    } else if (name == "repair-via-ownership-analysis") {
       if (value == "true") {
-        excludeFromOwnershipAnalysis = true;
+        repairViaOwnershipAnalysis = true;
         return true;
       } else if (value == "false") {
-        excludeFromOwnershipAnalysis = false;
+        repairViaOwnershipAnalysis = false;
         return true;
       } else {
         ctx.error("invalid value for attribute: '" + name + "'");
@@ -3381,18 +3366,18 @@ class SymlinkCommand : public Command {
 };
 
 class SymlinkTool : public Tool {
-  bool excludeFromOwnershipAnalysis = false;
+  bool repairViaOwnershipAnalysis = false;
 public:
   using Tool::Tool;
 
   virtual bool configureAttribute(const ConfigureContext& ctx, StringRef name,
                                   StringRef value) override {
-    if (name == "exclude-from-ownership-analysis") {
+    if (name == "repair-via-ownership-analysis") {
       if (value == "true") {
-        excludeFromOwnershipAnalysis = true;
+        repairViaOwnershipAnalysis = true;
         return true;
       } else if(value == "false") {
-        excludeFromOwnershipAnalysis = false;
+        repairViaOwnershipAnalysis = false;
         return true;
       } else {
         ctx.error("invalid value for attribute: '" + name + "'");
@@ -3420,7 +3405,7 @@ class SymlinkTool : public Tool {
 
   virtual std::unique_ptr<Command> createCommand(StringRef name) override {
     auto res = llvm::make_unique<SymlinkCommand>(name);
-    res->excludeFromOwnershipAnalysis = excludeFromOwnershipAnalysis;
+    res->repairViaOwnershipAnalysis = repairViaOwnershipAnalysis;
     return res;
   }
 };

lib/BuildSystem/ExternalCommand.cpp

@@ -97,12 +97,12 @@ configureAttribute(const ConfigureContext& ctx, StringRef name,
     alwaysOutOfDate = value == "true";
     return true;
 
-  } else if (name == "exclude-from-ownership-analysis") {
+  } else if (name == "repair-via-ownership-analysis") {
     if (value == "true") {
-      excludeFromOwnershipAnalysis = true;
+      repairViaOwnershipAnalysis = true;
       return true;
     } else if (value == "false") {
-      excludeFromOwnershipAnalysis = false;
+      repairViaOwnershipAnalysis = false;
       return true;
     } else {
       ctx.error("invalid value for attribute: '" + name + "'");

products/libllbuild/BuildSystem-C-API.cpp

@@ -848,12 +848,12 @@ class CAPIExternalCommand : public ExternalCommand {
       } else {
         return false;
       }
-    } else if (name == "exclude-from-ownership-analysis") {
+    } else if (name == "repair-via-ownership-analysis") {
       if (value == "true") {
-        excludeFromOwnershipAnalysis = true;
+        repairViaOwnershipAnalysis = true;
         return true;
       } else if (value == "false") {
-        excludeFromOwnershipAnalysis = false;
+        repairViaOwnershipAnalysis = false;
         return true;
       } else {
         return false;

tests/BuildSystem/Build/directory-input-and-output.llbuild

@@ -7,8 +7,8 @@
 #
 # raw/
 #  |   libX.fake-h <-- written on disk (initial state)
-#  |   libX.fake-h <-- written on disk (initial state)
 #  |   libY.fake-h <-- written on disk (initial state)
+#  |   libZ.fake-h <-- written on disk (initial state)
 #  v
 # [TaskA] (verbatim copy; mixed case)
 #  |
@@ -115,6 +115,7 @@ commands:
           cat $outputFile
       done
       '
+    repair-via-ownership-analysis: true
 
   taskA:
     tool: shell
@@ -136,3 +137,4 @@ commands:
       ls .
       cat *
       '
+    repair-via-ownership-analysis: true

tests/BuildSystem/Build/directory-input-must-scan-after-paths-error.llbuild

@@ -32,17 +32,20 @@ commands:
     outputs: ["out/a.txt"]
     description: "TaskA"
     args: "cat raw.txt > out/a.txt; cat out/a.txt; false"
+    repair-via-ownership-analysis: true
 
   taskB:
     tool: shell
     inputs: []
     outputs: ["out/b.txt"]
     description: "TaskB"
     args: "echo 'Output from TaskB for b.txt' > out/b.txt; cat out/b.txt"
+    repair-via-ownership-analysis: true
 
   taskC:
     tool: shell
     inputs: ["out/"]
     outputs: ["final.txt"]
     description: "TaskC"
     args: "cat out/* | tr a-z A-Z > final.txt; cat final.txt"
+    repair-via-ownership-analysis: true

tests/BuildSystem/Build/directory-input-must-scan-after-paths.llbuild

@@ -64,17 +64,20 @@ commands:
     outputs: ["out/a.txt"]
     description: "TaskA"
     args: "cat raw.txt > out/a.txt; cat out/a.txt"
+    repair-via-ownership-analysis: true
 
   taskB:
     tool: shell
     inputs: []
     outputs: ["out/b.txt"]
     description: "TaskB"
     args: "echo 'Output from TaskB for b.txt' > out/b.txt; cat out/b.txt"
+    repair-via-ownership-analysis: true
 
   taskC:
     tool: shell
     inputs: ["out/"]
     outputs: ["final.txt"]
     description: "TaskC"
     args: "cat out/* | tr a-z A-Z > final.txt; cat final.txt"
+    repair-via-ownership-analysis: true