"generic" rule: verify staging conflicts on inputs

The "generic" rules deliberately resolves conflicts on identical
paths in a latest-wins fashion (seeing all artifacts as later than
all runfiles) to allow an easy way to define actions. However, the
inputs stage obtained by this resolution can still contain conflicts
and those are an error. Properly detect those. Also clarify in the
documentation, that only conflicts on identical paths are resolved
in the described priority, not semantic overlap.

3 files changed, 16 insertions, 4 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0d3234e3..8850971d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,7 @@ A feature release on top of `1.4.0`, backwards compatible.
 
 ### Fixes
 
+- The "generic" rule now properly detects staging conflicts.
 - Fixes ensuring proper pointer life time and access check.
 - A race condition in the use of `libgit2` was fixed that could
   result in a segmentation fault.
diff --git a/doc/concepts/built-in-rules.md b/doc/concepts/built-in-rules.md
index 6c80ffb4..e7a5e2e3 100644
--- a/doc/concepts/built-in-rules.md
+++ b/doc/concepts/built-in-rules.md
@@ -86,10 +86,12 @@ choice.
 
 The `"deps"` argument is evaluated and has to evaluate to a list of
 target names. The runfiles and artifacts of these targets form the
-inputs of the action. Conflicts are not an error and resolved by giving
-precedence to the artifacts over the runfiles; conflicts within
-artifacts or runfiles are resolved in a latest-wins fashion using the
-order of the targets in the evaluated `"deps"` argument.
+inputs of the action. Conflicting definitions for individual paths
+are not an error and resolved by giving precedence to the artifacts
+over the runfiles; conflicts within artifacts or runfiles are
+resolved in a latest-wins fashion using the order of the targets in
+the evaluated `"deps"` argument. However, the input stage obtained
+by those resolution rules has to be free of semantic conflicts.
 
 The fields `"cmds"`, `"cwd"`, `"sh -c"`, `"out_dirs"`, `"outs"`, and `"env"`
 are evaluated fields where `"cmds"`, `"out_dirs"`, and `"outs"`
diff --git a/src/buildtool/build_engine/target_map/built_in_rules.cpp b/src/buildtool/build_engine/target_map/built_in_rules.cpp
index 220502c4..534d2fea 100644
--- a/src/buildtool/build_engine/target_map/built_in_rules.cpp
+++ b/src/buildtool/build_engine/target_map/built_in_rules.cpp
@@ -1369,9 +1369,18 @@ void GenericRuleWithDeps(
     for (auto const& dep : dependency_values) {
         inputs = ExpressionPtr{Expression::map_t{inputs, (*dep)->RunFiles()}};
     }
+    auto inputs_conflict = BuildMaps::Target::Utils::tree_conflict(inputs);
     for (auto const& dep : dependency_values) {
         inputs = ExpressionPtr{Expression::map_t{inputs, (*dep)->Artifacts()}};
     }
+    // While syntactical conflicts are resolved in a latest wins (with artifacts
+    // after runfiles), semantic path conclicts are an error.
+    if (inputs_conflict) {
+        (*logger)(fmt::format("Input artifacts have staging conflict on {}",
+                              nlohmann::json(*inputs_conflict).dump()),
+                  /*fatal=*/true);
+        return;
+    }
     std::vector<Tree::Ptr> trees{};
     inputs = BuildMaps::Target::Utils::add_dir_for(
         cwd_value->String(), inputs, &trees);