Merge pull request swiftlang#51 from ddunbar/mkdir-must-follow

[BuildSystem] Allow inputs for Mkdir/Symlink.

Author: ddunbar

docs/buildsystem.rst

@@ -307,8 +307,9 @@ tracking. This tool should be used when clients only care about the existence of
 the directory, not any other aspects of it. In particular, it ignores changes to
 the directory timestamp when consider whether to run.
 
-No attributes are supported other than the common keys. No inputs may be
-declared, and the sole output should be the node for the path to create.
+No attributes are supported other than the common keys. The sole output should
+be the node for the path to create. Arbitrary inputs can be declared, but they
+will only be used to establish the order in which the command is run.
 
 Symlink Tool
 ------------
@@ -325,6 +326,10 @@ it will retrieve the status information of the target, not the link itself. This
 may lead to unnecessary recreation of the link (and triggering of subsequent
 work).
 
+The sole output should be the node for the path to create. Arbitrary inputs can
+be declared, but they will only be used to establish the order in which the
+command is run.
+
 .. note::
 
    The issue here may be encountered by any other tool which needs to create

lib/BuildSystem/BuildSystem.cpp

@@ -1403,8 +1403,17 @@ class MkdirCommand : public Command {
   // FIXME: This seems wasteful.
   std::string description;
 
+  /// Declared command inputs, used only for ordering purposes.
+  std::vector<BuildNode*> inputs;
+  
   virtual uint64_t getSignature() {
-    return basic::hashString(output->getName());
+    // FIXME: Use a more appropriate hashing infrastructure.
+    using llvm::hash_combine;
+    llvm::hash_code code = hash_value(output->getName());
+    for (const auto* input: inputs) {
+      code = hash_combine(code, input->getName());
+    }
+    return size_t(code);
   }
 
   virtual void configureDescription(const ConfigureContext&,
@@ -1429,7 +1438,10 @@ class MkdirCommand : public Command {
 
   virtual void configureInputs(const ConfigureContext& ctx,
                                const std::vector<Node*>& value) override {
-    ctx.error("unexpected explicit input: '" + value[0]->getName() + "'");
+    inputs.reserve(value.size());
+    for (auto* node: value) {
+      inputs.emplace_back(static_cast<BuildNode*>(node));
+    }
   }
 
   virtual void configureOutputs(const ConfigureContext& ctx,
@@ -1511,6 +1523,15 @@ class MkdirCommand : public Command {
 
     // Eventually we would like to use the system itself to manage recursive
     // directory creation.
+
+    // The command itself takes no inputs, so just treat any declared inputs as
+    // "must follow" directives.
+    //
+    // FIXME: We should make this explicit once we have actual support for must
+    // follow inputs.
+    for (auto it = inputs.begin(), ie = inputs.end(); it != ie; ++it) {
+      bsci.taskMustFollow(task, BuildKey::makeNode(*it));
+    }
   }
 
   virtual void providePriorValue(BuildSystemCommandInterface&, core::Task*,
@@ -1613,11 +1634,20 @@ class SymlinkCommand : public Command {
   /// The command description.
   std::string description;
 
+  /// Declared command inputs, used only for ordering purposes.
+  std::vector<BuildNode*> inputs;
+
   /// The contents to write at the output path.
   std::string contents;
 
   virtual uint64_t getSignature() {
-    return basic::hashString(output->getName()) ^ basic::hashString(contents);
+    using llvm::hash_combine;
+    llvm::hash_code code = hash_value(output->getName());
+    code = hash_combine(code, contents);
+    for (const auto* input: inputs) {
+      code = hash_combine(code, input->getName());
+    }
+    return size_t(code);
   }
 
   virtual void configureDescription(const ConfigureContext&,
@@ -1642,7 +1672,10 @@ class SymlinkCommand : public Command {
 
   virtual void configureInputs(const ConfigureContext& ctx,
                                const std::vector<Node*>& value) override {
-    ctx.error("unexpected explicit input: '" + value[0]->getName() + "'");
+    inputs.reserve(value.size());
+    for (auto* node: value) {
+      inputs.emplace_back(static_cast<BuildNode*>(node));
+    }
   }
 
   virtual void configureOutputs(const ConfigureContext& ctx,
@@ -1715,6 +1748,15 @@ class SymlinkCommand : public Command {
                      core::Task* task) override {
     // Notify the client the command is preparing to run.
     bsci.getDelegate().commandPreparing(this);
+
+    // The command itself takes no inputs, so just treat any declared inputs as
+    // "must follow" directives.
+    //
+    // FIXME: We should make this explicit once we have actual support for must
+    // follow inputs.
+    for (auto it = inputs.begin(), ie = inputs.end(); it != ie; ++it) {
+      bsci.taskMustFollow(task, BuildKey::makeNode(*it));
+    }
   }
 
   virtual void providePriorValue(BuildSystemCommandInterface&, core::Task*,

tests/BuildSystem/Build/mkdir.llbuild

@@ -6,6 +6,7 @@
 # RUN: %{llbuild} buildsystem build --serial --chdir %t.build > %t.out
 # RUN: %{FileCheck} --input-file=%t.out %s
 #
+# CHECK: ECHO
 # CHECK: MKDIR
 # CHECK: MUTATE
 
@@ -43,9 +44,15 @@ commands:
     tool: phony
     inputs: ["subdir/subdir2/thing.txt"]
     outputs: ["<all>"]
+  C.echo:
+    tool: shell
+    description: ECHO
+    outputs: ["<echo>"]
+    args: echo hi
   C.mkdir:
     tool: mkdir
     description: MKDIR
+    inputs: ["<echo>"]
     outputs: ["subdir/subdir2"]
   C.mutate:
     tool: shell