Repository navigation
Follow-ups from #761/#763/#765: link-planning gaps, PE export surface and key naming, glob input boundaries #766
Description
Context
A combined review of #761 (multi-target executables vs. sibling shared targets), #763 (LLVM LTO DLL exports, auto_export) and #765 (parent-directory rerun_if_changed_glob inputs) found the three fixes correct and consistent with each other:
- The three branches merge cleanly, pairwise and together. They share three byte-identical files:
README.zh-CN.md,tests/e2e/_host_path.shandtests/e2e/00_fixture_path_hygiene.sh. - A binary built from all three merged passes E2E
00,880,882,194,612and872on Linux x86_64 / gcc 16.1. E2E881requires MSVC and is covered by Windows CI.
This issue records the remaining problems: gaps the reviews found, pre-existing defects they exposed, and naming and specification points. None of them is a regression introduced by these PRs. They are grouped by priority, and each item can be fixed in its own PR.
P0: decide before the next release
1. [targets.<name>] auto_export must follow the platform-key naming convention (#763)
auto_export only has an effect on MSVC-ABI PE shared libraries. ELF, Mach-O, MinGW, static libraries and executables ignore it. The existing platform-scoped target keys all carry the platform in their name, and docs/04 states that they "render nothing on a target that is not PE":
windows_subsystemwindows_entrywindows_code_page
A neutral name suggests a cross-platform meaning. For example, users may expect auto_export = false on ELF to behave like -fvisibility=hidden, but it does nothing there. SPEC-004 §5.2 forbids renaming a key once it appears in a released descriptor. The name must therefore be settled before the key ships. It is currently documented as "unreleased".
Rule: a key that is not general must be named according to the spec. A key that is strongly tied to one platform carries that platform's prefix.
- Rename to
windows_auto_export(recommended). Alternatively, give the key a real cross-platform meaning, which is a larger design question (see item 3). - Update the TOML reader, the xpkg reader, the unit tests, E2E 881, docs/04 (en/zh) and the indexes.
- Replace the "(unreleased)" marker with the release version
(mcpp X.Y.Z+), following the existing convention.
2. The documentation claims that exports remains effective on MSVC, but the PE path never applies it (#763 docs, pre-existing code)
The new docs/04 text says: "Native __declspec(dllexport), linker flags and explicit exports lists remain effective when discovery is disabled", and "exports narrows the linker's published set". The code shows the PE path never uses exportPatterns:
exports_flag()returns""foros == "windows"(src/build/ninja_backend.cppm). Its comment says narrowing is the.defstep's business.- The
coff_defrule receives only--output --name $coff_tools $in. No patterns are passed. coff_exports.cppm,pe_exports.cppmandcmd_coff_defdo no pattern filtering.- No E2E covers
exports = ...on Windows.
On MSVC, exports is therefore a no-op today. Combined with auto_export = false, a target that only lists exports = [...] produces a DLL with no exports. Without exports, the linker also writes no import library, so consumers fail far from the cause. The pre-#763 docs had the same misleading sentence about exports narrowing the generated .def.
- Choose one:
- (a) Implement pattern filtering in
coff-def: pass the patterns, filter the candidates and add a Windows E2E. - (b) Correct docs/04 (en/zh) to state that
exportscurrently has no effect on PE.
- (a) Implement pattern filtering in
- Diagnose a PE shared link that ends up with no exports and no import library, for example with
auto_export = falseand no annotations, instead of letting consumers fail later.
P1: correctness
3. Executables omit the shared dependencies of statics placed in their owner's own image (#761 follow-up)
Both #761 paths (workspace members, src/build/plan.cppm around line 3179, and artifacts, around line 2937) now link objects from static packages that place_static_packages assigned to the owner's own shared image. When the code collects shared links, however, it still skips every placedInImage package. If such a static package has its own shared dependency, the executable has unresolved symbols.
Reproduction: change E2E 880 so that common depends on support (shared) and dual no longer depends on support directly. Building with --workspace then fails:
bin/dual/dual: undefined reference to `delta@t880_support()'
| Binary | Result |
|---|---|
main 6e3cb974 |
delta, answer, interface_value undefined |
#761 19637260 |
delta undefined |
This is not a regression: #761 fixed two of the three missing symbols. Suggested fix: treat the packages in staticsByImagePackage[owner] like the owner itself in both closure loops, and add this variant to E2E 880.
- Fix the member and artifact paths.
- Extend E2E 880.
4. Consumers link objects of statics placed in another package's image (pre-existing)
In the same variant, client (which depends on dual) also fails on main and with #761. The member and artifact object loops do not skip placedInImage packages that belong to another package's image. client therefore links common's objects directly while also linking libdual_dll. As a result, common's shared dependencies are missing, and the process can end up with two copies of common's state.
- In the member and artifact object loops, skip statics placed in a shared image other than the unit's own.
- Add an E2E with a consumer of a shared package whose static dependency has an external shared dependency.
5. A silently empty glob fingerprint must be reported (#765)
glob_fingerprint returns the empty-set fingerprint in three cases:
- a component of the literal prefix is a symlink;
- a component is
.git; - a component equals this package's output directory name.
The glob then never re-runs the build program, which is the same failure #764 fixed, only with a different trigger. SPEC-004 §3.1 already applies "must be reported, not silently ignored" to unread sections, and the same principle should apply here.
Related problems:
-
Absolute patterns are not validated. They are walked but can never match, because
path_matches_globmatches paths relative to the package root. This was inferred from the code and not executed. -
Excluding this package's output directory name inside foreign trees is a heuristic. A legitimate
../assets/target/**input would be hidden. -
Warn when a declared glob cannot be watched, naming the pattern and the reason.
-
Reject or warn about absolute
rerun_if_changed_globpatterns. -
Apply the output-directory exclusion only to the actual output directory path, not to every directory with that name.
P2: design decisions (discussion first)
6. Precedence of the DLL export sources
On the MSVC ABI, a DLL's export surface now has three possible sources:
- source annotations (
dllexport,/EXPORT:directives, linker-option metadata); - the target's
exportspatterns; - automatic discovery.
Only one rule is defined and implemented: any annotation suppresses automatic discovery for the whole DLL. This matches GNU ld and lld in MinGW mode. It also means one annotated object in a static dependency placed into the image disables automatic exports for the whole DLL. Proposal: state one precedence rule, for example annotations > exports > discovery, in SPEC-004 or docs/04, and implement it. This depends on item 2.
7. Boundary of ../ glob inputs for registry and git dependencies (#765)
Package-relative paths may now leave the package. This is consistent with rerun_if_changed, which goes through abs_against, and with Cargo's rerun-if-changed. In a registry or git dependency, however, ../ resolves into the local package store, so the fingerprint depends on which other packages are installed and is not reproducible.
Proposal: ../ inputs are meaningful for path dependencies and workspace members. mcpp pack and publish warn about, or refuse, globs that escape the package. Docs/30 should also advise keeping the literal prefix specific, because ../../** walks a large tree on every fast-path check.
8. LLVM IR text as an interface (#763)
Export intent in bitcode is detected by parsing textual IR from clang -S -emit-llvm. The textual IR format is not a stable interface. If it changes, the failure mode is to export more than intended: annotations are no longer recognised, and automatic discovery runs. Proposal: E2E 881 is a required gate whenever the Windows LLVM pin changes. Record this in the toolchain-maintenance spec.
P3: performance and documentation
-
coff-defcost with LTO (fix: support LLVM LTO DLL exports and optional automatic discovery #763). It runs oneclang -S -emit-llvmand onellvm-nmper bitcode object, serially, on every relink.llvm-nmaccepts many files in one call, and the intent checks could run in parallel. - An executable cannot link its own package's shared library (fix: keep multi-target executables independent of sibling shared targets #761 semantics). This follows docs/04's compile-once model ("linked into every target") and matches Cargo, where a crate's bin links the crate's rlib, not its cdylib. Document the supported shape for a host and its own DLL, for example LLVM's
LLVM_LINK_LLVM_DYLIB, Chrome's exe + dll, or a plugin host: split them into workspace members, one with the shared library and one with the executable that depends on it. Note that a program that also loads a plugin built against its own DLL holds two copies of that DLL's state. - PE executables export annotated symbols (to verify). Because objects are compiled once, the objects an executable links directly may carry
dllexport. The EXE then gets an export table, and the linker writes<exe>.lib/<exe>.expintobin/. Root packages already behave this way. Verify, then decide whethermcpp packshould drop these files or whether the behavior should be documented. - SPEC-004 dangling reference. §1 and §6 cite "docs/05 Appendix A (Schema Ownership Principle)", which no longer exists in docs/05.
Infrastructure
- E2E 721 hangs on macOS.
721_an_android_row_links_on_a_macos_host.shhung onmacos-e2e 1/2until the 20-minute step timeout (fix: support LLVM LTO DLL exports and optional automatic discovery #763's run 37238977683), and the following artifact upload timed out. The "every e2e test runs somewhere" job then reported266and94as uncovered. Give the test its own timeout, or investigate the NDK provisioning step it waits on. - Shared fixture changes. fix: keep multi-target executables independent of sibling shared targets #761, fix: support LLVM LTO DLL exports and optional automatic discovery #763 and fix: watch parent-directory glob inputs in build programs #765 each carried the same CI-fixture change plus an unrelated README badge move. Next time, land shared fixture changes as their own PR first.
Activity
2026.10.5.1: mcpp run hands the terminal to the program, and the foll…