Skip to content

Commit c5cdcec

Browse files
committed
fix(manifest): 不认识的依赖键降级而不是让整份 manifest 加载失败
`reexport` 要写进**已发布包**的 manifest(grpc 的 [feature-deps.codegen]), 于是冒出一个此前没人问过的问题:比它旧的 mcpp 读到会怎样? 答案曾经是整份加载失败,而且报错误导 —— `reexport = true` 被告知 「must be a string, inline dep table, or nested table」。 根因是**一个谓词兼任两职**:`looks_like_inline_dep_spec` 既判定「内联 spec 还是 嵌套命名空间表」,又枚举「哪些键有意义」。不认识的键因此走不到「未知选项」那条 路上 —— 表直接判不出是 spec,被当成命名空间。 后果不是提示不友好,而是**任何已发布包都永远无法采用新键**。这与 #349 确立的 是同一条性质:数据不得决定程序是否可用。 判据换轴:内联 spec 的判定是「它是否指名了一个来源」(path/version/git/workspace)。 嵌套命名空间表的键是**包名**,不会有包叫 `version`,所以不会误判。识别为 spec 之后,不认识的键记为降级(--strict 仍拒绝),与 xpkg 读取器的 xpkgUnknownKeys 「record rather than swallow」同一条纪律。 救不了已经发布出去的旧客户端,但从这一版起这类问题不再复发。
1 parent 6345e3c commit c5cdcec

5 files changed

Lines changed: 160 additions & 13 deletions

File tree

‎.agents/docs/2026-08-06-provisions-and-build-inputs.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,30 @@ feature 存在的全部理由。
177177

178178
这与索引解析裸包名的阶梯(`(mcpplibs,X) → (compat,X) → (∅,X)`)是同一条规则的同一次应用;第 3 级是必需的补充,否则 `grpc.grpc-plugin` 这类非默认命名空间的包连裸名都拿不到(grpc-m 现在写的正是 `dep_bin("grpc-plugin", ...)`)。`dep_dir()` 沿用同一套。
179179

180+
### D1.4 发布一个新键不能让旧客户端加载失败
181+
182+
`reexport` 要写进**已发布包**的 manifest(grpc 的 `[feature-deps.codegen]`),
183+
于是冒出一个此前没人问过的问题:比它旧的 mcpp 读到这份 manifest 会怎样?
184+
185+
答案曾经是**整份加载失败**,而且报错是误导性的:
186+
`reexport = true` 被告知「must be a string, inline dep table, or nested table」。
187+
188+
根因是**一个谓词兼任两职**:`looks_like_inline_dep_spec` 既判定「这是内联依赖
189+
spec 还是嵌套命名空间表」,又枚举「哪些键有意义」。于是「不认识的键」不会走到
190+
「未知选项」那条路上——表直接判不出是 spec,被当成命名空间。
191+
192+
后果不是提示不友好,而是**任何已发布包都永远无法采用新键**。这与 #349 确立的
193+
性质是同一条:**数据不得决定程序是否可用**。
194+
195+
判据换轴:**内联 spec 的判定是「它是否指名了一个来源」**(`path` / `version` /
196+
`git` / `workspace`)。嵌套命名空间表的键是**包名**,不会有包叫 `version`,所以
197+
这个判据不会误判。识别为 spec 之后,不认识的键**记为降级**(`--strict` 仍拒
198+
绝),而不是让整份 manifest 加载失败——与 xpkg 读取器的 `xpkgUnknownKeys`
199+
(「record rather than swallow」)同一条纪律。
200+
201+
这不能救**已经发布出去的**旧客户端(它们的解析器就是那样),但从这一版起,
202+
这类问题不再复发。
203+
180204
### D2. 输入种类进表,指纹由种类决定
181205

182206
新增一种输入:**glob**。

‎docs/05-mcpp-toml.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,19 @@ int main() { return grpcgen::generate_all() ? 0 : 1; }
11221122
unnamespaced `<x>`, else the single remaining candidate — and when it is
11231123
contested mcpp says so instead of picking silently.
11241124
1125+
##### Older mcpp reading a manifest that uses this
1126+
1127+
An unrecognized dependency key is reported as a **degradation** and ignored
1128+
(mcpp 2026.8.6.2+), so a package written for a newer mcpp still loads and the
1129+
parts this reader understands still apply. Before that release it was a hard
1130+
load failure with a misleading message, which is why a published package could
1131+
not adopt a new key at all — the same property the index floor establishes:
1132+
data must not decide whether the program works.
1133+
1134+
Consequently a package that *relies* on `reexport` for its ergonomics still
1135+
needs a client new enough to implement it; what changed is that everything else
1136+
about that package keeps working on an older one.
1137+
11251138
##### Scoping a provision per platform
11261139
11271140
A package may declare a `bin` target on some platforms only. Because the

