Fold the rule's directory into command identity (fixes #167)

include/pup/index/format.hpp

@@ -36,7 +36,12 @@ inline constexpr auto INDEX_MAGIC = std::array<char, 4> { 'P', 'U', 'P', 'I' };
 ///  14 - Guard-unsatisfied commands are no longer serialized; v13 indexes carry
 ///       their identities and would mask reactivated output-less commands as
 ///       already known, so they never run (issue #118)
-inline constexpr auto INDEX_VERSION = std::uint32_t { 14 };
+///  15 - Command identity folds the rule's source directory; v14 identities are
+///       directory-blind, so identical rules in sibling directories collide and
+///       resolve to one another through the identity join (issue #167). Also
+///       covers OrderOnly edges, persisted since issue #166 and load-bearing for
+///       removal routing; v14 indexes predate them and would lose one removal.
+inline constexpr auto INDEX_VERSION = std::uint32_t { 15 };
 
 /// Index file header (56 bytes) - v9
 struct alignas(8) RawHeader {

src/cli/cmd_build.cpp

@@ -979,7 +979,8 @@ auto is_dir_authoritative(
 /// Carry forward old-index commands from directories this run has no
 /// authoritative knowledge of, so a scoped build's saved index still describes
 /// the whole project (issue #125). Copies each command with its operand files
-/// (stat data preserved) and Normal/Sticky edges, remapping ids to the new
+/// (stat data preserved) and every edge except Implicit — OrderOnly among them,
+/// which carries removal routing for order-only inputs — remapping ids to the new
 /// index's dense sequences; implicit edges are re-attached afterwards by
 /// preserve_old_implicit_edges through the identity match.
 auto merge_out_of_scope_commands(
@@ -1469,11 +1470,11 @@ auto reconcile_input_set(
         if (!file) {
             continue;
         }
-        // The complementary half of the input edge types; expand_implicit_deps
-        // routes Implicit and Sticky through the same index for any changed file.
+        // Edge types that reach a command in one hop; expand_implicit_deps routes
+        // Implicit and Sticky. Group is neither: it runs file -> group, and the
+        // group -> command hop is a separate OrderOnly edge, so it needs two (#169).
         for (auto const* edge : idx.edges_from(pup::NodeId { file->id })) {
-            if (edge->type != pup::LinkType::Normal && edge->type != pup::LinkType::OrderOnly
-                && edge->type != pup::LinkType::Group) {
+            if (edge->type != pup::LinkType::Normal && edge->type != pup::LinkType::OrderOnly) {
                 continue;
             }
             auto const* cmd = idx.find_command_by_id(pup::NodeId { edge->to });

src/graph/dag.cpp

@@ -1035,10 +1035,16 @@ auto compute_command_identity(Graph const& graph, NodeId cmd_id, PathCache& cach
 {
     auto& pool = global_pool();
     auto state = sha256_init();
+    auto constexpr SEP = std::byte { 0 };
 
     // Base: the fully-expanded command text (instruction + operand paths + in-text vars).
     state = sha256_update(state, pool.get(expand_instruction(graph, cmd_id, cache)));
 
+    // Command text is Tupfile-relative, so the same rule in sibling directories renders
+    // identically; without the directory those distinct rules share one identity.
+    state = sha256_update(state, std::span<std::byte const> { &SEP, 1 });
+    state = sha256_update(state, pool.get(get<SourceDir>(graph, cmd_id)));
+
     // Fold in (name, value-hash) of each Variable node reached via a Sticky edge.
     // This captures vars that affect output without appearing in the rendered text —
     // exported env vars the subprocess reads as $VAR, config vars gating an export, etc.
@@ -1057,7 +1063,6 @@ auto compute_command_identity(Graph const& graph, NodeId cmd_id, PathCache& cach
         vars.end()
     );
 
-    auto constexpr SEP = std::byte { 0 };
     for (auto const& [name, value_hash] : vars) {
         state = sha256_update(state, std::span<std::byte const> { &SEP, 1 });
         state = sha256_update(state, name);

test/e2e/fixtures/identity_collision/a/Tupfile.fixture

@@ -0,0 +1,2 @@
+: main.c |> gcc -c %f -o %o |> main.o
+: main.o |> gcc %f -o %o |> program

test/e2e/fixtures/identity_collision/a/config.h

@@ -0,0 +1,4 @@
+#ifndef CONFIG_H
+#define CONFIG_H
+#define VERSION 1
+#endif

test/e2e/fixtures/identity_collision/a/main.c

@@ -0,0 +1,3 @@
+#include <stdio.h>
+#include "config.h"
+int main(void) { printf("Version %d\n", VERSION); return 0; }

test/e2e/fixtures/identity_collision/b/Tupfile.fixture

@@ -0,0 +1,2 @@
+: main.c |> gcc -c %f -o %o |> main.o
+: main.o |> gcc %f -o %o |> program

test/e2e/fixtures/identity_collision/b/config.h

@@ -0,0 +1,4 @@
+#ifndef CONFIG_H
+#define CONFIG_H
+#define VERSION 1
+#endif

test/e2e/fixtures/identity_collision/b/main.c

@@ -0,0 +1,3 @@
+#include <stdio.h>
+#include "config.h"
+int main(void) { printf("Version %d\n", VERSION); return 0; }

test/unit/test_e2e.cpp

@@ -1107,6 +1107,44 @@ SCENARIO("Implicit dependencies track header changes", "[e2e][incremental]")
     }
 }
 
+SCENARIO("Implicit deps survive identical rules in sibling directories", "[e2e][incremental][identity]")
+{
+    // Command text is Tupfile-relative, so these two rules render the same string.
+    // If identity ignores the directory they collide, and one directory's header
+    // edges get attached to the other's command.
+    auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" };
+
+    GIVEN("two directories whose rules and sources are byte-identical")
+    {
+        auto f = E2EFixture { "identity_collision" };
+        REQUIRE(f.init().success());
+        REQUIRE(f.build().success());
+        REQUIRE(f.run("a/program").stdout_output == "Version 1\n");
+        REQUIRE(f.run("b/program").stdout_output == "Version 1\n");
+
+        WHEN("only the second directory's header changes")
+        {
+            f.write_file("b/config.h", "#ifndef CONFIG_H\n"
+                                       "#define CONFIG_H\n"
+                                       "#define VERSION 2\n"
+                                       "#endif\n");
+            auto result = f.build();
+
+            THEN("that directory rebuilds")
+            {
+                REQUIRE(result.success());
+                REQUIRE(f.run("b/program").stdout_output == "Version 2\n");
+            }
+
+            THEN("the other directory is untouched")
+            {
+                REQUIRE(result.success());
+                REQUIRE(f.run("a/program").stdout_output == "Version 1\n");
+            }
+        }
+    }
+}
+
 SCENARIO("Implicit deps cover every source of a multi-source command", "[e2e][incremental]")
 {
     // gcc -M emits one rule per source; stopping at the first leaves b.h untracked and the program silently stale

test/unit/test_graph.cpp

@@ -1636,3 +1636,45 @@ TEST_CASE("node_id encoding: 30-bit index roundtrips and kinds are mutually excl
 
     REQUIRE_FALSE(pup::node_id::is_file(pup::INVALID_NODE_ID));
 }
+
+TEST_CASE("compute_command_identity separates rules by directory", "[graph][identity]")
+{
+    auto bs = make_build_graph();
+    auto& g = bs.graph;
+
+    // Command text is Tupfile-relative, so identical rules in sibling directories
+    // render the same string; identity must still tell them apart.
+    auto in_a = add_command_node(g, CommandNode {
+        .source_dir = intern("a"),
+        .instruction_id = intern("cat src.txt > out.txt"),
+    });
+    auto in_b = add_command_node(g, CommandNode {
+        .source_dir = intern("b"),
+        .instruction_id = intern("cat src.txt > out.txt"),
+    });
+    REQUIRE(in_a.has_value());
+    REQUIRE(in_b.has_value());
+
+    SECTION("same text in different directories yields different identities")
+    {
+        CHECK(compute_command_identity(g, *in_a, bs.path_cache)
+            != compute_command_identity(g, *in_b, bs.path_cache));
+    }
+
+    SECTION("identity is stable for the same command")
+    {
+        CHECK(compute_command_identity(g, *in_a, bs.path_cache)
+            == compute_command_identity(g, *in_a, bs.path_cache));
+    }
+
+    SECTION("different text in the same directory yields different identities")
+    {
+        auto other = add_command_node(g, CommandNode {
+            .source_dir = intern("a"),
+            .instruction_id = intern("cat other.txt > out.txt"),
+        });
+        REQUIRE(other.has_value());
+        CHECK(compute_command_identity(g, *in_a, bs.path_cache)
+            != compute_command_identity(g, *other, bs.path_cache));
+    }
+}