Log a diagnostic when a filepath approaches or exceeds its platform's practical length limit - #1207
Merged
mkarlesky merged 4 commits intoAug 14, 2026
Conversation
… practical length limit Deep project subdirectories (especially under CI) can silently exceed Windows' 260-char MAX_PATH and similar OS/libc ceilings, producing mysterious build failures far from their root cause. FileWrapper now checks path length on every mutating file operation, and FilePathUtils checks the two shared builders nearly all generated build paths funnel through, logging a WARNING as a path approaches the limit and an ERROR once it reaches or exceeds it -- diagnostic only, never blocking the operation itself. Making this check genuinely singular (rather than partial) required routing the six remaining raw File.open/File.read/File.write call sites through FileWrapper, and moving Loginator off FileWrapper (onto raw File I/O for its own logfile) to break the circular dependency that introduced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nstructor args spec_system_helper.rb required ceedling/yaml_wrapper before spec_helper (which loads the 'constructor' gem), so YamlWrapper's new `constructor :file_wrapper` call blew up at load time -- reordered the requires. That in turn surfaced every YamlWrapper.new/FileWrapper.new call site across the system-spec support files that predates this feature and never supplied the now-required dependencies; updated each to match the pattern already established in dependency_tracker_system_helper.rb (real collaborators where behavior is actually exercised, null objects otherwise). SystemContext is a plain standalone class rather than an RSpec-included module, so `double` isn't reachable from its instance methods -- used a trivial NullLoginator and a real Verbosinator there instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
path_of_length(n) assumed File.expand_path('/some/path').length == the raw
string's own length, true on POSIX but not on Windows, where expand_path
prepends a drive letter (e.g. "/aaa" -> "C:/aaa") -- two extra characters
that pushed the 95%-threshold boundary test across the line on Windows CI.
Size the padding off File.expand_path('/')'s own actual length instead of
assuming it's 1, so the helper produces the intended expanded length on
any platform.
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
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.
Summary
FileWrappernow checks path length on every mutating file operation (mkdir,mkdir_tmp,cp,write,write_blank_file, write-modeopen), logging a WARNING as a path approaches its platform's practical length ceiling and an ERROR once it reaches or exceeds it. Diagnostic only — never blocks the operation itself.FilePathUtils#form_named_path/#form_build_context_path, the two shared builders nearly all generated build paths funnel through, get the same check.SystemWrapper.macos?, mirroring the existingwindows?pattern, so the platform-specific limit (Windows 260 / macOS 1024 / Linux 4096) can be selected.File.open/File.read/File.writecall sites (ErbWrapper,YamlWrapper,PreprocessinatorLineMarkerIncludesExtractor,PreprocessinatorCommentStripper,PreprocessinatorCodeFinder,CExtractor) throughFileWrapperso the check is genuinely singular rather than partial.Loginator↔FileWrappercircular dependency by movingLoginator's own logfile write onto rawFileI/O (it's the one place in the codebase that can't route throughFileWrapper, sinceFileWrapper's own diagnostics log throughLoginator).Test plan
FileWrapper#check_path_length(threshold math, origin-prefix gating, platform-limit selection) andSystemWrapper.macos?--verbosity=debug(origin prefix present only at DEBUG), normal-length paths produce no false positives, and the build/test run completes and passes throughout