fix: a member inside an index archive inherits its workspace; publish reads the one key table (#690)

inherit_as_workspace_member is the one function for a sibling path
dependency, a git-hosted member and a member inside an index package's
archive (Form A pointer), searched no higher than the install root.
normalize.cppm writes [workspace.build] back through kWorkspaceBuildKeys
instead of a second copy of the key set. e2e 774.

Author: speak-agent

src/build/prepare.cppm

@@ -734,6 +734,54 @@ unfolded_defines_error(const mcpp::manifest::Manifest& m) {
         d.size(), d.size() == 1 ? "y" : "ies", d.front());
 }
 
+// WHAT A MEMBER RECEIVES FROM ITS WORKSPACE WHEN IT IS REACHED AS A DEPENDENCY.
+//
+// Three parts of the inheritance matter to a dependency: `[workspace.package]`
+// (a member may omit `version`), `x.workspace = true` dependency entries
+// (without the merge the entry reaches resolution with neither version nor
+// path), and `[workspace.build]`. They are applied at the dependency's LOAD
+// site, before the conditional merge and the `defines` fold, which is the
+// order the root follows; `makePackageRoot` only captures the result (#690).
+// `[toolchain]`, `[target.<triple>]` and `[indices]` are decided by the root
+// for the whole graph and are not applied to a dependency.
+//
+// One function for every way a member is reached: a sibling `path`
+// dependency, a member of a git-hosted workspace, and a member inside an
+// index package's archive. The same commit then compiles the same way in its
+// own checkout and in every consumer's graph.
+std::optional<std::string>
+inherit_as_workspace_member(mcpp::manifest::Manifest& member,
+                            const mcpp::manifest::Manifest& workspace,
+                            const std::filesystem::path& workspaceRoot,
+                            const std::filesystem::path& memberDir) {
+    mcpp::project::inherit_workspace_package(member, workspace);
+    mcpp::project::merge_workspace_deps(member, workspace, workspaceRoot);
+    mcpp::project::inherit_workspace_build(member, workspace, workspaceRoot);
+    return mcpp::project::workspace_inheritance_error(member, memberDir);
+}
+
+// The workspace whose `members` list `memberDir`, searched upward from its
+// parent and never above `bound` (an index package's install root: the
+// archive is the only tree the package's author wrote).
+std::optional<std::pair<mcpp::manifest::Manifest, std::filesystem::path>>
+workspace_listing(const std::filesystem::path& memberDir,
+                  const std::filesystem::path& bound) {
+    auto inside = [&](const std::filesystem::path& p) {
+        auto rel = p.lexically_normal().lexically_relative(bound.lexically_normal());
+        return !rel.empty() && *rel.begin() != "..";
+    };
+    for (auto p = memberDir.parent_path(); inside(p); p = p.parent_path()) {
+        if (std::filesystem::exists(p / "mcpp.toml")) {
+            if (auto ws = mcpp::manifest::load(p / "mcpp.toml");
+                ws && ws->workspace.present
+                && mcpp::project::is_workspace_member(*ws, p, memberDir))
+                return std::pair{std::move(*ws), p};
+        }
+        if (p == p.parent_path()) break;
+    }
+    return std::nullopt;
+}
+
 // ── The SECOND conditional pass: predicates that name a target-side layer ────
 //
 // #540/#494. `docs/14` documents a package adapting to the C library it was
