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

Remove --no-binary disabling wheel cache #11872

Merged
merged 3 commits into from
Mar 31, 2023
Merged

Remove --no-binary disabling wheel cache #11872

merged 3 commits into from
Mar 31, 2023

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Mar 18, 2023 •
edited
Loading

closes #11453
closes #9162

@sbidoul sbidoul added this to the 23.1 milestone Mar 18, 2023
@sbidoul sbidoul changed the title Remmove --no-binary disabling wheel cache sbi Remove --no-binary disabling wheel cache sbi Mar 18, 2023
@sbidoul sbidoul changed the title Remove --no-binary disabling wheel cache sbi Remove --no-binary disabling wheel cache Mar 18, 2023
@sbidoul sbidoul force-pushed the rm-no-binary-disables-wheel-cache-sbi branch 2 times, most recently from a1ab613 to a266f3d Compare March 18, 2023 12:13
@sbidoul sbidoul added project: setuptools Related to setuptools type: deprecation Related to deprecation / removal. labels Mar 21, 2023
@sbidoul sbidoul mentioned this pull request Mar 25, 2023
Merged
1 task
@sbidoul sbidoul force-pushed the rm-no-binary-disables-wheel-cache-sbi branch from a266f3d to 41f9bc9 Compare March 27, 2023 13:50
@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Mar 27, 2023
@sbidoul sbidoul force-pushed the rm-no-binary-disables-wheel-cache-sbi branch from 41f9bc9 to c6d74cb Compare March 27, 2023 14:54
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Mar 27, 2023
pradyunsg
pradyunsg reviewed Mar 27, 2023
View reviewed changes
src/pip/_internal/cli/cmdoptions.py Outdated
@@ -1004,7 +1004,7 @@ def check_list_path_option(options: Values) -> None:
choices=[
"fast-deps",
"truststore",
"no-binary-enable-wheel-cache",
"no-binary-enable-wheel-cache", # now always on
Copy link
Member

Choose a reason for hiding this comment

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

Why not remove this, as we've done with other --use-feature flags?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kind of remember folks complaining that we remove feature flags that became always on because it was causing unnecessary churn and I was kind of agreeing. But yeah if we keep it we probably need some warning that it's going away one day. Not sure what's best.

Copy link
Member

@pradyunsg pradyunsg Mar 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Let's leave it in with a warning -- it's less churn for end users. However, I do feel like we should print these "ignored use-feature" warnings in a single central location, instead of scattering them across the commands. Do that seem reasonable?

If so, the only place that I can think of at the moment is Command._main, and... well, I trust your judgement on determining a good place. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@pradyunsg I have centralized the handling of always enabled features, so it will be easy to use for future similar cases.
I do that immediately after setting up the logger so we get the colored warning.

pradyunsg
pradyunsg approved these changes Mar 27, 2023
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.

LGTM, except for one minor concern.

@sbidoul sbidoul force-pushed the rm-no-binary-disables-wheel-cache-sbi branch from c6d74cb to 2617ccd Compare March 29, 2023 06:41
@sbidoul sbidoul merged commit fded808 into pypa:main Mar 31, 2023
@sbidoul sbidoul deleted the rm-no-binary-disables-wheel-cache-sbi branch March 31, 2023 07:44
@atugushev atugushev mentioned this pull request Apr 1, 2023
Merged
4 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pradyunsg pradyunsg pradyunsg approved these changes

Assignees
No one assigned
Labels
project: setuptools Related to setuptools type: deprecation Related to deprecation / removal.
Projects
None yet
Milestone
23.1
Development

Successfully merging this pull request may close these issues.

Deprecate --no-binary disabling the wheel cache Using --no-binary disables reuse of locally compiled wheels
3 participants
@sbidoul @pradyunsg @pypa-bot