From 0020239c3d05d603ca6a0707d9679d0bf1e81a00 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 16 Sep 2026 08:32:03 +0800 Subject: [PATCH] Two spellings of one version constraint are one source (#647 E4.2 follow-up) Self-review of #650 before releasing 2026.9.16.1 found that the gate E4.2 added refuses a manifest the previous release builds. `[feature-deps.]` must restate its dependency's source, and a restatement naming another source is refused rather than silently dropped. The comparison was made on the bytes of the declaration. A path is normalised first, so a path dependency is judged on what it means; a version constraint was not, so two spellings of one constraint were read as two sources. Measured, with the fixture in the leg below: [dependencies] fmtlib.fmt = ">=11.0.0" [feature-deps.extra] fmtlib.fmt = { version = ">= 11.0.0", reexport = true } 2026.9.15.2 (released): builds, no error and no warning 2026.9.16.1 (f4529b2a): error: ... restates the dependency 'fmtlib.fmt' with version = ">= 11.0.0", while the declaration in effect on this row names version = ">=11.0.0". One space turns a build that worked yesterday into a refusal. The judgement is now made on what the declarations mean: the constraint is compared with its whitespace removed, while the message still shows each declaration as it was written, so a genuine disagreement reads as the manifest spells it. A gate refuses a restatement that names another source, and nothing else. The same review asked why the first custom-index sync does not go through `decide_for_miss`/`apply` when the record says the refresh sites do. It should not: that pair's debounce and one-sync-per-process guard are about the index that resolves a dependency, while this sync creates a local copy of a different set of repositories that nothing else will create, so taking the guard would let an earlier refresh of the builtin index suppress a clone the build cannot proceed without. Only the opt-outs are shared. The comment said "the policy's answer is taken" and now states which half is taken, and why the other is not. Criterion: e2e 711 gains a leg. A restatement differing from the declaration only in whitespace is not refused, and one naming `>=9.9.8` against `>=9.9.9` still is. On f4529b2a the first half fails with the refusal quoted above. Readings: unit 121 passed, 0 failed; e2e 187, 710 to 714 pass. --- src/build/prepare.cppm | 36 +++++++++++-- ...ature_deps_restatement_names_one_source.sh | 54 ++++++++++++++++++- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 1311da41..1678eabe 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -4711,9 +4711,20 @@ prepare_build(bool print_fingerprint, } // A first sync is a refresh of an index that has no local // copy yet, and `[index] auto_refresh = false` means that no - // refresh happens implicitly (docs/05). The policy's answer is - // taken rather than restated (#648 A5); offline, the sync is a - // no-op as before and resolution reports what is missing. + // refresh happens implicitly (docs/05). The opt-outs are the + // policy's (#648 A5); offline, the sync is a no-op as before + // and resolution reports what is missing. + // + // WHY THIS ONE DOES NOT GO THROUGH `decide_for_miss`/`apply`. + // Those answer "may this run refresh the index that would + // resolve a dependency", and their debounce and one-sync-per- + // process guard are about that one index. This sync creates a + // local copy that does not exist yet, of a DIFFERENT set of + // repositories, and nothing else will create it: taking the + // guard would let a refresh of the builtin index earlier in + // the same run suppress a clone the build cannot proceed + // without. Only the opt-outs are shared, and they are read + // from the same `policy_for`. const auto refreshPolicy = mcpp::pm::policy_for(**cfg2); if (needsRemoteUpdate && !refreshPolicy.offline && !refreshPolicy.autoRefresh) { return std::unexpected(std::string( @@ -6782,6 +6793,14 @@ prepare_build(bool print_fingerprint, // while the manifest said it came from somewhere else. The comparison runs // against `dependencies` after the conditional fold, so a row's replacement // (#634 A1) is the declaration a restatement is held to. + // + // TWO SPELLINGS OF ONE SOURCE ARE ONE SOURCE. The comparison below decides + // whether a restatement names something else, so it has to be made on what + // the two declarations MEAN, not on their bytes: a path is normalised, and + // a version constraint is compared with its whitespace removed, because + // `">= 1.2.0"` and `">=1.2.0"` are one constraint and the manifest that + // spells them differently built on 2026.9.15.2. A gate added for #647 E4.2 + // must refuse a restatement that names another source, and nothing else. auto dependencySourceOf = [](const mcpp::manifest::DependencySpec& s) { if (s.inheritWorkspace) return std::string("workspace = true"); if (s.isPath()) { @@ -6794,6 +6813,15 @@ prepare_build(bool print_fingerprint, s.gitRefKind.empty() ? "rev" : s.gitRefKind, s.gitRev); return std::format("version = \"{}\"", s.version); }; + // What the comparison is made on. The message shows the declaration as it + // was written; the judgement drops the whitespace inside a constraint, so + // the two declarations are compared on what they mean. + auto dependencySourceKey = [&](const mcpp::manifest::DependencySpec& s) { + auto spelled = dependencySourceOf(s); + if (!s.inheritWorkspace && !s.isPath() && !s.isGit()) + std::erase_if(spelled, [](char c) { return c == ' ' || c == '\t'; }); + return spelled; + }; auto mergeActiveFeatureDeps = [&](mcpp::manifest::Manifest& pm, const std::vector& requested, bool seedDefault = true) @@ -6808,7 +6836,7 @@ prepare_build(bool print_fingerprint, if (!pos->second.inheritWorkspace && !spec.inheritWorkspace) { const auto inEffect = dependencySourceOf(pos->second); const auto restated = dependencySourceOf(spec); - if (inEffect != restated) + if (dependencySourceKey(pos->second) != dependencySourceKey(spec)) return std::unexpected(std::format( "[feature-deps.{}] of '{}' restates the dependency '{}' " "with {}, while the declaration in effect on this row " diff --git a/tests/e2e/711_a_feature_deps_restatement_names_one_source.sh b/tests/e2e/711_a_feature_deps_restatement_names_one_source.sh index bdb924d8..fb13b4b8 100755 --- a/tests/e2e/711_a_feature_deps_restatement_names_one_source.sh +++ b/tests/e2e/711_a_feature_deps_restatement_names_one_source.sh @@ -16,7 +16,9 @@ # Readings: the source-less entry is refused and names the remedy; the # restatement with the same source builds the tool only under the feature, and # the build program sees both spellings; a restatement naming another path is -# refused under `--strict` naming both sources. +# refused under `--strict` naming both sources; and two spellings of one version +# constraint, differing only in whitespace, are one source, while a genuinely +# different constraint is still refused. set -e TMP=$(mktemp -d) @@ -109,4 +111,54 @@ fi grep -q '../other' b4.log && grep -q '../installer' b4.log \ || fail "the refusal does not name both sources" b4.log +# ── 4. two spellings of one constraint are one source ──────────────────── +# The judgement is on what the declarations mean. `">= 9.9.9"` and `">=9.9.9"` +# are one constraint, and a manifest spelling them differently built on +# 2026.9.15.2, so refusing it would be an upgrade cliff. The package is never +# resolved here: the merge runs before resolution, so the reading is whether the +# refusal appears at all, and the leg below with a genuinely different +# constraint is what shows the gate still closes. +mkdir -p "$TMP/spell/src" +echo 'int main() { return 0; }' > "$TMP/spell/src/main.cpp" +write_spell() { # write_spell +cat > "$TMP/spell/mcpp.toml" <= 9.9.9" +MCPP_OFFLINE=1 "$MCPP" build --features extra > s1.log 2>&1 || true +grep -q 'restates the dependency' s1.log \ + && fail "a restatement differing from the declaration only in whitespace was refused" s1.log + +write_spell ">=9.9.8" +rm -rf target +if MCPP_OFFLINE=1 "$MCPP" build --features extra > s2.log 2>&1; then + fail "a restatement naming another constraint was accepted" s2.log +fi +grep -q 'restates the dependency' s2.log \ + || fail "a restatement naming another constraint was not refused" s2.log + echo "PASS: 711 a feature-deps restatement names one source; dep_bin answers both spellings"