@@ -2327,7 +2375,7 @@ prepare_build(bool print_fingerprint,
     //
     // A PRELOADED manifest (a host-tool sub-build) is already effective: the
     // resolver loaded it at the dependency's load site, where a member
-    // inherits (see `inheritAsMember`). It is not inherited a second time; the
+    // inherits (see `inherit_as_workspace_member`). It is not inherited a second time; the
     // workspace it belongs to is still recorded below, so that its own sibling
     // dependencies inherit as members.
     std::optional<mcpp::project::EffectiveManifest> effective;
@@ -6199,10 +6247,22 @@ prepare_build(bool print_fingerprint,
         auto loadFrom = [&](const std::filesystem::path& mcppToml)
             -> std::expected<void, std::string>
         {
-            auto dm = mcpp::manifest::load(mcppToml);
+            // A manifest that is a member of a workspace inside the archive
+            // receives that workspace's inheritance, as it does from a git
+            // clone of the same commit (#690).
+            auto repoWorkspace = workspace_listing(mcppToml.parent_path(), verRoot);
+            auto dm = mcpp::manifest::load(
+                mcppToml, {.insideWorkspace = repoWorkspace.has_value()});
             if (!dm) return std::unexpected(std::format(
                 "dependency '{}' (at '{}'): {}",
                 depName, mcppToml.string(), dm.error().format()));
+            if (repoWorkspace) {
+                if (auto bad = inherit_as_workspace_member(
+                        *dm, repoWorkspace->first, repoWorkspace->second,
+                        mcppToml.parent_path()))
+                    return std::unexpected(std::format(
+                        "dependency '{}': {}", depName, *bad));
+            }
             manifest = std::move(*dm);
             effRoot  = mcppToml.parent_path();
             return {};
@@ -8279,32 +8339,13 @@ prepare_build(bool print_fingerprint,
                     name, dep_root.string(), dm.error().format()));
             }
             dep_manifest = std::move(*dm);
-            // A MEMBER REACHED AS A DEPENDENCY INHERITS HERE, AT ITS LOAD SITE,
-            // EXACTLY AS THE ROOT INHERITS AT ITS OWN.
-            //
-            // Three parts of what a member receives from its workspace matter
-            // to a dependency: `[workspace.package]` (a member may omit
-            // `version`), `x.workspace = true` dependency entries (without the
-            // merge the entry reaches resolution with no version and no path,
-            // and is reported as an unreadable index entry), and
-            // `[workspace.build]`. All three run before the conditional merge
-            // and the `defines` fold below, which is the order the root
-            // follows; the snapshot in `makePackageRoot` only captures the
-            // result (#690). The remaining parts of `inherit_workspace_config`
-            // (`[toolchain]`, `[target.<triple>]`, `[indices]`) are decided by
-            // the root for the whole graph and are not applied to a dependency.
-            //
-            // A member of a git-hosted workspace inherits from ITS repository,
-            // anchored at the clone, so that the same commit compiles the same
-            // way in its own checkout and in a consumer's graph.
+            // A member reached as a dependency inherits here, at its load
+            // site; see `inherit_as_workspace_member`. A member of a
+            // git-hosted workspace inherits from ITS repository, anchored at
+            // the clone.
             auto inheritAsMember = [&](const mcpp::manifest::Manifest& ws,
-                                       const std::filesystem::path& wsRoot)
-                -> std::optional<std::string> {
-                mcpp::project::inherit_workspace_package(*dep_manifest, ws);
-                mcpp::project::merge_workspace_deps(*dep_manifest, ws, wsRoot);
-                mcpp::project::inherit_workspace_build(*dep_manifest, ws, wsRoot);
-                return mcpp::project::workspace_inheritance_error(
-                    *dep_manifest, dep_root);
+                                       const std::filesystem::path& wsRoot) {
+                return inherit_as_workspace_member(*dep_manifest, ws, wsRoot, dep_root);
             };
             if (depIsMember) {
                 if (auto bad = inheritAsMember(*wsManifest, runtimeWorkspaceRoot))

src/publish/normalize.cppm

@@ -58,46 +58,12 @@ namespace {
 
 namespace t = mcpp::libs::toml;
 
-// `[workspace.build]` keys, each paired with the field of `BuildConfig` that
-// carries it. Vectors of flags, vectors of directories and scalars are written
-// back by three different rules, so the table records which rule applies.
-struct StringVectorKey {
-    std::string_view key;
-    std::vector<std::string> mcpp::manifest::BuildConfig::* field;
-};
-struct PathVectorKey {
-    std::string_view key;
-    std::vector<std::filesystem::path> mcpp::manifest::BuildConfig::* field;
-};
-struct ScalarKey {
-    std::string_view key;
-    std::string mcpp::manifest::BuildConfig::* field;
-};
-
+// The `[workspace.build]` keys are read from `mcpp::manifest::kWorkspaceBuildKeys`,
+// the one statement of the inheritable subset that the parser also reads.
+// Vectors of flags, vectors of directories and scalars are written back by
+// three different rules, selected by the type of the row's field.
 using BC = mcpp::manifest::BuildConfig;
 
-const StringVectorKey kStringVectors[] = {
-    {"cflags",           &BC::cflags},
-    {"cxxflags",         &BC::cxxflags},
-    {"ldflags",          &BC::ldflags},
-    {"defines",          &BC::defines},
-    {"dialect_cxxflags", &BC::dialectCxxflags},
-};
-const PathVectorKey kPathVectors[] = {
-    {"include_dirs",         &BC::includeDirs},
-    {"include_dirs_after",   &BC::includeDirsAfter},
-    {"private_include_dirs", &BC::privateIncludeDirs},
-};
-const ScalarKey kScalars[] = {
-    {"c_standard",              &BC::cStandard},
-    {"linkage",                 &BC::linkage},
-    {"target",                  &BC::target},
-    {"cxx_runtime",             &BC::cxxRuntime},
-    {"dependency_linkage",      &BC::dependencyLinkage},
-    {"macos_deployment_target", &BC::macosDeploymentTarget},
-    {"ios_deployment_target",   &BC::iosDeploymentTarget},
-};
-
 t::Value string_array(const std::vector<std::string>& v) {
     t::Array a;
     for (auto const& s : v) a.emplace_back(s);
@@ -380,47 +346,49 @@ normalize_for_publish(const std::filesystem::path&           packageDir,
                 if (!build) build = table_at(root, "build", "build", explicitTables);
                 return build;
             };
-            for (auto const& k : kStringVectors) {
-                if ((w.*k.field).empty()) continue;
-                auto* bt = build_table();
-                if (!bt) return std::unexpected(std::format(
-                    "{}: `build` is not a table", manifestPath.string()));
-                (*bt)[std::string(k.key)] = string_array(b.*k.field);
-                changed = true;
-            }
-            for (auto const& k : kPathVectors) {
-                const auto n = (w.*k.field).size();
-                if (n == 0) continue;
-                std::vector<std::string> dirs;
-                const auto& all = b.*k.field;
-                for (std::size_t i = 0; i < all.size(); ++i) {
-                    if (i >= n) { dirs.push_back(all[i].string()); continue; }
-                    if (!inside(packageDir, all[i]))
-                        return std::unexpected(std::format(
-                            "{}: [workspace.build] {} entry '{}' resolves to '{}', "
-                            "outside this package's directory, and the published "
-                            "archive contains only that directory. Move the headers "
-                            "into the package, or declare the directory in the "
-                            "package's own [build] for its own build only.",
-                            manifestPath.string(), k.key, (w.*k.field)[i].string(),
-                            all[i].lexically_normal().string()));
-                    dirs.push_back(all[i].lexically_normal()
-                        .lexically_relative(packageDir.lexically_normal()).generic_string());
+            for (auto const& row : mcpp::manifest::kWorkspaceBuildKeys) {
+                const std::string key(row.key);
+                if (auto const* f = std::get_if<std::vector<std::string> BC::*>(&row.field)) {
+                    if ((w.**f).empty()) continue;
+                    auto* bt = build_table();
+                    if (!bt) return std::unexpected(std::format(
+                        "{}: `build` is not a table", manifestPath.string()));
+                    (*bt)[key] = string_array(b.**f);
+                    changed = true;
+                } else if (auto const* f = std::get_if<
+                               std::vector<std::filesystem::path> BC::*>(&row.field)) {
+                    const auto n = (w.**f).size();
+                    if (n == 0) continue;
+                    std::vector<std::string> dirs;
+                    const auto& all = b.**f;
+                    for (std::size_t i = 0; i < all.size(); ++i) {
+                        if (i >= n) { dirs.push_back(all[i].string()); continue; }
+                        if (!inside(packageDir, all[i]))
+                            return std::unexpected(std::format(
+                                "{}: [workspace.build] {} entry '{}' resolves to '{}', "
+                                "outside this package's directory, and the published "
+                                "archive contains only that directory. Move the headers "
+                                "into the package, or declare the directory in the "
+                                "package's own [build] for its own build only.",
+                                manifestPath.string(), key, (w.**f)[i].string(),
+                                all[i].lexically_normal().string()));
+                        dirs.push_back(all[i].lexically_normal()
+                            .lexically_relative(packageDir.lexically_normal()).generic_string());
+                    }
+                    auto* bt = build_table();
+                    if (!bt) return std::unexpected(std::format(
+                        "{}: `build` is not a table", manifestPath.string()));
+                    (*bt)[key] = string_array(dirs);
+                    changed = true;
+                } else if (auto const* f = std::get_if<std::string BC::*>(&row.field)) {
+                    if ((w.**f).empty()) continue;
+                    auto* bt = build_table();
+                    if (!bt) return std::unexpected(std::format(
+                        "{}: `build` is not a table", manifestPath.string()));
+                    if (bt->contains(key)) continue;
+                    (*bt)[key] = t::Value{b.**f};
+                    changed = true;
                 }
-                auto* bt = build_table();
-                if (!bt) return std::unexpected(std::format(
-                    "{}: `build` is not a table", manifestPath.string()));
-                (*bt)[std::string(k.key)] = string_array(dirs);
-                changed = true;
-            }
-            for (auto const& k : kScalars) {
-                if ((w.*k.field).empty()) continue;
-                auto* bt = build_table();
-                if (!bt) return std::unexpected(std::format(
-                    "{}: `build` is not a table", manifestPath.string()));
-                if (bt->contains(k.key)) continue;
-                (*bt)[std::string(k.key)] = t::Value{b.*k.field};
-                changed = true;
             }
         }
     }

tests/e2e/774_an_index_member_inherits_its_archive_workspace.sh

@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+# requires: python3
+# 774 -- a member inside an index package's archive receives its workspace's
+# inheritance, as the same commit does from its own checkout and from a git
+# clone (#690, design record F11).
+#
+# The index descriptor points at a member manifest (`mcpp = "*/libs/wlib/mcpp.toml"`)
+# inside an archive whose root is a workspace declaring `[workspace.package]
+# version` and `[workspace.build] defines`. The member omits `version` and
+# guards the define with `#error`. Before #690 the member was loaded as a
+# stand-alone manifest and refused for the missing version.
+#
+# The archive is seeded into the install path, so the test needs no network:
+# the resolver accepts an installed tree whose layout matches the descriptor.
+set -e
+
+T=$(mktemp -d)
+trap 'rm -rf "$T"' EXIT
+export MCPP_HOME="$T/home"
+source "$(dirname "$0")/_inherit_toolchain.sh"
+fail() { echo "FAIL: $1"; [ -n "${2:-}" ] && cat "$2"; exit 1; }
+
+X="$MCPP_HOME/registry/data/xpkgs/probe774-x-wlib/1.0.0/wrepo-1.0.0"
+mkdir -p "$X/libs/wlib" "$T/idx/pkgs/p" "$T/app/src"
+printf '[workspace]\nmembers = ["libs/wlib"]\n\n[workspace.package]\nversion = "1.0.0"\n\n[workspace.build]\ndefines = ["REPO_DEF=4"]\n' > "$X/mcpp.toml"
+printf '[package]\nnamespace = "probe774"\nname = "wlib"\n\n[targets.wlib]\nkind = "lib"\n\n[build]\nsources = ["w.cpp"]\n' > "$X/libs/wlib/mcpp.toml"
+printf '#ifndef REPO_DEF\n#error "the archive workspace did not reach its member"\n#endif\nint w_v() { return REPO_DEF; }\n' > "$X/libs/wlib/w.cpp"
+cat > "$T/idx/pkgs/p/probe774.wlib.lua" <<'EOF'
+package = {
+    spec = "1",
+    namespace = "probe774",
+    name = "probe774.wlib",
+    description = "Form A member inside a workspace archive",
+    licenses = {"MIT"},
+    type = "package",
+    xpm = {
+        linux   = { ["1.0.0"] = { url = "https://example.invalid/wrepo-1.0.0.tar.gz", sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } },
+        macosx  = { ["1.0.0"] = { url = "https://example.invalid/wrepo-1.0.0.tar.gz", sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } },
+        windows = { ["1.0.0"] = { url = "https://example.invalid/wrepo-1.0.0.tar.gz", sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } },
+    },
+    mcpp = "*/libs/wlib/mcpp.toml",
+}
+EOF
+printf '[package]\nname = "app"\nversion = "0.1.0"\n\n[indices]\nprobe774 = { path = "../idx" }\n\n[dependencies.probe774]\nwlib = "1.0.0"\n\n[targets.app]\nkind = "bin"\nmain = "src/main.cpp"\n' > "$T/app/mcpp.toml"
+printf 'int w_v();\nint main() { return w_v() == 4 ? 0 : 1; }\n' > "$T/app/src/main.cpp"
+cd "$T/app"
+"$MCPP" run > run.log 2>&1 || fail "the member inside the archive did not build or run" run.log
+cdb=compile_commands.json
+n=$(python3 - "$cdb" <<'EOF'
+import json, sys
+for e in json.load(open(sys.argv[1])):
+    if e["file"].replace("\\", "/").endswith("libs/wlib/w.cpp"):
+        print(e["arguments"].count("-DREPO_DEF=4")); sys.exit(0)
+print("missing")
+EOF
+)
+[ "$n" = 1 ] || fail "-DREPO_DEF=4 occurs $n times in the member's unit"
+
+echo "PASS: 774_an_index_member_inherits_its_archive_workspace"