Split command identity into a join key and a signature (fixes #170, fixes #188)

include/pup/core/hash.hpp

@@ -64,6 +64,10 @@ auto hex_to_hash(std::string_view hex) -> Result<Hash256>;
 [[nodiscard]]
 auto hash_equal(Hash256 const& a, Hash256 const& b) -> bool;
 
+/// Byte-lexicographic order, for sorting and binary-searching hash-keyed ranges.
+[[nodiscard]]
+auto hash_less(Hash256 const& a, Hash256 const& b) -> bool;
+
 /// Zero hash constant
 inline constexpr auto ZERO_HASH = Hash256 {};
 

include/pup/graph/dag.hpp

@@ -390,15 +390,23 @@ auto expand_instruction(
 [[nodiscard]]
 auto expand_instruction(Graph const& graph, NodeId cmd_id) -> StringId;
 
-/// Compute a command's structural identity: a content hash that determines whether
-/// the command must re-run. It folds the fully-expanded command text together with
-/// the values (content hashes) of every Variable node the command depends on via a
-/// Sticky edge — capturing config/env vars that affect the output without appearing
-/// in the rendered text (e.g. an exported env var the subprocess reads via $VAR).
-/// Two builds yield the same identity iff the command's effective definition is the
-/// same, so it is the correct cross-build join key (unlike the rendered string).
-[[nodiscard]]
-auto compute_command_identity(Graph const& graph, NodeId cmd_id, PathCache& cache) -> Hash256;
+/// What the command will do, for deciding whether it must re-run: the fully-expanded
+/// command text plus the values (content hashes) of every Variable node it depends on
+/// via a Sticky edge — capturing config/env vars that affect the output without
+/// appearing in the rendered text (e.g. an exported env var read as $VAR).
+///
+/// Not a cross-build key. Editing a recipe changes the signature and must not make the
+/// rule a different rule; that is what compute_command_key answers.
+[[nodiscard]]
+auto compute_command_signature(Graph const& graph, NodeId cmd_id, PathCache& cache) -> Hash256;
+
+/// Which rule this is, for joining a command against the previous build. A command that
+/// produces files is keyed by the files themselves — output ownership is unique among
+/// guard-satisfied commands, enforced when the output edge is created — so this textual
+/// key is the fallback for output-less commands, which have nothing else to be named by.
+/// It deliberately excludes variable values: a var change means re-run, not a new rule.
+[[nodiscard]]
+auto compute_command_key(Graph const& graph, NodeId cmd_id, PathCache& cache) -> Hash256;
 
 /// Set the build root name (relative path from source root to build root)
 /// For in-tree builds, this should be empty. For variant builds, e.g. "build".

include/pup/index/entry.hpp

@@ -54,7 +54,8 @@ struct CommandEntry {
     StringId display = StringId::Empty;             ///< Display text (from ^ ^ markers)
     StringId env = StringId::Empty;                 ///< Environment variables
 
-    Hash256 identity = {}; ///< Structural identity: command text + values of vars it depends on
+    Hash256 key = {};       ///< Which rule this is: the cross-build join key
+    Hash256 signature = {}; ///< What it will do: changes mean re-run, not a different rule
 
     Vec<NodeId> inputs = {};  ///< Input file operands (for %f expansion)
     Vec<NodeId> outputs = {}; ///< Output file operands (for %o expansion)

include/pup/index/format.hpp

@@ -50,7 +50,15 @@ inline constexpr auto INDEX_MAGIC = std::array<char, 4> { 'P', 'U', 'P', 'I' };
 ///       both the membership and the order of %f changed for any pattern a
 ///       generated file matches (issues #177, #178); v16 identities no longer
 ///       join for those commands.
-inline constexpr auto INDEX_VERSION = std::uint32_t { 17 };
+///  18 - A dep-scan command's identity folds its parent command's identity. Two
+///       compiles of one source with equal flags render byte-identical scans, so
+///       v17 gave them one identity and the join between them was arbitrary
+///       (issue #170); every dep-scan command's identity changes.
+///  19 - The single `identity` splits into `key` (which rule this is) and
+///       `signature` (what it will do), because one value cannot both stay stable
+///       across a recipe edit and change when the recipe changes (issue #188).
+///       v18 entries carry neither field.
+inline constexpr auto INDEX_VERSION = std::uint32_t { 19 };
 
 /// Index file header (56 bytes) - v9
 struct alignas(8) RawHeader {
@@ -98,11 +106,11 @@ struct alignas(8) RawCommandEntry {
     std::uint32_t cmd_offset = 0;     ///< Offset to template string with %f/%o patterns (v8)
     std::uint32_t display_offset = 0; ///< Display text offset (length-prefixed)
     std::uint32_t env_offset = 0;     ///< Environment variables offset (length-prefixed)
-    Hash256 identity = {};            ///< Structural identity (v11): SHA-256 over command text
-                                      ///< plus the values of vars it depends on (sticky/exported)
+    Hash256 key = {};                 ///< Which rule this is, for the cross-build join (v19)
+    Hash256 signature = {};           ///< What it will do, for deciding whether to re-run (v19)
 };
 
-static_assert(sizeof(RawCommandEntry) == 48, "RawCommandEntry must be 48 bytes");
+static_assert(sizeof(RawCommandEntry) == 80, "RawCommandEntry must be 80 bytes");
 
 /// Raw edge entry (16 bytes)
 /// Represents dependencies between nodes