fix: keep the case when getting the executable #2528
Conversation
|
If I remember correctly we did this in purpose. On windows the casing doesnt matter and we ran into issues were the specified name did not match the actual file. Perhaps we should just limit normalizing the casing on oses that are case insensitive (mac and windows)? |
| tracing::debug!("Could not find 'PATHEXT' variable, using a default list"); | ||
| [ | ||
| ".COM", ".EXE", ".BAT", ".CMD", ".VBS", ".VBE", ".JS", ".JSE", ".WSF", ".WSH", ".MSC", | ||
| ".CPL", | ||
| ".com", ".exe", ".bat", ".cmd", ".vbs", ".vbe", ".js", ".jse", ".wsf", ".wsh", ".msc", |
There was a problem hiding this comment.
The PATHEXT variable also uses uppercase so I would keep it that way.
There was a problem hiding this comment.
I removed it because the strings were followed by a to_lowercase, but sure, I can keep it.
There was a problem hiding this comment.
Since this keeps current behavior, let's leave it as is
Sounds like a good idea to me. Ideally we would normalize depending on the filesystem, not OS. But for now I think switching behaviour depending on OS is fine. |
|
@baszalmstra I can't come up with a scenario where this would fail. The original .exe files would have had the same casing anyways. |
|
I highly doubt it was intentional. This is the original PR introducing this: https://github.com/prefix-dev/pixi/pull/2136/files Also note: there is no test that fails now. Let's just keep the original casing. |
For comparison purposes we were always lower-casing the executable name. However, we also returned the lowercase version and exposed executables as lowercase which doesn't match user expectations.
With this PR we are instead returning the original case.
I cannot judge if this affects current exposed binaries. However, I assume we would just keep the lower-case exposed binary as written in the global manifest and only change newly installed tools.
fixes #2478