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

Deprecate --allow-unsafe option #989

Open
atugushev opened this issue Nov 16, 2019 · 9 comments
Open

Deprecate --allow-unsafe option #989

atugushev opened this issue Nov 16, 2019 · 9 comments
Labels
cli Related to command line interface things deprecation Related to deprecations or removals needs discussion Need some more discussion

Comments

@atugushev
Copy link
Member

atugushev commented Nov 16, 2019

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.

Originally posted by @jcushman in #806 (comment)

Additional context

pypa/pip#6459
https://stackoverflow.com/a/58864335/1529164

@atugushev atugushev added deprecation Related to deprecations or removals needs discussion Need some more discussion labels Nov 16, 2019
@agostbiro
Copy link

As a user, I'd love to see this! I find both the flag and the warning message printed without the flag confusing.

@atugushev atugushev added the cli Related to command line interface things label Jan 10, 2020
atugushev pushed a commit that referenced this issue Dec 30, 2020
In future versions of pip-tools, the --allow-unsafe behavior will used
by default. Adding the --no-allow-unsafe option now allows users to
continue using the old behavior in a backward compatible way.

Refs #989
@apljungquist
Copy link
Contributor

Out of curiosity, does anyone know the reason why these packages were considered unsafe in the first place?

I tried to track it down a while back to assure myself that --allow-unsafe is actually safe. Since I could not find anything I assumed the reason is that the result of installing from a requirements file could be different depending on the order that packages are installed and that, consequently, I would be safe if I installed setuptools and pip explicitly before the other packages. (I am aware that some other packages are also sensitive to order, such as opencv and opencv-contrib 😒)

@jcushman
Copy link
Contributor

I don't know, but atugushev asked the same question back in 2019 and tagged some people who might know, and no one offered any answers. So it seems like whatever the answer is, we're not super likely to find someone who remembers.

atugushev also noted in that thread that "unsafe packages are also used in pip-sync to ignore uninstall those packages," so maybe another guess could be that they were originally marked as "unsafe" packages because they were unsafe to uninstall rather than unsafe to pin, and that concept spread to other parts of the codebase accidentally.

@apljungquist
Copy link
Contributor

apljungquist commented May 26, 2022

Thanks! I never considered that pip-sync may be a factor. I just sort of assumed that it was copied from pip where setuptools, and later pip and wheel, have been excluded from freeze for time immemorial (the migration to git in 2008).

@okuuva
Copy link

okuuva commented Jan 4, 2023

Would it be time to do the switch? --allow-unsafe has been deprecated for two years now and while I don't have any kind of numbers to back this up I think plenty of people have been using it just to silence the warning for much longer than that. I didn't even realise that it might be related to pip-sync since the most common use case of pip-tools for me has always been generating a "lockfile" to be fed for pip install -r in the CI. And I'm pretty sure I'm not alone with that one.

@jeffwidman
Copy link
Contributor

