fix #457: Make --exclude option follow symlinked directories
#458
Conversation
jeancochrane
changed the title
exclude option follow symlinks--exclude option follow symlinks
jeancochrane
changed the title
--exclude option follow symlinks--exclude option follow symlinked directories
| except IndexError: | ||
| # for some reason Path.glob(".") raises an index error, | ||
| # although glob.glob(".") returns ["."] | ||
| pass |
There was a problem hiding this comment.
I wonder if we can remove this error handling block given that we're no longer calling Path.glob() at all?
| # latter would be more concise, since Path.glob() does not | ||
| # resolve symlinks properly in Python <3.13 | ||
| exclude_set.update( | ||
| [Path(g) for g in glob(str(exclude_root / s), recursive=True)] |
There was a problem hiding this comment.
It feels a bit hacky to cast the Path object to a string here, but it's the only way to get it to work with glob.glob(), which expects a string. It's probably fine, and hopefully the tests would tell us if it wasn't, but part of me is a bit nervous about the platform compatibility of this change -- It works fine on my Ubuntu machine, but I can't help being nervous that e.g. Windows may handle this string conversion incorrectly.
There was a problem hiding this comment.
Should be safe to do it this way, but thanks for calling out this change!
| # latter would be more concise, since Path.glob() does not | ||
| # resolve symlinks properly in Python <3.13 | ||
| exclude_set.update( | ||
| [Path(g) for g in glob(str(exclude_root / s), recursive=True)] |
There was a problem hiding this comment.
Should be safe to do it this way, but thanks for calling out this change!
jeancochrane
deleted the
fix/#457/exclude-option-does-not-follow-symlinks
branch
This PR updates the globbing behavior for the
--excludeoption inapi.get_matching_paths()to rely only onglob.glob()instead ofPath.glob(), the latter of which contains a bug on Python <3.13 such that it does not always follow symlinked directories correctly.