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.

Sign up for GitHub

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

Jump to bottom

Warn when pip is not building a wheel due to lack of wheel package #7768

Merged
merged 5 commits into from
Feb 23, 2020
Merged

Warn when pip is not building a wheel due to lack of wheel package #7768

merged 5 commits into from
Feb 23, 2020

Conversation

anudit
Copy link
Contributor

@anudit anudit commented Feb 22, 2020

Add a logger.info to log a message about why pip is not building a wheel. In reference to #7709

@anudit
Copy link
Contributor Author

anudit commented Feb 22, 2020

Getting an issue in python2.7 with cryptography for the tox unit tests.
https://dev.azure.com/pypa/pip/_build/results?buildId=19206&view=logs&j=3648b180-0078-523b-656c-6a6149186a03&t=15f4043e-683d-5c16-df12-a78a7649deee&l=49

@chrahunt chrahunt mentioned this pull request Feb 22, 2020
Closed
@anudit
Copy link
Contributor Author

anudit commented Feb 23, 2020

All checks passing now.

@anudit anudit requested review from sbidoul and chrahunt and removed request for sbidoul February 23, 2020 03:00
@uranusjr
Copy link
Member

A couple of thoughts. Both are minor and I can live with it being merged without change though:

  • This could use a news entry, since users can see the addition.
  • I would say something like Could not build wheels for XXX since package wheel is not installed. The word “legacy” is’t particularly relevent to the user, and the word “wheel” appearing twice in the sentence might confuse the user. Also “skipping” sounds like we’re not installing the package at all (but in fact we are, only with a different method).

@anudit anudit requested a review from uranusjr February 23, 2020 08:47
pradyunsg
pradyunsg approved these changes Feb 23, 2020
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit-pick wrt the NEWS fragment. LGTM otherwise!

news/7768.feature Outdated Show resolved Hide resolved
@xavfernandez xavfernandez merged commit e6ed38d into pypa:master Feb 23, 2020
@xavfernandez
Copy link
Member

Thanks @anuditnagar 👍

@pradyunsg pradyunsg mentioned this pull request Feb 25, 2020
Closed
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Mar 27, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 27, 2020
@pradyunsg pradyunsg changed the title Add Logger Alert Warn when pip is not building a wheel for installation Apr 21, 2020
@pradyunsg pradyunsg changed the title Warn when pip is not building a wheel for installation Warn when pip is not building a wheel due to lack of wheel package Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sbidoul sbidoul sbidoul left review comments

@uranusjr uranusjr uranusjr approved these changes

@xavfernandez xavfernandez xavfernandez approved these changes

@pradyunsg pradyunsg pradyunsg approved these changes

@chrahunt chrahunt Awaiting requested review from chrahunt

Assignees
No one assigned
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@anudit @uranusjr @xavfernandez @sbidoul @pradyunsg