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

Deprecate setup.py install fallback when wheel package is absent #8560

Closed
wants to merge 5 commits into from
Closed

Deprecate setup.py install fallback when wheel package is absent #8560

wants to merge 5 commits into from

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Jul 8, 2020 •
edited
Loading

Towards #8559

TODO

  • review tests that check the absence of setup.py install in output
  • do we need to patch distlib or drop that test that checks a script can have : in its name?
  • search for egg-info in tests and verify all occurences are pertinent
  • tests that do virtualenv.clear() need to keep wheel installed in the cleared virtualenv (should fix test_env_vars_override_config_file, test_config_file_override_stack)

@sbidoul sbidoul force-pushed the deprecate-setup-py-install-when-wheel-not-installed branch 4 times, most recently from 7dfe724 to 646db6e Compare July 9, 2020 13:29
@sbidoul sbidoul added C: build logic Stuff related to metadata generation / wheel generation type: deprecation Related to deprecation / removal. labels Jul 9, 2020
@sbidoul
Copy link
Member Author

sbidoul commented Jul 9, 2020

This turns out to have a big impact on the test suite, because most tests that do a pip install do not have wheel installed and therefore raise the new warning. Moreover many tests check that the installation succeeded by verifying the presence of a .egg-info directory. I opted to installing wheel by default and check for .dist-info.

There is still work to do (see checklist in issue description), but comments are already most welcome.

@pradyunsg
Copy link
Member

a big impact on the test suite

Would it be worthwhile to perform some house keeping on the test suite before moving further with this PR?

@sbidoul
Copy link
Member Author

sbidoul commented Jul 15, 2020

Anything specific you have in mind @pradyunsg ? The big housekeeping I see necessary is using the "normal" path by default which means having wheel installed by default in the script fixture (or adding with_wheel everywhere) and therefore relying on .dist-info instead of .egg-info, which is basically the meat of this PR :)

I tried to create focused commits as much as I could. Maybe we should merge them progressively in separate PRs...

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Jul 18, 2020
@pradyunsg
Copy link
Member

Anything specific you have in mind @pradyunsg?

Nope.

I took your comment to mean you were hitting some kind of issue w/ the test "infrastructure" (basically stuff in tests/lib), but looks like it was more a case of the tests being hard-coded to look for the wrong thing instead. :)

@sbidoul sbidoul mentioned this pull request Jul 25, 2020
Merged
@sbidoul sbidoul force-pushed the deprecate-setup-py-install-when-wheel-not-installed branch from 646db6e to 31d7bb8 Compare July 25, 2020 11:25
sbidoul added 5 commits July 25, 2020 16:13
@sbidoul
Deprecate setup.py install fallback when wheel package is absent
d18b766
@sbidoul
Test with brokenwheel instead of broken setup.py install
4595716 
Since the 'script' fixture will not lead to "setup.py install" anymore,
the broken-0.2broken package
that is designed to fail on "setup.py install" does
not fail installation anymore.
So we use a broken wheel instead.
@sbidoul
Fix invalid console_scripts entry points in tests?
d1e92fd
@sbidoul
Patch distlib to accept colons in script names
6c09078
@sbidoul
Do not depend on "setup.py install" message in install output
2dfd06f
@sbidoul sbidoul force-pushed the deprecate-setup-py-install-when-wheel-not-installed branch from 31d7bb8 to 24a6fa9 Compare July 25, 2020 14:13
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jul 25, 2020
@sbidoul sbidoul force-pushed the deprecate-setup-py-install-when-wheel-not-installed branch from 24a6fa9 to 2dfd06f Compare July 25, 2020 14:14
@uranusjr uranusjr mentioned this pull request Nov 30, 2020
Open
@sbidoul sbidoul mentioned this pull request Jan 24, 2021
Closed
@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 20, 2021
@pradyunsg
Copy link
Member

Closing this oit, since this is significantly out of date. We can come back around to this, once we have the energy to tackle #8559. :)

@pradyunsg pradyunsg closed this Sep 18, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pradyunsg pradyunsg pradyunsg left review comments

Assignees
No one assigned
Labels
C: build logic Stuff related to metadata generation / wheel generation needs rebase or merge PR has conflicts with current master type: deprecation Related to deprecation / removal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sbidoul @pradyunsg @BrownTruck @pypa-bot