libs/fileset: cover leading-slash anchoring in glob patterns - #6345
Open
TangoEnSkai wants to merge 1 commit into
Open
libs/fileset: cover leading-slash anchoring in glob patterns#6345TangoEnSkai wants to merge 1 commit into
TangoEnSkai wants to merge 1 commit into
Conversation
NewGlobSet compiles include patterns with go-gitignore, so a leading slash anchors the pattern to the fileset root while a bare name matches at any depth. Nothing tested that distinction, even though sync.include depends on it to select a top-level directory without also matching same-named directories nested deeper. Add a test that pins both halves: "/dir/" selects only the root directory, "dir/" selects the nested one as well.
Contributor
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
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.
Changes
Adds a test for how
NewGlobSettreats a leading slash in an include pattern.NewGlobSetcompiles patterns withgo-gitignore, so gitignore anchoringrules apply:
/dir/matches only adirat the fileset root, whiledir/matches a directory of that name at any depth.
glob_test.gocoveredrecursive patterns, directory patterns and quoted file patterns, but nothing
pinned that anchored/unanchored distinction.
The new test asserts both halves against a temporary tree holding
dir/andnested/dir/.Why
sync.includepasses user patterns straight through toNewGlobSet(
bundle.GetSyncIncludePatterns→sync.NewFileList→fileset.NewGlobSet),so this anchoring is user-visible bundle behaviour: it is what lets someone
include a top-level directory without also sweeping in same-named directories
nested elsewhere in the tree.
It is currently unprotected. Normalising the pattern slightly differently in
NewGlobSet— for example trimming the leading slash before compiling —silently widens every anchored include, and the existing suite stays green. I
checked that by making exactly that change locally: only the new test fails.
This came up while looking at #3456, which reports anchored
sync.includepatterns matching nested directories. I could not reproduce that against
main— the behaviour is correct today, at the pattern level and end to endthrough
NewGlobSet— so this PR only locks the current behaviour in ratherthan changing anything. I have left a note on the issue with the details.
Tests
go test ./libs/fileset/— all green, including the newTestGlobFilesetLeadingSlashAnchorsToRoot.Test-only change, so no changelog fragment.