Skip to content
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

Don't normalize name when writing Pipfile #3338

Merged
merged 4 commits into from
Dec 2, 2018
Merged

Don't normalize name when writing Pipfile #3338

merged 4 commits into from
Dec 2, 2018

Conversation

frostming
Copy link
Contributor

@frostming frostming commented Dec 1, 2018

Signed-off-by: Frost Ming mianghong@gmail.com

The fix

Don't canonicalize the name user passes in. Just make sure the name is safe. With that said, a package with dots in its name like fsc.export won't be normalized to fsc-export in Pipfile

Closes #3324

The impact

This PR doesn't fix the wrong package name which already exists in user's Pipfile. Taking the following examples:

Item Content
Original name requests-html
Canonical name(PEP-503) requests-html
User command $ pipenv install Requests_HTML
Entry in Pipfile requests-html = "*"
Item Content
Original name fsc.export
Canonical name(PEP-503) fsc-export
User command $ pipenv install Fsc.Export
Entry in Pipfile "fsc.export" = "*"
User command $ pipenv install fsc-export
Entry in Pipfile Installation fails, nothing is added

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix, .feature, .behavior, .doc. .vendor. or .trivial (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

Signed-off-by: Frost Ming <mianghong@gmail.com>
Signed-off-by: Frost Ming <mianghong@gmail.com>
Signed-off-by: Frost Ming <mianghong@gmail.com>
@techalchemy
Copy link
Member

Approach gets a +1 from me

@techalchemy techalchemy added the PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. label Dec 1, 2018
@frostming frostming merged commit 613d99b into master Dec 2, 2018
@frostming frostming deleted the bugfix/3324 branch December 2, 2018 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

package in a namespace from pip fails to lock
2 participants