Deprecate --allow-unsafe CLI option for eventual removal #1247
Conversation
| @@ -302,6 +304,17 @@ def cli( | |||
| ) | |||
| emit_index_url = index | |||
|
|
|||
| if allow_unsafe is None: | |||
There was a problem hiding this comment.
This warning might affect most of the users and force them to use --no-allow-unsafe, which could annoy them. Would it make more sense to change the default value in the next major release instead (as we did it with --build-isolation in 5.0.0)?
There was a problem hiding this comment.
The goal is to nudge users towards --allow-unsafe, not --no-allow-unsafe. That way, they become early adopters of the behavior planned for 6.0.0 in #1249.
If you prefer to wait for a major release, can we jump straight to #1249 and drop this PR? We'd be relying on the release notes rather than using a warning.
There was a problem hiding this comment.
I'd prefer to add --no-allow-unsafe option and set it on by default. I'm not sure about removing --allow-unsafe/--no-allow-unsafe options completely.
There was a problem hiding this comment.
The comment in #989 suggests differently:
I'm starting to think it might make sense to deprecate --allow-unsafe and make it the default behavior, but that would be as a separate issue/PR for discussion.
To me, the suggestion to deprecate it also implies eventual removal. Deprecating to me sends the message "don't use this option".
My impression from the linked ticket is that the idea of "unsafe" is an outdated notion. Do you think there are still scenarios where these packages are unsafe? Is there still a use case?
and set it on by default
In this PR or in a PR destined for the major release?
I can comply with the requests above, but having a good understanding of your position would be helpful.
There was a problem hiding this comment.
Actually, we haven't discussed yet the issue properly. My point is that we could:
- introduce new option
--no-allow-unsafe - update
--allow-unsafeto true by default
There are some rare cases where a user might need --no-allow-hash, for example, when some dependency depends on an old pip version (for some reason) which could break the latest pip-tools.
Later we could deprecate --no-allow-unsafe and gather feedback from users.
There was a problem hiding this comment.
In this PR or in a PR destined for the major release?
Any PR you like, but it should be definitely introduced in the major releases.
There was a problem hiding this comment.
introduce new option --no-allow-unsafe
Done in #1265. This is backward compatible so can come before the next major release.
There are some rare cases where a user might need --no-allow-unsafe, for example, when some dependency depends on an old pip version (for some reason) which could break the latest pip-tools.
How would this work? If the user installs a version of pip that is incompatible with pip-tools, won't pip-tools break? I'm a bit confused how this is a viable environment.
There was a problem hiding this comment.
Done in #1265. This is backward compatible so can come before the next major release.
Thanks!
How would this work? If the user installs a version of pip that is incompatible with pip-tools, won't pip-tools break? I'm a bit confused how this is a viable environment.
If a user installs a requirements.txt built with --no-allow-unsafe then pip-tools wouldn't be broken.
In future versions of pip-tools, the --allow-unsafe behavior will used by default and the option will be removed. According to the pip developers, it is safe to pin the packages currently listed as unsafe. These might have been unsafe long ago before pip vendored its dependencies, but in current versions, there is no demonstrable scenario where this should be considered unsafe. Additionally, using the --generate-hashes argument requires that all packages, including these "unsafe" ones, be pinned. This also suggests that --allow-unsafe should be the default. As a personal anecdote, I've been using the --allow-unsafe option for over a year without any install-time or runtime conflicts. Fixes #989
Codecov Report
@@ Coverage Diff @@
## master #1247 +/- ##
==========================================
- Coverage 99.59% 99.42% -0.17%
==========================================
Files 36 36
Lines 2947 2959 +12
Branches 333 334 +1
==========================================
+ Hits 2935 2942 +7
- Misses 6 7 +1
- Partials 6 10 +4
Continue to review full report at Codecov.
|
In future versions of pip-tools, the --allow-unsafe behavior will used
by default and the option will be removed.
According to the pip developers, it is safe to pin the packages
currently listed as unsafe.
These might have been unsafe long ago before pip vendored its
dependencies, but in current versions, there is no demonstrable scenario
where this should be considered unsafe.
Additionally, using the --generate-hashes argument requires that all
packages, including these "unsafe" ones, be pinned. This also suggests
that --allow-unsafe should be the default.
As a personal anecdote, I've been using the --allow-unsafe option for
over a year without any install-time or runtime conflicts.
Fixes #989
Contributor checklist