Overeager pip normalization #206
Merged
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
blast-hardcheese
requested review from
masad-frost
and removed request for
a team
ryantm
requested changes
Jan 2, 2024
internal/backends/python/python.go
Outdated
| @@ -368,6 +368,12 @@ func makePythonPipBackend(python string) api.LanguageBackend { | |||
| span, ctx := tracer.StartSpanFromContext(ctx, "pip install") | |||
| defer span.Finish() | |||
|
|
|||
| normalizedPkgs := make(map[api.PkgName]api.PkgName) | |||
| for name := range pkgs { | |||
| // Deleting this denormalizes the output of upm list, fixing Workspace bug, but breaks upm add --lang pip | |||
There was a problem hiding this comment.
fixing Workspace bug
Can we reference or describe the bug so this isn't scary to change later?
There was a problem hiding this comment.
I think that was a WIP comment, it doesn't read well. I'll fix it up, thanks
There was a problem hiding this comment.
Fleshed that out a bit. The underlying issue was Workspace was attempting to install discord.py, which we were normalizing and returning as discord-py, which would then (rightfully) confuse the workspace.
Hopefully the comment makes more sense.
|
python wtf moment |
pip freeze output is actually pip's internal canonicalized form, which can be different from the normalized package name format. Example: `discord.py` gets normalized into `discord-py`, but `pip show discord-py` still outputs `Name: discord.py`.
Names in requirements.txt are the package as tracked in package metadata, not necessarily the normalized name of the package. Example: pip install discord-py will add `discord.py` to the output of `pip freeze`.
blast-hardcheese
force-pushed
the
dstewart/overeager-pip-normalization
branch
from
4ed1ffd
to
8e129bc
Compare
ryantm
approved these changes
Jan 2, 2024
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.
Unfortunately, even normalization rules are not the end of the story when it comes to
pip freeze!pip install discord-pywill still result inpip freeze | grep discord==discord.py, since that's the name tracked in the metadata.Exciting!