Merge pull request #501 from dmbryson/dcfix-5.1

Cherry-pick 'Handle SkippedCommand values in DirectoryContents'
swiftlang · Jun 27, 2019 · f73b84b · f73b84b
2 parents a75c546 + 1808e05
commit f73b84b
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 1 deletion.
diff --git a/lib/BuildSystem/BuildSystem.cpp b/lib/BuildSystem/BuildSystem.cpp
@@ -880,6 +880,13 @@ class DirectoryContentsTask : public Task {
       return;
     }
 
+    // The input directory may be a 'mkdir' command, which can be cancelled or
+    // skipped by the engine or the delegate. rdar://problem/50380532
+    if (directoryValue.isSkippedCommand()) {
+      engine.taskIsComplete(this, BuildValue::makeSkippedCommand().toData());
+      return;
+    }
+
     std::vector<std::string> filenames;
     getContents(path, filenames);
 
@@ -1294,7 +1301,7 @@ class DirectoryTreeStructureSignatureTask : public Task {
 
       // Request the inputs for each subpath.
       auto value = BuildValue::fromData(valueData);
-      if (value.isMissingInput())
+      if (value.isMissingInput() || value.isSkippedCommand())
         return;
 
       assert(value.isDirectoryContents());

diff --git a/unittests/BuildSystem/BuildSystemTaskTests.cpp b/unittests/BuildSystem/BuildSystemTaskTests.cpp
@@ -1078,3 +1078,91 @@ TEST(BuildSystemTaskTests, producedNodeAfterPreviouslyMissing) {
     ASSERT_TRUE(!afterFileInfo.isMissing());
   }
 }
+
+/// Check that directory contents properly handles when commands have been
+/// skipped. rdar://problem/50380532
+TEST(BuildSystemTaskTests, directoryContentsWithSkippedCommand) {
+  TmpDir tempDir(__func__);
+
+  SmallString<256> manifest{ tempDir.str() };
+  for (auto& c : manifest) {
+    if (c == '\\')
+      c = '/';
+  }
+  sys::path::append(manifest, "manifest.llbuild");
+
+
+  {
+    std::error_code ec;
+    llvm::raw_fd_ostream os(manifest, ec, llvm::sys::fs::F_Text);
+    assert(!ec);
+
+    os <<
+    "client:\n"
+    "  name: mock\n"
+    "\n"
+    "targets:\n"
+    "  \"\": [\"<all>\"]\n"
+    "\n"
+    "commands:\n"
+    "  \"mkdir-inputDir\":\n"
+    "    tool: mkdir\n"
+    "    outputs: [\"inputDir\"]\n"
+    "  \"read-inputDir\":\n"
+    "    tool: shell\n"
+    "    inputs: [\"inputDir/\"]\n"
+    "    outputs: [\"read-inputDir\"]\n"
+    "    description: \"read-inputDir\"\n"
+    "    args:\n"
+    "      touch read-inputDir\n"
+    "  \"<all>\":\n"
+    "    tool: phony\n"
+    "    inputs: [\"read-inputDir\"]\n"
+    "    outputs: [\"<all>\"]";
+  }
+
+  class CancellingDelegate: public MockBuildSystemDelegate {
+  public:
+    BuildSystem* system = nullptr;
+
+    CancellingDelegate(): MockBuildSystemDelegate() { }
+
+    virtual bool shouldCommandStart(Command* command) override {
+      if (command->getName() == "mkdir-inputDir") {
+        return false;
+      }
+      return true;
+    }
+  };
+
+  // Create the build system.
+  CancellingDelegate delegate;
+  BuildSystem system(delegate, createLocalFileSystem());
+  delegate.system = &system;
+  bool loadingResult = system.loadDescription(manifest);
+  ASSERT_TRUE(loadingResult);
+
+  // Build the default target
+  auto result = system.build(BuildKey::makeTarget(""));
+  ASSERT_TRUE(result.hasValue());
+
+  // The contents should propagate the skipped command
+  result = system.build(BuildKey::makeDirectoryContents("inputDir"));
+  ASSERT_TRUE(result.hasValue());
+  ASSERT_TRUE(result.getValue().isSkippedCommand());
+
+  // Signatures don't need the contents and should be fine with encoding the
+  // skipped value in the signature.
+  result = system.build(BuildKey::makeDirectoryTreeSignature("inputDir", {}));
+  ASSERT_TRUE(result.hasValue());
+  ASSERT_FALSE(result.getValue().isSkippedCommand());
+
+  auto filters = StringList({"filter"});
+  result = system.build(BuildKey::makeDirectoryTreeSignature("inputDir", filters));
+  ASSERT_TRUE(result.hasValue());
+  ASSERT_FALSE(result.getValue().isSkippedCommand());
+
+  result = system.build(BuildKey::makeDirectoryTreeStructureSignature("inputDir"));
+  ASSERT_TRUE(result.hasValue());
+  ASSERT_FALSE(result.getValue().isSkippedCommand());
+}