Normalize case before comparing

[pradyunsg]

To the degree that I can figure out, while on a break from doing math problem, this should fix the case problem with #4493.

[pradyunsg]

@pfmoore Could you give this a try and see how this runs?

[pfmoore]

Will do. Might not be till tomorrow...

[pradyunsg]

Cool. :)

No hurries.

[pradyunsg]

Ping! ^.^

[pfmoore]

Yeah, as far as I can tell it fixes the issue (and sorry for the delay!)

[pradyunsg]

fixes the issue

Awesome! ^.^

[anntzer]

A minor annoyance is that paths are now all represented as lowercase on Windows when prompting for removal ("c:/users/...") rather than as their canonical value ("C:/Users/..."). Solutions would include re-canonicalizing the paths (https://stackoverflow.com/questions/3692261/in-python-how-can-i-get-the-correctly-cased-path-for-a-file, https://stackoverflow.com/questions/2113822/python-getting-filename-case-as-stored-in-windows) or switch to pathlib as WindowsPath handles the comparisons fine.

On the other hand it is indeed really only a minor annoyance.

[pfmoore]

@anntzer Good point, I agree that we should be displaying pathnames to the user in the correct case as held on the filesystem. It's a quality of implementation issue, and as you say only a minor cosmetic annoyance, but I'd be happy to review and approve a PR to fix this.

[anntzer]

It's likely not that hard, I think something like

diff --git i/src/pip/_internal/req/req_uninstall.py w/src/pip/_internal/req/req_uninstall.py
index f7ec3a8f..dd65fe43 100644
--- i/src/pip/_internal/req/req_uninstall.py
+++ w/src/pip/_internal/req/req_uninstall.py
@@ -119,6 +119,7 @@ def compress_for_output_listing(paths):
         if path.endswith("__init__.py") or ".dist-info" in path:
             folders.add(os.path.dirname(path))
         files.add(path)
+    _normcased_files = set(map(os.path.normcase, files))
 
     folders = compact(folders)
 
@@ -130,8 +131,9 @@ def compress_for_output_listing(paths):
                 if fname.endswith(".pyc"):
                     continue
 
-                file_ = os.path.normcase(os.path.join(dirpath, fname))
-                if os.path.isfile(file_) and file_ not in files:
+                file_ = os.path.join(dirpath, fname)
+                if (os.path.isfile(file_)
+                        and os.path.normcase(file_) not in _normcased_files):
                     # We are skipping this file. Add it to the set.
                     will_skip.add(file_)
 
diff --git i/tests/unit/test_req_uninstall.py w/tests/unit/test_req_uninstall.py
index dc851796..8f999457 100644
--- i/tests/unit/test_req_uninstall.py
+++ w/tests/unit/test_req_uninstall.py
@@ -50,9 +50,9 @@ def test_compressed_listing(tmpdir):
     def in_tmpdir(paths):
         li = []
         for path in paths:
-            li.append(str(os.path.normcase(
-                os.path.join(tmpdir, path.replace("/", os.path.sep))
-            )))
+            li.append(
+                str(os.path.join(tmpdir, path.replace("/", os.path.sep)))
+            )
         return li
 
     sample = in_tmpdir([

will work? (The diff on the test is just reverting this PR.)

Feel free to adopt the patch as a PR.

[pfmoore]

OK, I made #5642 based on your code. Thanks!

One thing I wonder about - the original code here does normcase() on file_, but doesn't actually seem to normcase the original files list! So the original may have actually been wrong. Tracking back from the original comment from @pradyunsg on this PR, I can't actually work out what "the case problem with #4493" was, so I don't really know how to confirm that suspicion.

Your version avoids that problem, though, so I think it's better in any case.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.