feat: #230–#243 batch unified — #239/#240/#237/#241/#242/#238 + #243 design (0.0.98)#245
Merged
Conversation
…of-root gen source) Two follow-up gaps in #233's object-path disambiguation, each reproduced as a failing e2e test (red on 0.0.97) plus a decomposed design doc: - 123 (#240): a dependency and the consumer ship a same-named source (the near-universal both-have-src/main.cpp). #233 renames the scanned consumer main to obj/<pkg>/src/main.o, but the link still references the stale flat obj/main.o -> 'obj/main.o missing and no known rule'. Blocks mcpplibs #79. - 124 (#239): a dependency build.mcpp emits a source into OUT_DIR (outside the package root); its relPath carries '..', so #233 pastes obj/<pkg>/../.. and the object escapes obj/ (and the '@' in <name>@<ver> gets ninja-quoted, breaking the #235 "$out.d" depfile redirect). Design: .agents/docs/2026-07-19-object-path-disambiguation-followups-239-240-design.md
…iguation (#240, #239) #233 disambiguated object paths only for scanner-produced units, leaving two gaps that both surface as broken/invalid ninja edges. Converge object-path assignment into one source of truth (object_for) and make the collision prefix always downward + shell-safe (safe_object_prefix). #240 — the entry main's object path was recomputed independently as flat obj/<name>.o (plan.cppm) and pushed to the link inputs even when the source had already been scanned into a disambiguated compile unit. Fix: - the synthesized main no longer computes its own object path; - if the entry was globbed (scanned), the link reuses THAT unit's already- disambiguated object; - if not scanned, it is added to the basename census and routed through the same object_for, so it disambiguates against a dependency's same-named source too. Common single-binary projects (unique main) stay flat obj/main.o. #239 — the collision prefix was u.relPath.parent_path() pasted raw. For sources outside their package root (dependency build.mcpp OUT_DIR-generated sources) the relPath carries '..'/absolute roots, so obj/<prefix> climbed out of the build tree, and the '@' in <name>@<ver> got ninja-quoted (breaking the #235 "$out.d" redirect). safe_object_prefix folds each component downward: drop root, '.' skipped, '..'->'__up', non-portable chars (e.g. '@')->'_'. Per-component injective, so #233's uniqueness (and its L1b assertion backstop) holds. Non-colliding paths are untouched (flat obj/<name>.o); colliding paths with a clean relPath are byte-identical to before.
CHANGELOG entry for the two #233 follow-up fixes. Bootstrap pin bump + release mirror + index are the usual post-merge operational steps (not in this PR).
…erved architecture assessment Authoritative per-issue ledger (classification / status / root-cause subsystem / verified file:line anchor / root-cause-level fix / commit) for the whole #230-#243 wave, plus the standing no-workaround discipline and the criteria for the final holistic architecture assessment (§4, filled once all land). Records the current state: #230 already fixed (0.0.96); #239/#240 fixed on this branch (0.0.98); #237/#241/#242 mechanical root-cause fixes pending; #243 needs design; #238 root cause lives in xlings (not fixable in this repo, must not be band-aided as done).
…ou-mean (#237) The mcpp-segment key vocabulary is a closed whitelist; the parser collected an unknown key into xpkgUnknownKeys and skipped it, but only \`mcpp xpkg parse\` surfaced it. So a descriptor written with \`dependencies = {...}\` (correct key: \`deps\`) built with the dependency silently dropped and no diagnostic. - closest_known_xpkg_key (pure, manifest layer): maps an unknown key to its most likely intended one — a well-known-confusable table (dependencies->deps, ...) plus a Levenshtein<=2 fallback for typos; "" when nothing is close. The vocabulary array is kept in sync with the parse loop else-if chain. - warn_unknown_xpkg_keys (prepare.cppm, owns the ui layer): warns per unknown key with the suggestion, at the two descriptor-adoption sites (a fetched dep synthesized from the index mcpp={} block). Warning not hard-error keeps forward-compat (older mcpp building a newer descriptor). Extends the 0.0.97 closed-grammar loud-failure principle to the xpkg build path. Unit test: XpkgUnknownKeys.CollectedAndDidYouMean.
…241) A package build.mcpp had no contract way to locate a dependency payload (the real case: compat.opencv unifont feature -> data-asset package compat.opencv-unifont, whose font blob the build.mcpp reads). It had to reverse-engineer the store layout, turning store internals into a package-side API. - BuildProgramEnv.depDirs (name -> dir); contract_env emits MCPP_DEP_<SANITIZED_NAME>_DIR (same sanitizer as MCPP_FEATURE_), so it also auto-participates in the build.mcpp rerun hash. - mcpp::dep_dir("<name>") helper added to the bundled build.mcpp module. - Populated at the dependency-build.mcpp site from the authoritative consumer->dep edge graph (no name-guessing); covers feature-activated deps (mergeActiveFeatureDeps folds them into dependencies before edges record). Scope: the dependency-build.mcpp path (the stated scenario). The ROOT project build.mcpp runs before dependency resolution so it does not yet receive these; tracked as a follow-up in the #230-#243 ledger. e2e 125.
…a dependency's default feature set (#242) A dependency can declare a default feature set via `[features] default = [...]`, but a consumer had no way to opt out of it (Cargo's `default-features = false`) — blocking e.g. compat.ffmpeg's slimmed profile. Root cause fixed at the single convergence point: feature_closure() UNCONDITIONALLY seeded `[features].default`. It now takes a `seedDefault` flag (default true); when false the dependency's own default set is not seeded, so only explicitly requested `features = [...]` (and their transitive `implies`) activate. No side special-case. Threading: - DependencySpec gains `bool defaultFeatures = true` (pm/dep_spec.cppm). - toml.cppm: `default-features` added to is_dep_spec_key (so `{ default-features = false }` is recognized as an inline dep spec) and read in fill_inline_spec. - prepare.cppm: every feature_closure caller threads the flag. Root package and root build.mcpp keep seedDefault=true; each dependency (main feature pass, Stage 2a feature-deps merge, dep build.mcpp env) passes its dep spec's defaultFeatures. xpkg descriptor `deps` entries carry only a version string (no per-dep features), so no default-features knob applies there — left untouched. Tests: unit Manifest.ParsesDependencyDefaultFeaturesOptOut asserts the flag parses (false on opt-out, true by default) with explicit features intact; e2e 126 exercises a path-dep whose default feature's define is ABSENT when opted out, PRESENT by default, and re-activatable via explicit features.
…es fails with multiple index repos (#238, root cause in xlings) The opaque `fetch <pkg>@<ver> failed (exit 1)` hid the context mcpp actually has. The known #238 failure mode — >=2 project-level index_repos (root [indices] inheritance + a default-ns redirect, #224) making the xlings resolver silently exit non-zero — left the user with no next step. The NDJSON child emits {"exitCode":1} with no error event, so mcpp reconstructs the diagnostic from what it knows. - InstallProgressHandler.captured_error(): retain any error/warn/error-event text the child DID emit (never discard it). - read_seeded_index_repos + format_install_failure_diagnostic (pure, unit- tested): name the target, the configured index repos, flag the known >=2-repo xlings resolution gap + link the tracking issue, fold in child output, and point at MCPP_VERBOSE=1 for the raw invocation. - xlings::call logs the exact interface invocation under MCPP_VERBOSE. DIAGNOSTICS ONLY: the correctness fix belongs in openxlings/xlings (multi-repo resolution in install_packages). This makes the failure diagnosable, not fixed.
…, follow-up PR Scopes the real gap: feature-conditional deps already exist ([feature-deps.<name>] + xpkg features.x.deps -> Manifest::featureDeps); the missing half is dep/feat FORWARDING. Recommends a single per-package requested-feature funnel (requestedFeaturesByPkg) consumed by both mergeActiveFeatureDeps and apply(), which also closes the transitive dep->dep gap (prepare.cppm:2653 TODO). Composes additively with #242 default-features opt-out. Implementation deferred to a follow-up PR (0.0.99+).
…#374 The install_packages multi-repo failure diagnostic, ledger, and CHANGELOG now point at the concrete upstream tracking issue (openxlings/xlings#374) for the root-cause fix. mcpp side remains diagnostics-only.
…y edge graph (#242 transitive; #241 naming) Post-#242 architecture review found the feature subsystem derived a package's requested-feature set in two places that disagreed on transitive edges: resolution (mergeActiveFeatureDeps) read the true per-edge spec, but activation (apply) + the dep build.mcpp env re-derived it by scanning ONLY the root manifest's direct deps. So a transitive dep's requested features and its consumer's `default-features = false` were silently dropped — activation kept seeding a default feature (defining its macro / keeping default-gated sources) that resolution had already skipped, which at best ignores the opt-out and at worst compiles default-gated sources against a dep that was never resolved. - DependencyEdge now carries the per-edge requestedFeatures + defaultFeatures (recorded in recordDependencyEdge from the spec). - New aggregatedRequest(depPkgIndex): union requested features and OR default-features over ALL incoming edges (Cargo diamond semantics), sourced from the authoritative edge graph. Both feature activation and the dep build.mcpp env consume it. Direct-dep behavior is unchanged (root edge carries root spec); transitive edges are now honored, closing the pre-existing "transitive dep->dep feature requests not propagated" gap too. - #241 (review finding): emit MCPP_DEP_<NAME>_DIR under BOTH the dep's canonical name AND its short (namespace-stripped) name, so dep_dir("compat.zlib") and dep_dir("zlib") both resolve (code/doc no longer contradict); + a collision guard in contract_env (two names sanitizing to one var warns instead of silent last-wins). e2e 127 (transitive opt-out honored + control). All feature e2e (67/71/72/79/ 80/81/82/83/100/106/125/126) + unit 35/35 stay green.
…nvergence optimization Independent adversarial review of the full batch diff; per-issue root-cause vs workaround verdict; consistency/stability/tech-debt reckoning; and the executed headline optimization (feature request set converged onto the DependencyEdge graph). All fixes assessed root-cause; the one real latent risk (#242 transitive default-features) fixed by convergence, not patch. Residual debt explicitly logged (no silent leftovers).
Per maintainer decision, ship the whole batch (#239/#240 object-path + #237/#241/ #242/#238-diag + #243 design + feature-edge convergence) as ONE 0.0.98 release instead of split 0.0.98/0.0.99. Roll MCPP_VERSION back to 0.0.98, merge the CHANGELOG sections, restore the dropped [0.0.97] header, and retag the ledger commit notes to 0.0.98. #243 forwarding impl stays a 0.0.99+ follow-up.
Sunrisepeak
changed the title
This was referenced Jul 18, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unified #230–#243 batch shipped as a single 0.0.98 release (per maintainer decision — supersedes the earlier split; #244 closed).
Object-path (was 0.0.98):
object_for(e2e 123).safe_object_prefixkeeps out-of-root/@object paths downward + shell-safe (e2e 124).Batch (was 0.0.99):
MCPP_DEP_<NAME>_DIRbuild.mcpp contract (mcpp::dep_dir(), canonical+short, collision-guarded, edge-graph sourced).default-features = false+ transitive-edge convergence (feature request derivation unified ontoDependencyEdge/aggregatedRequest; retires the transitive-feature-propagation gap) — e2e 127.install_packagesfailures. Root cause in xlings → install_packages silently fails (exit 1) when ≥2 project-level index_repos are configured openxlings/xlings#374 (mcpp side diagnostics-only).Governance ledger + unified architecture assessment (§4, independent adversarial review — all fixes root-cause, residual debt logged):
.agents/docs/2026-07-19-issues-230-243-batch-ledger-and-architecture-assessment.md.Local: unit 35/35; all feature/object-path/dep e2e green (128 passed; 22/54/62 known-env). Built clean at 0.0.98.