Rewrite the IgnoreList logic to be more distutils-like #114
Conversation
This file contains 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
This should eliminate most of the inconsistencies with how distutils treats MANIFEST.in files. (Some inconsistencies remain: distutils allows you to add back previously excluded files.) The way I went about it: - explicitly defined the internal canonical format for lists of file names - changed it to use / instead of os.path.sep - changed it to omit directories and list only files, which makes ignore handling simpler To elaborate on the last point, here are some examples: 1. 'exclude foo' in MANIFEST.in works only on regular files, not directories, but how can we know if all we have is a string? 2. if you exclude all the files in a directory, distutils will omit that directory from the sdist, but it's difficult to do the same when all you have is a list of strings Next, I changed IgnoreList to avoid fnmatch and be entirely based on regular expressions, and this time I'm using distutils's own translate_pattern() to produce exactly the same regular expressions that distutils itself will use for matching. This may introduce some surprising behaviour, like 'global-exclude a*.txt' now matches on suffixes of file names, so it would exclude files named bar.txt! See https://bugs.python.org/issue14106. Then tests started failing on Python 2.7, but all I get from the __repr__ is a bunch of opaque <_sre.SRE_Pattern object at 0xdeadbeef>, which are impossible to debug. Therefore I'm declaring Python 2.7 support officially dropped! check-manifests's own configuration (--ignore or [check-manifest] ignore=) is now treated as a global-exclude statement in MANIFEST.in, which means: - it's matched anywhere in the file tree - it's matched against a filename suffix due to the above-mentioned Python bug - it's ignored if it matches a directory You can ignore directories only by ignoring every file inside it. There's no way currently of specifying that that works for arbitrarily nested trees, but you can try --ignore=dir/*,dir/*/*,dir/*/*/* until you get the result you want. This decision is not cast in stone: I may in the future change the handling of --ignore to match files and directories, because there's no reason it has to be distutils-compatible. Expect regressions on Windows. I'll fix any that I find.
distutils's translate_pattern() expects the paths to use os.path.sep, not slashes.
mgedmin
force-pushed
the
ignore-logic-rewrite
branch
from
f7c67c9
to
d5f6c72
Compare
Do not call normpath on test data: now our canonical form is using /s. Remove useless isdir() check in find_suggestions, now that our canonical filelists contain only files and not directories.
mgedmin
force-pushed
the
ignore-logic-rewrite
branch
from
ff3bd4e
to
6e14e93
Compare
|
Nice! Thanks a lot :-) |
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.
This should eliminate most of the inconsistencies with how distutils treats MANIFEST.in files. (Some inconsistencies remain: distutils allows you to add back previously excluded files.)
The way I went about it:
To elaborate on the last point, here are some examples:
Next, I changed IgnoreList to avoid fnmatch and be entirely based on regular expressions, and this time I'm using distutils's own translate_pattern() to produce exactly the same regular expressions that distutils itself will use for matching.
This may introduce some surprising behaviour, like 'global-exclude a*.txt' now matches on suffixes of file names, so it would exclude files named bar.txt! See https://bugs.python.org/issue14106.
Then tests started failing on Python 2.7, but all I get from the
__repr__is a bunch of opaque<_sre.SRE_Pattern object at 0xdeadbeef>, which are impossible to debug. Therefore I'm declaring Python 2.7 support officially dropped!check-manifests's own configuration (--ignore or [check-manifest] ignore=) is now treated as a global-exclude statement in MANIFEST.in, which means:
You can ignore directories only by ignoring every file inside it. There's no way currently of specifying that that works for arbitrarily nested trees, but you can try
--ignore=dir/*,dir/*/*,dir/*/*/*until you get the result you want.This decision is not cast in stone: I may in the future change the handling of --ignore to match files and directories, because there's no reason it has to be distutils-compatible.
Expect regressions on Windows. I'll fix any that I find.
Closes #98, #113.