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

Should --generate-hashes imply --allow-unsafe? #806

Closed
jcushman opened this issue May 3, 2019 · 9 comments
Closed

Should --generate-hashes imply --allow-unsafe? #806

jcushman opened this issue May 3, 2019 · 9 comments
Labels
question User question

Comments

@jcushman
Copy link
Contributor

jcushman commented May 3, 2019

Some packages, such as pytest and Markdown, include minimum versions of setuptools in their requirements. Running pip-compile --generate-hashes without --allow-unsafe for those packages creates a requirements.txt file that doesn't work with pip install. This creates a bug that is easy to miss until deployment. Should --allow-unsafe be the default behavior for --generate-hashes?

Environment Versions
  1. OS Type: macOS 10.14.4
  2. Python version: Python 3.5.3
  3. pip version: pip 19.1
  4. pip-tools version: pip-compile, version 3.6.2.dev19+g8a1a8ef
Steps to replicate
  1. New virtualenv:
$ pyenv virtualenv 3.5.3 piptest
$ pyenv local piptest
$ pip show setuptools
Name: setuptools
Version: 28.8.0
  1. Require Markdown, which requires a newer version of setuptools:
$ echo "Markdown" > requirements.in
$ $ pip-compile --generate-hashes
#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --generate-hashes
#
markdown==3.1 \
    --hash=sha256:fc4a6f69a656b8d858d7503bda633f4dd63c2d70cf80abdc6eafa64c4ae8c250 \
    --hash=sha256:fe463ff51e679377e3624984c829022e2cfb3be5518726b06f608a07a3aad680
  1. Attempt pip install:
$ pip install -r requirements.txt
Collecting markdown==3.1 (from -r requirements.txt (line 7))
  Using cached https://files.pythonhosted.org/packages/f5/e4/d8c18f2555add57ff21bf25af36d827145896a07607486cc79a2aea641af/Markdown-3.1-py2.py3-none-any.whl
Collecting setuptools>=36 (from markdown==3.1->-r requirements.txt (line 7))
ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
    setuptools>=36 from https://files.pythonhosted.org/packages/ec/51/f45cea425fd5cb0b0380f5b0f048ebc1da5b417e48d304838c02d6288a1e/setuptools-41.0.1-py2.py3-none-any.whl#sha256=c7769ce668c7a333d84e17fe8b524b1c45e7ee9f7908ad0a73e1eda7e6a5aebf (from markdown==3.1->-r requirements.txt (line 7))
Expected result

Successful install of Markdown.

Actual result

Install fails because setuptools==41.0.1 is not pinned.

Discussion

Seems like it would be good to do one of the following:

  • Print a warning to use --allow-unsafe when --generate-hashes is provided and there are unsafe package requirements.
  • Treat --generate-hashes as implying --allow-unsafe, maybe with a different comment in the generated requirements.txt.

I don't know why pip-compile chooses not to include setuptools or why pip chooses to require it, so I'm not sure what the correct thing to do is, but it seems like pip-compile has all the info it needs to recommend whatever that thing is.

(FWIW this is easy to miss and create latent bugs -- I did pip install Markdown first, which upgraded setuptools and masked the error, and then things broke a few days later when another dev tried to recreate their virtualenv.)

@atugushev
Copy link
Member

atugushev commented May 3, 2019

Hello @jcushman,

Thank you for the very detailed report and good catch!

I don't know why pip-compile chooses not to include setuptools

See the comment in the examples/ptrotection.in file:

# This package depends on setuptools, which should not end up in the compiled
# requirements, because it may cause conflicts with pip itself

or why pip chooses to require it

The pip uses setuptools to install packages in editable mode (i.e. setuptools "develop mode"), see:

$ pip uninstall setuptools -y
Uninstalling setuptools-41.0.1:
  Successfully uninstalled setuptools-41.0.1

$ pip install -e git+git@github.com:jazzband/pip-tools.git#egg=pip-tools
Obtaining pip-tools from git+git@github.com:jazzband/pip-tools.git#egg=pip-tools
  Cloning git@github.com:jazzband/pip-tools.git to ./.venv/src/pip-tools
  Running command git clone -q git@github.com:jazzband/pip-tools.git /apps/pip-tools/.venv/src/pip-tools
    ERROR: Complete output from command python setup.py egg_info:
    ERROR: Traceback (most recent call last):
      File "<string>", line 1, in <module>
    ModuleNotFoundError: No module named 'setuptools'
    ----------------------------------------