@atugushev I'm unclear of the scope of this issue... is it:

  1. drive towards a decision of do/don't deprecate --allow-unsafe
  2. the decision has already been made, execute the code / docs changes necessary to alert users of the impending removal
  3. decision has been made and users have been warned, need to actually remove it in the next major release (too bad it didn't land in 7.0)

Anyway, curious what's the next action on this issue?

@atugushev
Copy link
Member Author

It's stated on deprecations section.

In future versions, the --allow-unsafe behavior will be enabled by default. Use --no-allow-unsafe to keep the old behavior. It is recommended to pass the --allow-unsafe now to adapt to the upcoming change.

We simply forgot to switch the default in 7.0. :/

@ThiefMaster
Copy link

I'd love to understand why these packages are considered (un)safe for pining...

One thing that comes to my mind: My application is really an application and not a library. I have a fully pinned set of dependencies, and my users should never use other versions (yes, we release updates if libraries have security fixes that are applicable to our usage of them). So the requirements.txt from pip-compile is used for my package's install_requires metadata.

Now, when someone who's installing that app creates a venv, then that venv contains pip and setuptools. Not sure which version is chosen, "latest" would be my expectation especially for pip. But now I pip install myapp, and unless I did a release very recently, it'll probably downgrade pip and setuptools - just because pip-compile now thinks it's a good idea to pin those as well.

AFAIK pip and setuptools are both VERY strict about backwards compatibility and I'd even consider them part of the Python stdlib even though they aren't really part of it. So why would I want to pin those...?

If there was at least an option to exclude those packages (either as "unsafe" or just ignoring them) without having to specify this on the command line all the time (e.g. a config file), I think the change of the default wouldn't feel so annoying.

@hauntsaninja
Copy link
Contributor

setuptools is not particularly strict about backwards compatibility. I've been broken by it many times over. The community is moving away from setuptools having any sort of special-cased position in the ecosystem. There's maybe more of an argument for pip, since it's special-cased by stdlib via ensurepip / the last time I was broken by pip was 20.3. But pip does still make breaking changes, e.g. I know 24.1 will break some things for me (and if you see the changelog they make breaking changes every now and then).

IMO, pip-tools is definitely doing the right thing by changing the default here.

If explicitly specifying --unsafe-package setuptools is too verbose, maybe use a wrapper or an alias? An --unsafe-package-file option seems like a reasonable but separate feature request.

toofar added a commit to qutebrowser/qutebrowser that referenced this issue Aug 10, 2024
This is to match the `pip freeze` requirements compilation method. It's
not clear to me if we actually want this behaviour or not.

If seems `pip freeze` will exclude dependencies of itself: https://pip.pypa.io/en/stable/cli/pip_freeze/#cmdoption-all
Even if there are other packages installed that depend on those
dependencies.

`uv pip compile`, and now the original `pip-compile` both have decided
to include setuptools in generated requirements files:

    astral-sh/uv#1353
    jazzband/pip-tools#989 (comment)

So I'm not sure if we have a reason for going against this here or if
they were just being excluded because that's what pip freeze does.

Hopefully we can drop this commit and use the default behaviour in the
future. For now when I'm trying to provide the new backend it's here to
make the diff of generated files more precise.

This message prefix to identify a pip compile comment was taken from
these examples:

    # The following packages were excluded from the output:
    # setuptool

    # The following packages are considered to be unsafe in a requirements file:
    # setuptools==41.4.0        # via protobuf
toofar added a commit to qutebrowser/qutebrowser that referenced this issue Aug 10, 2024
This is to match the `pip freeze` requirements compilation method. It's
not clear to me if we actually want this behaviour or not.

If seems `pip freeze` will exclude dependencies of itself: https://pip.pypa.io/en/stable/cli/pip_freeze/#cmdoption-all
Even if there are other packages installed that depend on those
dependencies.

`uv pip compile`, and now the original `pip-compile` both have decided
to include setuptools in generated requirements files:

    astral-sh/uv#1353
    jazzband/pip-tools#989 (comment)

So I'm not sure if we have a reason for going against this here or if
they were just being excluded because that's what pip freeze does.

Hopefully we can drop this commit and use the default behaviour in the
future. For now when I'm trying to provide the new backend it's here to
make the diff of generated files more precise.

This message prefix to identify a pip compile comment was taken from
these examples:

    # The following packages were excluded from the output:
    # setuptool

    # The following packages are considered to be unsafe in a requirements file:
    # setuptools==41.4.0        # via protobuf
toofar added a commit to qutebrowser/qutebrowser that referenced this issue Aug 12, 2024
This is to match the `pip freeze` requirements compilation method. It's
not clear to me if we actually want this behaviour or not.

If seems `pip freeze` will exclude dependencies of itself: https://pip.pypa.io/en/stable/cli/pip_freeze/#cmdoption-all
Even if there are other packages installed that depend on those
dependencies.

`uv pip compile`, and now the original `pip-compile` both have decided
to include setuptools in generated requirements files:

    astral-sh/uv#1353
    jazzband/pip-tools#989 (comment)

So I'm not sure if we have a reason for going against this here or if
they were just being excluded because that's what pip freeze does.

Hopefully we can drop this commit and use the default behaviour in the
future. For now when I'm trying to provide the new backend it's here to
make the diff of generated files more precise.

This message prefix to identify a pip compile comment was taken from
these examples:

    # The following packages were excluded from the output:
    # setuptool

    # The following packages are considered to be unsafe in a requirements file:
    # setuptools==41.4.0        # via protobuf
toofar added a commit to qutebrowser/qutebrowser that referenced this issue Sep 7, 2024
This is to match the `pip freeze` requirements compilation method. It's
not clear to me if we actually want this behaviour or not.

If seems `pip freeze` will exclude dependencies of itself: https://pip.pypa.io/en/stable/cli/pip_freeze/#cmdoption-all
Even if there are other packages installed that depend on those
dependencies.

`uv pip compile`, and now the original `pip-compile` both have decided
to include setuptools in generated requirements files:

    astral-sh/uv#1353
    jazzband/pip-tools#989 (comment)

So I'm not sure if we have a reason for going against this here or if
they were just being excluded because that's what pip freeze does.

Hopefully we can drop this commit and use the default behaviour in the
future. For now when I'm trying to provide the new backend it's here to
make the diff of generated files more precise.

This message prefix to identify a pip compile comment was taken from
these examples:

    # The following packages were excluded from the output:
    # setuptool

    # The following packages are considered to be unsafe in a requirements file:
    # setuptools==41.4.0        # via protobuf
toofar added a commit to qutebrowser/qutebrowser that referenced this issue Sep 18, 2024
This is to match the `pip freeze` requirements compilation method. It's
not clear to me if we actually want this behaviour or not.

If seems `pip freeze` will exclude dependencies of itself: https://pip.pypa.io/en/stable/cli/pip_freeze/#cmdoption-all
Even if there are other packages installed that depend on those
dependencies.

`uv pip compile`, and now the original `pip-compile` both have decided
to include setuptools in generated requirements files:

    astral-sh/uv#1353
    jazzband/pip-tools#989 (comment)

So I'm not sure if we have a reason for going against this here or if
they were just being excluded because that's what pip freeze does.

Hopefully we can drop this commit and use the default behaviour in the
future. For now when I'm trying to provide the new backend it's here to
make the diff of generated files more precise.

This message prefix to identify a pip compile comment was taken from
these examples:

    # The following packages were excluded from the output:
    # setuptool

    # The following packages are considered to be unsafe in a requirements file:
    # setuptools==41.4.0        # via protobuf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to command line interface things deprecation Related to deprecations or removals needs discussion Need some more discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants