New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix copy ignore_case on Windows #7704
Fix copy ignore_case on Windows #7704
Conversation
conans/client/file_copier.py
Outdated
@@ -153,7 +153,7 @@ def _filter_files(src, pattern, links, excludes, ignore_case, excluded_folders): | |||
filenames = {f.lower(): f for f in filenames} | |||
pattern = pattern.lower() | |||
|
|||
files_to_copy = fnmatch.filter(filenames, pattern) | |||
files_to_copy = [n for n in filenames if fnmatch.fnmatchcase(n, pattern)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is broken, filter
is doing internally some os.path.normcase
, which changes / => \ path in Windows too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am seeing this possibly breaking users, because it is by default ignoring case.
Agree it is a bug, but as most users haven't had to specify it, I would say that the bug is in the default. The default should be ignore_case=True
, and then alter the behavior if it is explicitly changed to ignore_case=False
. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, indeed that's the original behavior, and changing it only after 4 years of project, probably will break users. I'll update this PR to use ignore_case=True
by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About filter
, thanks for checking it, I'll search a better way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@memsharded I've split both comparisons, now for non-case-sensitive, it will keep the old behavior, as fnmatch.filter changes all file names to lower-case in Windows due normcase. For case-sensitive check, I kept same modification, but normalizing the path with normpath.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
44bdc8f
to
b49dd84
Compare
Changelog: Fix:
self.copy()
followsigore_case
correctly on Windows.Docs: conan-io/docs#1862
This PR fixes
self.copy(ignore_case=True/False)
on Windows. The actual behavior, on Windows,ignore_case=False
doesn't work.Fixes #6116
UPDATE:
develop
branch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.