‎docs/zh/05-mcpp-toml.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,16 @@ int main() { return grpcgen::generate_all() ? 0 : 1; }
858858
依次绑定到 `mcpplibs.<x>`、`compat.<x>`、无命名空间的 `<x>`,最后才是「剩下
859859
的唯一候选」——存在争用时 mcpp 会说出来,而不是默默选一个。
860860
861+
##### 旧版 mcpp 读到用了这些键的 manifest
862+
863+
不认识的依赖键会被**记为降级**并忽略(mcpp 2026.8.6.2+),因此一份为更新的
864+
mcpp 写的包仍然能加载,这个读取器认识的部分照常生效。在那之前它是**整份加载
865+
失败**且报错误导,这正是「已发布的包永远无法采用新键」的原因——与索引下限确立
866+
的是同一条性质:**数据不得决定程序是否可用**。
867+
868+
因此,一个**依赖** `reexport` 才有那套人机工程的包,仍然需要足够新的客户端;
869+
变化在于该包的其余部分在旧客户端上不再一起失效。
870+
861871
##### 按平台裁剪提供物
862872
863873
一个包可能只在部分平台声明 `bin` 目标。既然现在是**库**决定请求什么,无条件的

‎src/manifest/toml.cppm‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -537,11 +537,7 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
537537
// namespaces (so existing fetcher / lockfile lookups by composite name
538538
// keep working) and the bare `<name>` for the default namespace (so the
539539
// common case stays unchanged).
540-
// MUST list every key `fill_inline_spec` below reads. The two are one
541-
// decision in two places: this predicate also distinguishes an inline dep
542-
// spec from a NESTED namespace table, so a key missing here does not read
543-
// as "unknown option" — the table is taken for a namespace and the user is
544-
// told their value "must be a string, inline dep table, or nested table".
540+
// MUST list every key `fill_inline_spec` below reads.
545541
// `Manifest.EveryDependencySpecKeyIsAccepted` holds the two in sync.
546542
auto is_dep_spec_key = [](std::string_view k) {
547543
return k == "path" || k == "version" || k == "git"
@@ -551,19 +547,49 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
551547
|| k == "backend" || k == "tools"
552548
|| k == "host-module" || k == "reexport";
553549
};
554-
auto looks_like_inline_dep_spec = [&](const t::Table& sub) {
550+
// What makes a table an inline dep spec is that it names a SOURCE. This
551+
// used to be "every key is known", which quietly coupled two unrelated
552+
// things: the discriminator (spec vs nested namespace table) and the
553+
// vocabulary (which keys mean something).
554+
//
555+
// The coupling is a compatibility hazard, not a style problem. A manifest
556+
// using a key introduced after the reader was built did not get "unknown
557+
// option" — the table failed the discriminator, was taken for a NAMESPACE,
558+
// and the user was told their `reexport = true` "must be a string, inline
559+
// dep table, or nested table". Worse, a published package cannot adopt a
560+
// new key at all, because every older client fails to load it outright
561+
// rather than ignoring what it does not understand. That is the same
562+
// property #349 established for the index floor: data must not be able to
563+
// decide whether the program works.
564+
//
565+
// An identity key is an unambiguous discriminator: a nested namespace
566+
// table's keys are PACKAGE names, and no package is named `version` /
567+
// `path` / `git` / `workspace`.
568+
auto looks_like_inline_dep_spec = [](const t::Table& sub) {
555569
if (sub.empty()) return false;
556-
for (auto& [sk, sv] : sub) {
557-
if (!is_dep_spec_key(sk)) return false;
558-
}
559-
return true;
570+
for (auto& [sk, sv] : sub)
571+
if (sk == "path" || sk == "version" || sk == "git" || sk == "workspace")
572+
return true;
573+
return false;
560574
};
561575

562576
auto fill_inline_spec = [&](DependencySpec& spec,
563577
std::string_view section,
564578
std::string_view fqName,
565579
const t::Table& sub) -> std::expected<void, ManifestError>
566580
{
581+
// Now that the discriminator no longer doubles as the vocabulary, an
582+
// unrecognized key can be REPORTED — as a degradation, so `--strict`
583+
// still refuses it, while an ordinary build of a package written for a
584+
// newer mcpp proceeds with the part this one understands. Same
585+
// discipline as the xpkg reader's `xpkgUnknownKeys`: record rather
586+
// than swallow, and never fail the whole load over it.
587+
for (auto& [sk, sv] : sub) {
588+
if (is_dep_spec_key(sk)) continue;
589+
m.schemaWarnings.push_back(std::format(
590+
"[{}.\"{}\"] has unrecognized key '{}' (ignored). It may be a "
591+
"typo, or a field a newer mcpp understands.", section, fqName, sk));
592+
}
567593
if (auto it = sub.find("path"); it != sub.end() && it->second.is_string()) spec.path = it->second.as_string();
568594
if (auto it = sub.find("version"); it != sub.end() && it->second.is_string()) spec.version = it->second.as_string();
569595
if (auto it = sub.find("git"); it != sub.end() && it->second.is_string()) spec.git = it->second.as_string();
@@ -664,9 +690,10 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
664690
auto& sub = value.as_table();
665691
if (!looks_like_inline_dep_spec(sub)) {
666692
return std::unexpected(error(origin, std::format(
667-
"[{}.{}] must be a version string or table of "
668-
"(path/version/git/rev/tag/branch/features/default-features/"
669-
"visibility/backend/workspace/tools/host-module/reexport)",
693+
"[{}.{}] must be a version string, or a table naming a "
694+
"source (one of path/version/git/workspace) alongside any "
695+
"of rev/tag/branch/features/default-features/visibility/"
696+
"backend/tools/host-module/reexport",
670697
section, key)));
671698
}
672699
if (auto r = fill_inline_spec(spec, section, key, sub); !r) return r;

‎tests/unit/test_manifest.cpp‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3160,3 +3160,76 @@ bypath = { path = "../sibling" }
31603160
EXPECT_NE(std::find(all->features.begin(), all->features.end(), "backend-openblas"),
31613161
all->features.end());
31623162
}
3163+
3164+
// #359: a manifest written for a NEWER mcpp must still load.
3165+
//
3166+
// What made this impossible was that one predicate served two purposes: it
3167+
// decided "inline dep spec vs nested namespace table" AND enumerated the
3168+
// vocabulary. A key the reader did not know therefore did not read as an
3169+
// unknown option — the table failed the discriminator, was taken for a
3170+
// namespace, and the user was told their `reexport = true` "must be a string,
3171+
// inline dep table, or nested table". A published package could consequently
3172+
// never adopt a new key: every older client failed to load it outright.
3173+
//
3174+
// The discriminator is now "does it name a source", which a namespace table
3175+
// cannot accidentally satisfy, and an unrecognized key is a recorded
3176+
// degradation (--strict still refuses it).
3177+
TEST(Manifest, UnknownDependencyKeyDegradesInsteadOfFailingTheLoad) {
3178+
auto tmp = std::filesystem::temp_directory_path() / "mcpp_dep_future_key";
3179+
std::filesystem::create_directories(tmp);
3180+
auto path = tmp / "mcpp.toml";
3181+
{
3182+
std::ofstream os(path);
3183+
os << R"(
3184+
[package]
3185+
name = "futurekey"
3186+
version = "0.1.0"
3187+
3188+
[dependencies]
3189+
zlib = { version = "1.3.1", some-future-field = true }
3190+
)";
3191+
}
3192+
auto m = mcpp::manifest::load(path);
3193+
ASSERT_TRUE(m) << (m ? "" : m.error().message);
3194+
3195+
const mcpp::pm::DependencySpec* z = nullptr;
3196+
for (auto const& [k, spec] : m->dependencies)
3197+
if (spec.shortName == "zlib") z = &spec;
3198+
ASSERT_NE(z, nullptr);
3199+
EXPECT_EQ(z->version, "1.3.1"); // the part this reader understands still applies
3200+
3201+
bool warned = false;
3202+
for (auto const& w : m->schemaWarnings)
3203+
if (w.find("some-future-field") != std::string::npos) warned = true;
3204+
EXPECT_TRUE(warned) << "the ignored key must be reported, not swallowed";
3205+
}
3206+
3207+
// The discriminator must not mistake a namespace subtable for an inline spec.
3208+
// `[dependencies.compat] zlib = "1.3.1"` has one key, `zlib`, which names no
3209+
// source — so it is a namespace, and the entry below it is the dependency.
3210+
TEST(Manifest, NamespaceSubtableIsNotMistakenForAnInlineSpec) {
3211+
auto tmp = std::filesystem::temp_directory_path() / "mcpp_ns_vs_spec";
3212+
std::filesystem::create_directories(tmp);
3213+
auto path = tmp / "mcpp.toml";
3214+
{
3215+
std::ofstream os(path);
3216+
os << R"(
3217+
[package]
3218+
name = "nsvsspec"
3219+
version = "0.1.0"
3220+
3221+
[dependencies.compat]
3222+
zlib = "1.3.1"
3223+
gtest = { version = "1.15.2", features = ["main"] }
3224+
)";
3225+
}
3226+
auto m = mcpp::manifest::load(path);
3227+
ASSERT_TRUE(m) << (m ? "" : m.error().message);
3228+
std::set<std::string> got;
3229+
for (auto const& [k, spec] : m->dependencies) {
3230+
got.insert(spec.shortName);
3231+
EXPECT_EQ(spec.namespace_, "compat") << spec.shortName;
3232+
}
3233+
EXPECT_TRUE(got.contains("zlib"));
3234+
EXPECT_TRUE(got.contains("gtest"));
3235+
}

0 commit comments

Comments
 (0)