Display windows path in its original case
Fixes #6823
Another deeper problem is that abspath() is not semantically the same as resolve()—the latter follows symbolic links. So we need to not use resolve, and review callers of this function to ensure they are passing in a correct value.
Another deeper problem is that
abspath()is not semantically the same asresolve()—the latter follows symbolic links. So we need to not useresolve, and review callers of this function to ensure they are passing in a correct value.
yep it does follow the symlinks but it also requires the path to exist tho, so i dont think that might be a issue?
If the path doesn’t exist and strict is True, FileNotFoundError is raised.
The issue is that after following symlinks path may no longer start with os.getcwd(), making the check return a different result.
so should we take this approach instead?:
if WINDOWS:
path = os.path._getfinalpathname(path).lstrip(r"\\?")
else:
path = os.path.normcase(os.path.abspath(path))
if path.startswith(os.getcwd() + os.path.sep):
path = "." + path[len(os.getcwd()) :]
return path
I definitely prefer to not use private functions.
Agreed. This seems like way too much effort for what is essentially a cosmetic issue, and it's going to result in a maintenance burden because future maintainers won't know why we've chosen to use such a complex and fragile approach.
Can I suggest that we take a step back and understand what we're trying to achieve with display_path? As far as I can see, all we're trying to do is to express the path as relative to the CWD, if we can. There's a standard function for that in pathlib:
original = Path(path)
try:
# Let's see if we can describe that relative to the CWD
simplified = original.relative_to(Path.getcwd())
except ValueError:
# Nope, so we assume what we have is good enough
return path
return str(simplified)
If that doesn't do what we want, then we can look at when it fails, and understand what we want in that case and extend the code appropriately. Ideally with comments explaining why we need each additional bit of complexity. But honestly, I wouldn't bother - I'd reframe display_path as doing precisely what relative_to does, on the basis that pathlib is the reference implementation on "how to manipulate paths in a cross-platform way".
return str(simplified)
That should be os.fsdecode instead of str but... same idea. :)
@pfmoore if you look at the original issue, we gotta use resolve() with pathlib in that case, but i dont think thats the way pip should be due to reasons previously mentioned by @uranusjr , so i wonder if there is a way to do it without resolve() from pathlib or private function from os?
Sorry, I don't see why we have to use resolve(). Can you be more specific about where that's necessary? If the problem is that display_path is being called with a value that's not in the original case, then the fix is to work out when we lost the original case, and fix the problem there, not try to recover the original case later after it's already been lost...
This is what @uranusjr said here, though, so I'm confused why you are still insisting we need to use resolve().
gotcha
Sorry, I don't see why we have to use
resolve(). Can you be more specific about where that's necessary? If the problem is thatdisplay_pathis being called with a value that's not in the original case, then the fix is to work out when we lost the original case, and fix the problem there, not try to recover the original case later after it's already been lost...This is what @uranusjr said here, though, so I'm confused why you are still insisting we need to use
resolve().
just to confirm , may i know how i might recover the original case of the path, from where it begins? like the path by default in python is lowercase, so any nudges on this would be helpful, for example, say this is the part from where we get the path of the dependecy:
@property
def location(self) -> Optional[str]:
return self._dist.location
@property
def installed_location(self) -> Optional[str]:
egg_link = egg_link_path_from_location(self.raw_name)
if egg_link:
location = egg_link
elif self.location:
location = self.location
else:
return None
return normalize_path(location)
so what should i do to retrieve the original casing, as u can see the path sensitivity is not getting "lost", its just lowercase by default
If you want to retain the casing, you shouldn't use normalize_path as that deliberately calls os.path.normcase.
And yes, I know that changing installed_location to return paths whose case is not normalised will cause other problems, as code will have to check properly[^1] whether paths match rather than just doing a string comparison. I'm not suggesting that it's easier to maintain the original case throughout - far from it, it's likely a lot of work. But I am saying that I think that if we want to fix this issue, we should do so properly, and not start introducing complex hacks involving internal functions just to recover the original form after we threw it away earlier.
I'll also point out that code like if path.startswith(os.getcwd() + os.path.sep) is exactly the sort of naive string comparison that results in paths getting normalised in the first place, to "make it work". Which is why I think it's far better to use path-specific methods like relative_to(), which provide the correct semantics directly.
And finally, I'll point out that if you think this is all too much work for a small cosmetic issue, then I agree with you. I'd frankly just leave the display using normalised case and not worry about it[^2].
[^1]: By which I mean taking case sensitivity into account [^2]: Hoping that at some point, someone will be interested enough to convert pip to use pathlib everywhere, at which point this issue will get resolved naturally.
makes sense, ima see if i can convert all of them to pathlib, in case i have some free time
closing it, since i am having a hard time making time for this migration, hope someone else will convert it someday :"c