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

tox4: fails to process requirement files with --hash #2373

Closed
ssbarnea opened this issue Mar 9, 2022 · 2 comments
Closed

tox4: fails to process requirement files with --hash #2373

ssbarnea opened this issue Mar 9, 2022 · 2 comments
Labels
bug:normal affects many people or has quite an impact
Milestone

Comments

@ssbarnea
Copy link
Member

ssbarnea commented Mar 9, 2022

There is a regression on tox4 where it fails to parse requirement files that contain hashes. While these are not very popular they are still the recommended for security reasons as they protect against potential hacks on pypi registry.

Example of file that causes tox4 to fail, while it works fine with tox3: https://github.com/ansible/ansible-language-server/blob/v0.5.0/docs/requirements.txt

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/ssbarnea/.pyenv/versions/3.10.2/lib/python3.10/site-packages/tox/session/cmd/run/single.py", line 45, in _evaluate
    tox_env.setup()
  File "/Users/ssbarnea/.pyenv/versions/3.10.2/lib/python3.10/site-packages/tox/tox_env/api.py", line 226, in setup
    self._setup_env()
  File "/Users/ssbarnea/.pyenv/versions/3.10.2/lib/python3.10/site-packages/tox/tox_env/python/runner.py", line 91, in _setup_env
    self._install_deps()
  File "/Users/ssbarnea/.pyenv/versions/3.10.2/lib/python3.10/site-packages/tox/tox_env/python/runner.py", line 95, in _install_deps
    self.installer.install(requirements_file, PythonRun.__name__, "deps")
  File "/Users/ssbarnea/.pyenv/versions/3.10.2/lib/python3.10/site-packages/tox/tox_env/python/pip/pip_install.py", line 84, in install
    self._install_requirement_file(arguments, section, of_type)
  File "/Users/ssbarnea/.pyenv/versions/3.10.2/lib/python3.10/site-packages/tox/tox_env/python/pip/pip_install.py", line 95, in _install_requirement_file
    raise HandledError(f"{exception} for tox env py within deps")
tox.report.HandledError: unrecognized arguments: --hash=sha256:446438bdcca0e05bd45ea2de1668c1d9b032e1a9154c2c259092d77031ddd359 --hash=sha256:a661d72d58e6ea8a57f7a86e37d86716863ee5e92788398526d58b26a4e4dc02 for tox env py within deps

It should be remarked that these files are produced by pip-compile (pip-tools).

Note: I temporary removed the hashes from the lock file but we cannot really ignore this issue.

@ssbarnea ssbarnea added the bug:normal affects many people or has quite an impact label Mar 9, 2022
ssbarnea added a commit to ssbarnea/ansible-language-server that referenced this issue Mar 9, 2022
Remove hashes from the deps pins as they are not compatible with
either dependabot or tox4.

Related: tox-dev/tox#2373
@gaborbernat
Copy link
Member

PRs are welcome 👍 Note we do support hashes, see https://github.com/tox-dev/tox/blob/rewrite/tests/tox_env/python/pip/req/test_file.py#L241-L272, so there must be some edge case with it.

@adamchainz
Copy link
Contributor

I'm seeing this too with the beta 2 release, e.g. this run

py38-django30: FAIL ✖ in 0.14 seconds
py38-django31: internal error
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/pip_install.py", line 93, in _install_requirement_file
    new_options, new_reqs = arguments.unroll()
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req_file.py", line 73, in unroll
    opts_dict = vars(self.options)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 148, in options
    self._ensure_requirements_parsed()
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 164, in _ensure_requirements_parsed
    self._requirements = self._parse_requirements(opt=self._opt, recurse=True)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 168, in _parse_requirements
    for parsed_line in self._parse_and_recurse(str(self._path), self.is_constraint, recurse):
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 202, in _parse_and_recurse
    yield from self._parse_and_recurse(req_path, nested_constraint, recurse)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 190, in _parse_and_recurse
    for line in self._parse_file(filename, constraint):
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 212, in _parse_file
    args_str, opts = self._parse_line(line)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/file.py", line 259, in _parse_line
    opts = self._parser.parse_args(args)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/argparse.py", line 1771, in parse_args
    self.error(msg % ' '.join(argv))
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/argparse.py", line 2521, in error
    self.exit(2, _('%(prog)s: error: %(message)s\n') % args)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/tox/tox_env/python/pip/req/args.py", line 18, in exit
    raise ValueError(msg)
ValueError: unrecognized arguments: --hash=sha256:2f8abc20f7248433085eda803936d98992f1343ddb022065779f37c5da0181d0 --hash=sha256:88d59c13d634dcffe0510be048210188edd79aeccb6a6c9028cdad6f31d730a9

After a little debugging I can see that the support for --hash is conditinoally added to the parser. This support is enabled for the parser constructed in the RequirementsFile class, but the PythonDeps subclass deliberately disables it.

I think the problem is that the parser from PythonDeps is being used for the top level install and the files from -r. But files from -r should use the parser with cli_only=False, as in RequirementsFile instead.

@gaborbernat gaborbernat added this to the 4.0 milestone May 8, 2022
masenf added a commit to masenf/tox that referenced this issue Nov 25, 2022
Add a test case for issue tox-dev#2373

* Specifying `--hash` in the deps list doesn't work (pip would
  reject this anyway).
* Specifying `--hash` in a requirements.txt file named in the deps list
  should work, and recursive parsing should correctly extract the hash.
masenf added a commit to masenf/tox that referenced this issue Nov 25, 2022
Remove `cli_only` parameter from `build_parser`.

Remove special case handling for `--hash` option (only valid in
requirements.txt files, not `pip install`).

Validate options in PythonDeps._parse_requirements:
  * Only check "cli_only" logic for ParsedRequirement objects that
    directly come from the PythonDeps.
  * Allow included requirements.txt lines to correctly parse `--hash` (Fix tox-dev#2373).
  * Provides a more contextual error message to end users when `--hash`
    is used in the deps list.
masenf added a commit to masenf/tox that referenced this issue Nov 25, 2022
gaborbernat pushed a commit that referenced this issue Nov 26, 2022
…2547)

* [test case] tox4: fails to process requirement files with --hash

Add a test case for issue #2373

* Specifying `--hash` in the deps list doesn't work (pip would
  reject this anyway).
* Specifying `--hash` in a requirements.txt file named in the deps list
  should work, and recursive parsing should correctly extract the hash.

* PythonDeps: move cli_only handling logic to _parse_requirements

Remove `cli_only` parameter from `build_parser`.

Remove special case handling for `--hash` option (only valid in
requirements.txt files, not `pip install`).

Validate options in PythonDeps._parse_requirements:
  * Only check "cli_only" logic for ParsedRequirement objects that
    directly come from the PythonDeps.
  * Allow included requirements.txt lines to correctly parse `--hash` (Fix #2373).
  * Provides a more contextual error message to end users when `--hash`
    is used in the deps list.

* changelog for issue #2373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:normal affects many people or has quite an impact
Projects
None yet
Development

No branches or pull requests

3 participants