ERROR: Command "python setup.py egg_info" failed with error code 1 in /apps/pip-tools/.venv/src/pip-tools/

Seems like it would be good to do one of the following:

  • Print a warning to use --allow-unsafe when --generate-hashes is provided and there are unsafe package requirements.

Sounds good to me. I'd rather to print a warning, instead of implying --allow-unsafe with --generate-hashes, since it may cause conflicts with the pip. Let a user to decide.

@jcushman
Copy link
Contributor Author

jcushman commented May 3, 2019

The pip uses setuptools to install packages in editable mode

But that's because the package you're installing is importing setuptools -- not because pip is importing setuptools. pip vendors setuptools and doesn't care what version you have installed locally. You can't break pip itself by pinning setuptools -- you should only have a problem if a target requirement relies on an incompatible version. But that's no different from any other incompatible sub-dependency, and can be resolved the same way.

@atugushev
Copy link
Member

atugushev commented May 4, 2019

pip vendors setuptools and doesn't care what version you have installed locally.

It's not vendored, look at here. Pip only calls subprocess setuptools, see this block. But you are right - pip doesn't care the version.

You can't break pip itself by pinning setuptools -- you should only have a problem if a target requirement relies on an incompatible version. But that's no different from any other incompatible sub-dependency, and can be resolved the same way.

It might be some legacy reason to consider the setuptools unsafe. Insights from creators @nvie @vphilippon @alekzvik would be very helpful.

PS
@jcushman it so nice that you've raised an issue in the pip. I'll Keep an eye on it 👀

@jcushman
Copy link
Contributor Author

jcushman commented May 6, 2019

It's not vendored, look at here.

It's partially vendored. :) From your link, "setuptools is completely stripped to only keep pkg_resources", which is the part of setuptools that pip imports rather than calling as a subprocess.

This discussion leads me to think that --allow-unsafe might actually be safer to use than not use. If you have pytest in your requirements.in then all your deployments will end up with different versions of setuptools based on the time of installation, because pytest will upgrade setuptools for you when it's installed. If you use --allow-unsafe then you end up with identical deployments starting from an identical base image, which sounds safer.

@atugushev
Copy link
Member

It's partially vendored. :) From your link, "setuptools is completely stripped to only keep pkg_resources", which is the part of setuptools that pip imports rather than calling as a subprocess.

Yes, only pkg_resources.

FTR unsafe packages are also used in pip-sync to ignore uninstall those packages.

@atugushev
Copy link
Member

I'll close this in favour of #814. Feel free to reopen if there's something to say.

@jcushman
Copy link
Contributor Author

Yep, thanks for closing this! 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.

@funtikas25
Copy link

hello pleas help what the hash is hier ???????
[18:21:27] [WARNING] there has been a problem while writing to the session file ('OperationalError: attempt to write a readonly database')
[18:21:30] [INFO] retrieved: 07b1e0439ba077bc7bdfbbf2341b1d1f:dc
[18:21:30] [DEBUG] performed 249 queries in 380.19 seconds
[18:21:30] [INFO] retrieving the length of query output
[18:21:30] [INFO] retrieved: 21
[18:23:18] [DEBUG] performed 10 queries in 107.48 seconds
[18:23:18] [DEBUG] starting 8 threads
[18:24:14] [INFO] retrieved: o_i________________ 2/21 (9%)

@atugushev
Copy link
Member

@jcushman

Yep, thanks for closing this! 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.

FTR, this topic on StackOverflow reminds me that we should file an issue and discuss it.

NicolasT added a commit to scality/metalk8s that referenced this issue Dec 16, 2020
Also, update the versions of `pip` and `pip-tools` used in the
`pip-compile` `tox` target, and use the `--allow-unsafe` flag which is
supposedly not really unsafe at all (and will become the default).

