Skip to content

Fix symlink prefixes for multiple globstar captures - #239

Merged
facelessuser merged 2 commits into
facelessuser:mainfrom
shkyyy18:fix/globstar-symlink-prefix
Oct 1, 2026
Merged

facelessuser merged 2 commits into
facelessuser:mainfrom
shkyyy18:fix/globstar-symlink-prefix

Conversation

@shkyyy18

@shkyyy18 shkyyy18 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Reproduction

With GLOBSTAR | REALPATH, the pattern **/literal/**/file.txt can incorrectly match outer/literal/link/leaf/final/file.txt when link is a symlink and FOLLOW is disabled. The second globstar checks outer/link instead of outer/literal/link. Conversely, a symlink at unrelated outer/link incorrectly rejects the real path.

_fs_match initializes base only for the first nonempty capture, losing literal components between later captures.

Fix

Initialize the filesystem prefix from each capture's actual start offset. No changes to parsing, symlink caching, or FOLLOW behavior.

Regression tests and validation

  • Eight synthetic filesystem cases (two/three globstars, text/bytes paths, actual-path vs unrelated-path symlink responses) all fail before the source fix and pass afterward. Only islink responses are mocked; paths/files and matching are real. Each also checks the FOLLOW control.
  • Added an actual symlink regression to the existing symlink test class. Windows here cannot create symlinks (WinError 1314), so this and existing platform-dependent cases are skipped locally, not claimed as executed.
  • Python 3.12.10 / Windows: baseline 1243 passed, 166 skipped; after 1251 passed, 167 skipped, 98% coverage.
  • python -m mypy: success, 10 source files.
  • python -m ruff check .: passed.
  • Full tox/Python/platform matrix and docs build not run locally. Remote CI subject to upstream approval/results.

Read contributing guidance; checked current main and existing issues/PRs. AI-assisted investigation, implementation and synthetic tests; listed validation was executed locally. No personal data or unrelated refactoring.

@gir-bot gir-bot added S: needs-review Needs to be reviewed and/or approved. C: glob Glob library. C: source Related to source code. C: tests Related to testing. labels Oct 1, 2026

@facelessuser facelessuser left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shkyyy18 This is a good catch. Please see comments and address them.

Comment thread tests/test_globmatch_prefix.py Outdated
Comment thread tests/test_globmatch.py Outdated
@shkyyy18

shkyyy18 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both review comments in fb97480: moved the eight regression cases into tests/test_globmatch.py and backticked the globstar term in the real-symlink test docstring. The source fix is unchanged. Local Python 3.12 verification: 1251 passed, 167 skipped, 98% coverage; Ruff and mypy passed. The local test run uses a workspace-local temporary directory because the default Windows temp directory was inaccessible. I could not run pyspelling locally because hunspell/aspell is unavailable; the updated docstring uses the inline-code exclusion already configured in .pyspelling.yml. The new remote CI run is not yet reported as passed.

@facelessuser

Copy link
Copy Markdown
Owner

@gir-bot lgtm

@gir-bot gir-bot added S: approved The pull request is ready to be merged. and removed S: needs-review Needs to be reviewed and/or approved. labels Oct 1, 2026
@facelessuser
facelessuser merged commit e0f74de into facelessuser:main Oct 1, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C: glob Glob library. C: source Related to source code. C: tests Related to testing. S: approved The pull request is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants