Disambiguate colliding user #includes; two related PathMatcher/line-marker fixes - #1282
Merged
mkarlesky merged 2 commits intoSep 11, 2026
Merged
Conversation
Two headers sharing only a basename in different directories (#include "hw/config.h" + #include "app/config.h" in the same file) silently collapsed to one in a reconstructed Partial/mock header, dropping whichever lost -- along with everything it declares. Root cause: Preprocessinator's two bare-includes unions deduped by filename, discarding one of the two genuinely distinct bare entries before Includes.reconcile ever got a chance to keep both distinct (which it already could -- a pre-existing unit spec proves it). Deduping by filepath instead keeps both without losing the union's original purpose (collapsing a literal the gcc pass and the text scan both happen to report identically). That alone still rendered both colliding entries with the SAME basename-only text, so the compiler's own search-path resolution would pick one file for both #include lines regardless. Includes.reconcile now disambiguates: when two or more of a reconciliation's own matched user includes share a basename, each renders with the shortest trailing-path suffix of its own real, resolved filepath that's unique among that colliding group -- computed from the accurate pass's (or fallback's) own resolved locations, never from a bare entry's literal, possibly-unresolved directive text. Growth stops at the first length that disambiguates, so it never reaches further than the colliding files' own real containing directories -- important because Ceedling has no project-root search path, only one per real source/test/ include directory, so a fuller, project-relative spelling can be genuinely unfindable once rendered into a generated file living somewhere else entirely (confirmed the hard way: an earlier attempt at preserving each include's own literal, `..`-resolved spelling verbatim broke exactly this way against a real three-way collision crossing directory trees). A matched user include with no colliding sibling stays exactly as before -- filename-only, since Ceedling's own per-directory search paths already find it unambiguously and preserving more achieves nothing. Two related, narrower fixes surfaced while building comprehensive coverage for this: - PathMatcher.resolve_relative dropped a `..`-resolved path's own leading "/" when its anchor was an absolute Unix path, silently turning it into a different, relative-looking string. - PreprocessinatorLineMarkerIncludesExtractor's own `..`-collapse (resolve_relative with an empty anchor) is a no-op for an absolute marker path, per resolve_relative's own documented contract -- reachable whenever the file being preprocessed itself has an absolute path, leaving a directives-only marker's `..` uncanonicalized and never able to path-correspond against anything downstream. Comprehensive integration coverage (single-directory, multi-level, and `..`-relative collisions, composed together, plus the existing non-colliding cases re-characterized as basename-only) and one new system spec proving the fix reaches a real Partials build: sensor.c #includes three headers that all share the basename config.h from three different directories, two ordinary subdirectories and one reached only via `..` -- any two colliding on a bare "config.h" in the generated file leaves at least one macro undeclared, a guaranteed compile error regardless of which file an ambiguous "config.h" happened to resolve to first. Confirmed failing for that exact reason before this fix, passing after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…xpand_path File.expand_path is CWD/drive-dependent: on Windows it silently prepended the current process's own drive letter to an already-absolute Unix-style marker path (/proj/... -> D:/proj/...) instead of leaving it alone, failing CI on that platform. A plain segment walk collapses the same .. without touching anything platform-dependent, preserving whichever absolute prefix the path already had (a leading "/", or a drive letter for a genuinely Windows path). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
What
Fixes the second issue found investigating #1267 (issue 2 of the follow-on, recorded and now closed out in memory): two headers sharing only a basename in different directories, both
#included from the same file, silently collapsed to one in a reconstructed Partial/mock header — dropping whichever lost, along with everything it declares.→ generated file carried only
#include "config.h"once. Whichever real header the compiler's search paths happened to find first "won"; the other's declarations were simply gone, a silent compile/link failure waiting to happen.Root cause and fix
Preprocessinator#preprocess_file_includes_common's two bare-includes unions deduped by filename, discarding one of the two genuinely distinct bare entries beforeIncludes.reconcileever saw both.Includes.reconcileitself already handles multiple genuinely-distinct same-basename bare entries correctly — a pre-existing unit spec proves it. Deduping by filepath instead keeps both, without losing the union's original purpose (collapsing a literal the gcc pass and the text scan both happen to report identically).That alone still rendered both colliding entries with the same basename-only text — two identical
#includelines resolve to the same file twice, not two different files once each.Includes.reconcilenow disambiguates: when two or more matched user includes in one reconciliation share a basename, each renders with the shortest trailing-path suffix of its own real, resolvedfilepaththat's unique among that colliding group — computed from the accurate pass's (or fallback's) own resolved location, never from a bare entry's literal, possibly-unresolved directive text. A non-colliding match is untouched — still filename-only, since Ceedling's own per-directory search-path convention already finds it unambiguously.A design dead-end worth recording
My first attempt preserved each include's own literal (
..-resolved) spelling verbatim, tagged at the text-scan bare contributor. It broke against a real three-way collision crossing directory trees: Ceedling has no project-root search path, only one per real source/test/include directory — so a fuller, project-relative spelling (src/shared/config.h) can be genuinely unfindable once rendered into a generated file living somewhere else entirely, even though it's the literally correct real path. The system spec below caught this directly (fatal error: src/shared/config.h: No such file or directory) before it shipped. The shortest-disambiguating-suffix approach never reaches further than the colliding files' own real containing directories, so it can't make this mistake.Two related, narrower fixes surfaced while building coverage
PathMatcher.resolve_relativedropped a..-resolved path's own leading/when its anchor was an absolute Unix path, silently turning it into a different, relative-looking string.PreprocessinatorLineMarkerIncludesExtractor's own..-collapse is a no-op for an absolute marker path, perresolve_relative's own documented contract — reachable whenever the file being preprocessed itself has an absolute path, leaving a directives-only marker's..uncanonicalized and never able topath_correspond?against anything downstream.Neither is reachable in an ordinary Ceedling project (project-relative paths throughout) — both surfaced through this session's own integration-test harness, which uses absolute
Dir.mktmpdirroots. Fixed anyway: both are real latent bugs in code this fix's own mechanism depends on.Tests (test-first, unit/integration/system)
spec/units/includes/includes_spec.rb— new cases fordisambiguating_user_include_path: two-way and three-way collisions, growing the suffix past one directory level when still ambiguous.spec/units/path_matcher_spec.rb— new case for the absolute-anchor leading-slash fix.spec/units/preprocess/preprocessinator_line_marker_includes_extractor_spec.rb— new case for the absolute-marker-path..-collapse fix.spec/units/preprocess/preprocessinator_spec.rb— new case proving the union keeps two same-basename, different-filepath bare entries distinct.spec/integration/includes_extraction_spec.rb— re-characterizes the non-colliding cases as basename-only (the correct behavior, not a limitation), plus new colliding cases (single-level, multi-level requiring a longer suffix, and one composing a non-colliding and a colliding include in the same file).spec/system/subdir_include_preservation_spec.rb(new) — one real Partials build:sensor.c#includes three headers that all share the basenameconfig.hfrom three different directories (two ordinary subdirectories, one reached only via..). Any two colliding on a bareconfig.hin the generated file leaves at least one macro undeclared — a guaranteed compile error regardless of which file an ambiguousconfig.hresolves to first. Confirmed failing for that exact reason before the fix (verified viagit stashof just the production files), passing after.Verification
rake specs:units3249/0,rake specs:integration34/0 (host + Docker,madsciencelab-plugins:1.1.0, real Linux gcc).conditional_include_macro_visibility,test_runner_system_include,header_include_path_validation,include_path_reconciliation,mocks_partials_configuration_validation) unmodified and green.🤖 Generated with Claude Code