See: jazzband/pip-tools#806 (comment)
NicolasT added a commit to scality/metalk8s that referenced this issue Dec 17, 2020
Due to changes in `pluggy` (a dependency of `pytest`) using the
`importlib.metadata` module from the standard library instead of the
`importlib-metadata` package when running on Python >= 3.8, we need to
pin the `basepython` used in the `pip-compile` Tox env to the version
that's actually used in CI, otherwise `pip` complains during
installation of dependencies the `importlib-metadata` package is not
listed (with hashes) in some `requirements.txt`. As such, setting
`basepython` to `python3.6`, similar to 8a9104b.

Also, update the versions of `pip` and `pip-tools` used in the
`pip-compile` `tox` target, and use the `--allow-unsafe` flag which is
supposedly not really unsafe at all (and will become the default).

See: 8a9104b
Closes: #2897
Closes: #2898
See: pytest-dev/pluggy#222
See: https://github.com/pytest-dev/pluggy/pull/223/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7
See: jazzband/pip-tools#806 (comment)
NicolasT added a commit to scality/metalk8s that referenced this issue Dec 17, 2020
Due to changes in `pluggy` (a dependency of `pytest`) using the
`importlib.metadata` module from the standard library instead of the
`importlib-metadata` package when running on Python >= 3.8, we need to
pin the `basepython` used in the `pip-compile` Tox env to the version
that's actually used in CI, otherwise `pip` complains during
installation of dependencies the `importlib-metadata` package is not
listed (with hashes) in some `requirements.txt`. As such, setting
`basepython` to `python3.6`, similar to 8a9104b.

Also, update the versions of `pip` and `pip-tools` used in the
`pip-compile` `tox` target, and use the `--allow-unsafe` flag which is
supposedly not really unsafe at all (and will become the default).

See: 8a9104b
Closes: #2897
Closes: #2898
See: pytest-dev/pluggy#222
See: https://github.com/pytest-dev/pluggy/pull/223/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7
See: jazzband/pip-tools#806 (comment)
NicolasT added a commit to scality/metalk8s that referenced this issue Dec 17, 2020
Due to changes in `pluggy` (a dependency of `pytest`) using the
`importlib.metadata` module from the standard library instead of the
`importlib-metadata` package when running on Python >= 3.8, we need to
pin the `basepython` used in the `pip-compile` Tox env to the version
that's actually used in CI, otherwise `pip` complains during
installation of dependencies the `importlib-metadata` package is not
listed (with hashes) in some `requirements.txt`. As such, setting
`basepython` to `python3.6`, similar to 8a9104b.

Also, update the versions of `pip` and `pip-tools` used in the
`pip-compile` `tox` target, and use the `--allow-unsafe` flag which is
supposedly not really unsafe at all (and will become the default).

See: 8a9104b
Closes: #2897
Closes: #2898
See: pytest-dev/pluggy#222
See: https://github.com/pytest-dev/pluggy/pull/223/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7
See: jazzband/pip-tools#806 (comment)
NicolasT added a commit to scality/metalk8s that referenced this issue Dec 17, 2020
Due to changes in `pluggy` (a dependency of `pytest`) using the
`importlib.metadata` module from the standard library instead of the
`importlib-metadata` package when running on Python >= 3.8, we need to
pin the `basepython` used in the `pip-compile` Tox env to the version
that's actually used in CI, otherwise `pip` complains during
installation of dependencies the `importlib-metadata` package is not
listed (with hashes) in some `requirements.txt`. As such, setting
`basepython` to `python3.6`, similar to 8a9104b.

Also, update the versions of `pip` and `pip-tools` used in the
`pip-compile` `tox` target, and use the `--allow-unsafe` flag which is
supposedly not really unsafe at all (and will become the default).

See: 8a9104b
Closes: #2897
Closes: #2898
See: pytest-dev/pluggy#222
See: https://github.com/pytest-dev/pluggy/pull/223/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7
See: jazzband/pip-tools#806 (comment)
NicolasT added a commit to scality/metalk8s that referenced this issue Dec 18, 2020
Due to changes in `pluggy` (a dependency of `pytest`) using the
`importlib.metadata` module from the standard library instead of the
`importlib-metadata` package when running on Python >= 3.8, we need to
pin the `basepython` used in the `pip-compile` Tox env to the version
that's actually used in CI, otherwise `pip` complains during
installation of dependencies the `importlib-metadata` package is not
listed (with hashes) in some `requirements.txt`. As such, setting
`basepython` to `python3.6`, similar to 8a9104b.

Also, update the versions of `pip` and `pip-tools` used in the
`pip-compile` `tox` target, and use the `--allow-unsafe` flag which is
supposedly not really unsafe at all (and will become the default).

Finally, since we're currently unable to properly update the MacOS X
platform-specific dependencies for the buildchain (in
`buildchain/requirements-Darwin.txt`), the updates to
`buildchain/requirements-Linux.txt` are manually reverted (after running
`tox -e pip-compile` on a Linux host) to ensure both
`buildchain/requirements-Linux.txt` and
`buildchain/requirements-Darwin.txt` remain in sync (except for
`pyinotify` and `macfsevents`, as usual).

See: 8a9104b
Closes: #2897
Closes: #2898
See: pytest-dev/pluggy#222
See: https://github.com/pytest-dev/pluggy/pull/223/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7
See: jazzband/pip-tools#806 (comment)
See: #2986 (comment)
rco-ableton added a commit to Ableton/groovylint that referenced this issue Jan 18, 2021
pip-compile considers several packages "unsafe" for pinning. However, we
would like to be able to pin the pip version itself in requirements-dev
("pip" is one of these unsafe packages).

Adding "--allow-unsafe" tells pip-compile to pin versions of "unsafe"
packages. Despite the warning, there's no evidence of any unsafe effects
from this, and pip-tools is considering deprecating this flag and making
it the default behaviour anyway [1].

I've tested this locally and it seems to work without issue.

[1]: jazzband/pip-tools#806 (comment)
karlbrown-va added a commit to department-of-veterans-affairs/wtf-bot that referenced this issue Jul 19, 2021
…al with setuptools and pip requirements (see jazzband/pip-tools#806 for more details)
diogoteles08 added a commit to diogoteles08/cryptography that referenced this issue Jul 4, 2023
The flag is needed to create hash-pinned requirements for pip and
setup-tools. Find more information about this at these issues from [pip-tools](jazzband/pip-tools#806) and from [pip](pypa/pip#6459).

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
reaperhulk pushed a commit to pyca/cryptography that referenced this issue Jul 11, 2023
)

* ci: Update GitHub owned actions to be referenced by SHA. Work automated using StepSecurity

Signed-off-by: StepSecurity Bot <bot@stepsecurity.io>

* ci: create hash-pinned requirements files for build and publish processes

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* ci: change ci files to install build and publish dependencies using hashes

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* ci: fix path to requirements files

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* ci: rebuild the requirement.txt files using `--allow-unsafe`

The flag is needed to create hash-pinned requirements for pip and
setup-tools. Find more information about this at these issues from [pip-tools](jazzband/pip-tools#806) and from [pip](pypa/pip#6459).

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* refactor(workflows): move build requirements files to a separated folder

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* fix(workflow): requirements download was erasing work from previous steps

Using the actions/checkout to download the requirements.txt was erasing
some necessary files that came from previous steps. Thus, this commit
changes moves the checkout action to the beginnig of the jobs.

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* ci: remove reference to inexistent input in pypi-publish.yml

* docs(workflows): remove comment related to a line already delated from code

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* refactor(workflows): use a workflow-level env var to define path to build requirements file

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* fix(workflows): refer to env vars using ${{  }} sintax

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* refactor(workflows): move build and publish requirements files

Moved from .github/workflows/requirements/ to .github/requirements/

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* docs(workflows): add comments on requirements files explaining their relation

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* ci(workflows): update build dependencies to match exactly the ones at pyproject.toml

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

* ci: remove unnecessary parameter

When calling actions/checkout , we were passing the `ref` parameter as `github.ref`, but it will likely be always main, or the vary same value as the default for this parameter.

* Update dependabot config to cover build/publish dependencies

---------

Signed-off-by: StepSecurity Bot <bot@stepsecurity.io>
Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>
Co-authored-by: StepSecurity Bot <bot@stepsecurity.io>
zsol added a commit to zsol/py.wtf that referenced this issue Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question User question
Projects
None yet
Development

No branches or pull requests

3 participants