Fix .juliabundleignore: directory patterns, comments, and content exclusion#137
Open
xtalax wants to merge 1 commit intoJuliaComputing:mainfrom
Open
Fix .juliabundleignore: directory patterns, comments, and content exclusion#137xtalax wants to merge 1 commit intoJuliaComputing:mainfrom
xtalax wants to merge 1 commit intoJuliaComputing:mainfrom
Conversation
Author
|
This is something I came up against a few months ago, claude was able to fix. The problem was failing to exclude items inside of directories as well as directories. |
…lusion
Three bugs in the bundleignore parser:
1. Directory patterns (e.g. "output-data/") were compiled into
Glob.FilenameMatch which only matched the literal string, not files
inside the directory. A pattern "foo/" would exclude the entry "foo/"
but NOT "foo/bar.jl".
Fix: plain directory prefixes (no glob chars) are matched via
startswith(). Glob-containing directory patterns (e.g. "*/bar/")
emit both the dir match and a "*/bar/*" contents match.
2. Comment lines (starting with #) and blank lines were compiled into
Glob.FilenameMatch patterns, potentially causing spurious matches.
Fix: _parse_bundleignore_line() skips comments and blanks.
3. A file named "foo.csv" could be incorrectly excluded by a directory
pattern "foo.csv/" because joinpath("foo.csv", "") == "foo.csv/".
Fix: the rpath_dir startswith check is only applied when the path
is actually a directory (isdir guard).
Updates existing tests to reflect the corrected behavior: files inside
directories matched by "*/bar/" patterns are now properly excluded.
Relates to JuliaComputing#130 (negated paths — this fix is a prerequisite, cleaning
up the same code path).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d735e9f to
a44c029
Compare
Member
|
Do you have a reproducer for the original issue? We have test coverage for this and explicitly check that files in excluded directories are not present in the bundle: JuliaHub.jl/test/packagebundler.jl Lines 414 to 437 in dabdf2b |
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.
Summary
output-data/) now correctly exclude all files inside the directory, not just the directory entry itself. Previously,Glob.FilenameMatch("output-data/")only matched the literal string and files inside were still bundled.# ...) and blank lines in.juliabundleignoreare now properly skipped instead of being compiled into glob patterns.foo.csv/no longer incorrectly excludes a file namedfoo.csv(only matches if it's actually a directory).Root cause
get_bundleignorepassed every line throughGlob.FilenameMatch()without filtering.Glob.FilenameMatch("output-data/")matches the literal string"output-data/"but NOT"output-data/foo.jld2"— so files inside ignored directories were silently bundled.Fix
New
_parse_bundleignore_line()classifies each line:nothingfor comments/blanks (skipped)Stringfor plain directory prefixes → matched viastartswithGlob.FilenameMatchfor glob patterns (unchanged behavior)*/bar/) emit both the dir match and apattern*contents matchTest plan
path_filterertests updated (removed "known limitation" comments — the limitation is now fixed)fixtures/bundle1integration tests all pass (22/22 including directory exclusion)bundle.*test sets pass (0 failures)Relates to #130 (negated paths — this PR cleans up the same code path and is a prerequisite).
🤖 Generated with Claude Code