commit 70e13d667703d79d7c61f6d01569038e9e0e88d4
parent 3d7104b41793be9c1eb15b50016bc42f3f4bd356
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Fri, 19 Jun 2026 10:36:35 -0700
Require config presence to match build traces
Diffstat:
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/doc/BUILD_COORDINATOR.md b/doc/BUILD_COORDINATOR.md
@@ -317,11 +317,12 @@ src/*.c <glob-result-hash>
config map and argv vector — canonical CAS blobs, kept by value and bundled with
shared traces — so the target's invocation and every `need` it issues replay
*exactly*, not merely check (see [Configuration model](#configuration-model)).
-- `[config]` lists the **names** of consumed propagated keys (consumed whether set
- *or* unset); the values live in the `config-id` map this trace references. A key
- *absent* from that map was consumed while unset, so later *adding* it busts the
- trace — no sentinel value needed. Matching compares, for each consumed key, the
- request's value against the built map's value (absent == unset).
+- `[config]` lists consumed propagated-key observations (consumed whether set
+ *or* unset), including the presence bit and any recipe-supplied default hash.
+ Presence is part of the match: a key consumed while unset does not match a
+ later explicit value, even if that value equals the default the recipe returned.
+ When both sides are present, matching compares the value hash; when both are
+ unset, the observation matches.
- `<blob-id>` = the source file's CAS blob id (`kit_blob_info`); source and glob
leaves are kept by hash because they are *verified*, never replayed. An absent
source is recorded as `<path> -` (a lone `-`), so *creating* the file later
@@ -687,17 +688,19 @@ command.
| `config-get <key>` | the propagated value (or *unset*) | config dep: key recorded as consumed; its value is the one in the effective `config-id` map (absent = unset) |
| `source <path>` | `blob-id` (or *absent*) + a path to read | source dep: `(path, blob-id)`, or `(path, absent)` |
| `glob <pattern>` | sorted list of matching paths (streamed) | glob dep: `(pattern, glob-result-hash)` |
-| `fetch <blob-id> <url>...` | the verified blob-id + a local CAS path | fetch dep: `blob-id` |
+| `fetch <blob-id> <url>...` | the verified blob-id + a local CAS path | blob dep: `blob-id` |
| `need <target> [k=v…] [env…] [argv…]` | the dep's output `tree-id` + a path (blocks) | target dep edge: `(dep, dep-config-id, dep-argv-id, output-tree-id)` |
| `need-submit <target> [k=v…] [env…] [argv…]` | a **token** (does not block) | *(nothing yet — logged on await)* |
| `need-await <token>` | the submitted dep's output `tree-id` + a path | target dep edge: `(dep, dep-config-id, dep-argv-id, output-tree-id)` |
-- **`config-get`** reads the target's *effective propagated* config and records the
- value's hash, so changing a consumed key invalidates while changing an unconsumed
- key does not. If a changed consumed key makes the recipe branch and request a
- *new* key next time, the old trace already fails to match on the changed key, so
- the new key-set is discovered on the rebuild — self-correcting, never needing to
- predict the input set ahead of time. Local config (argv) is *not* read here; it
+- **`config-get`** reads the target's *effective propagated* config and records
+ whether the key was present, plus the value/default hashes needed to replay that
+ observation. Changing a consumed key's presence or value invalidates while
+ changing an unconsumed key does not. If a changed consumed key makes the recipe
+ branch and request a *new* key next time, the old trace already fails to match
+ on the changed key, so the new key-set is discovered on the rebuild —
+ self-correcting, never needing to predict the input set ahead of time. Local
+ config (argv) is *not* read here; it
arrives as the process's argv. **Environment** is config too: keys under the
`env.` prefix (e.g. `env.PATH`) become the recipe's environment variables, so a
recipe's env is tracked, hermetic, and propagates like any config — the spawned
diff --git a/src/build/resolve.c b/src/build/resolve.c
@@ -52,13 +52,10 @@ static int config_observation_match(const BuildConfig* cfg,
if (build_config_get(cfg, kit_slice_cstr(obs->key), &value, &present) !=
BUILD_OK)
return 0;
- if (present) {
- hash_slice(value, value_hash);
- if (obs->present) return build_id_eq(value_hash, obs->value_hash);
- return obs->has_default && build_id_eq(value_hash, obs->default_hash);
- }
- if (obs->present) return 0;
- return 1;
+ if (present != obs->present) return 0;
+ if (!present) return 1;
+ hash_slice(value, value_hash);
+ return build_id_eq(value_hash, obs->value_hash);
}
static int shallow_direct_match(KitBuildCoordinator* c,
diff --git a/src/build/trace.h b/src/build/trace.h
@@ -64,10 +64,12 @@ typedef struct BuildDepEdge {
} BuildDepEdge;
/* One observed propagated config lookup. `present` records whether the key was
- * set in the underlying propagated config. `has_default` records whether the
- * recipe supplied a default. `value_hash` is the returned value hash: actual
- * value when present, default value when absent with a default, otherwise
- * unused. `default_hash` is meaningful only when has_default. Sorted by key. */
+ * set in the underlying propagated config, and is part of the cache match: an
+ * explicit value is not equivalent to an unset key returning the same default.
+ * `has_default` records whether the recipe supplied a default. `value_hash` is
+ * the returned value hash: actual value when present, default value when absent
+ * with a default, otherwise unused. `default_hash` is meaningful only when
+ * has_default. Sorted by key. */
typedef struct BuildConfigLeaf {
char key[BUILD_KEY_MAX];
uint8_t value_hash[BUILD_HASH_LEN];
diff --git a/test/buildcoord/run.sh b/test/buildcoord/run.sh
@@ -1007,10 +1007,8 @@ contains "buildcoord-config-default-absent-hit-no-run" \
"$work/buildcoord-config-default-absent-hit.err" "recipes_run=0"
build_assert_ok buildcoord-config-default-present-fallback --stats \
--config missing-default=fallback //cfg:default
-contains "buildcoord-config-default-present-fallback-hit" \
- "$work/buildcoord-config-default-present-fallback.err" "deep_hits=1"
-contains "buildcoord-config-default-present-fallback-no-run" \
- "$work/buildcoord-config-default-present-fallback.err" "recipes_run=0"
+contains "buildcoord-config-default-present-fallback-run" \
+ "$work/buildcoord-config-default-present-fallback.err" "recipes_run=1"
cfg_default_fallback_path=$(tree_path_from \
"$work/buildcoord-config-default-present-fallback.out")
contains "buildcoord-config-default-present-fallback-output" \
