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

Don't explode sdists before installing them #166

Merged
merged 12 commits into from
Dec 5, 2021
Merged

Don't explode sdists before installing them #166

merged 12 commits into from
Dec 5, 2021

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Dec 2, 2021

Fixes #159.

@woodruffw woodruffw added bug Something isn't working component:dep-sources Dependency sources labels Dec 2, 2021
@woodruffw woodruffw self-assigned this Dec 2, 2021
@woodruffw
Copy link
Member Author

woodruffw commented Dec 2, 2021

This now fails with a ResolutionImpossible, which I'll also tackle in this PR:

resolvelib.resolvers.ResolutionImpossible:
[
  RequirementInformation(
    requirement=<Requirement('setuptools==57.4.0')>,
    parent=<progress==1.6 wheel=False>
  ),
  RequirementInformation(
    requirement=<Requirement('setuptools<51.0.0,>=50.3.2')>,
    parent=<cyclonedx-python-lib==0.11.1 wheel=True>
  ),
  RequirementInformation(
    requirement=<Requirement('setuptools==57.4.0')>,
    parent=<progress==1.6 wheel=False>
  ),
  RequirementInformation(
    requirement=<Requirement('setuptools==50.3.2')>,
    parent=<cyclonedx-python-lib==0.11.1 wheel=False>
  )
]

Off the top of my head, I'm, not sure why all of these dependencies have different versions of setuptools as subdependencies. Especially since only cyclonedx-python-lib lists it explicitly as a dependency.

@woodruffw
Copy link
Member Author

Here's what happens when we install the cyclonedx-python-lib sdist:

DEBUG:pip_audit._virtual_env:completed: package_install_cmd=['/var/folders/zj/hy934vnj5xs68zv6w4b_f6s40000gn/T/tmpl7rquzwk/bin/python3', '-m', 'pip', 'install', '--no-cache-dir', '/var/folders/zj/hy934vnj5xs68zv6w4b_f6s40000gn/T/tmps27q9klr/cyclonedx-python-lib-0.11.1.tar.gz'], process.stdout=b"Processing /var/folders/zj/hy934vnj5xs68zv6w4b_f6s40000gn/T/tmps27q9klr/cyclonedx-python-lib-0.11.1.tar.gz
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
  Preparing metadata (pyproject.toml): started
  Preparing metadata (pyproject.toml): finished with status 'done'
  Collecting toml<0.11.0,>=0.10.2
  Downloading toml-0.10.2-py2.py3-none-any.whl (16 kB)
  Collecting setuptools<51.0.0,>=50.3.2
  Downloading setuptools-50.3.2-py3-none-any.whl (785 kB)
  Collecting types-toml<0.11.0,>=0.10.1
  Downloading types_toml-0.10.1-py3-none-any.whl (2.1 kB)
  Collecting requirements_parser<0.3.0,>=0.2.0
  Downloading requirements-parser-0.2.0.tar.gz (6.3 kB)
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'done'
  Collecting packageurl-python<0.10.0,>=0.9.4
  Downloading packageurl_python-0.9.6-py3-none-any.whl (23 kB)
  Collecting types-setuptools<58.0.0,>=57.4.2
  Downloading types_setuptools-57.4.4-py3-none-any.whl (27 kB)
  Building wheels for collected packages: cyclonedx-python-lib, requirements-parser
  Building wheel for cyclonedx-python-lib (pyproject.toml): started
  Building wheel for cyclonedx-python-lib (pyproject.toml): finished with status 'done'
  Created wheel for cyclonedx-python-lib: filename=cyclonedx_python_lib-0.11.1-py3-none-any.whl size=127708 sha256=ed6cf44f29b1f31835f818ca3b18deaee28be32a3179eadf9ad6c853217c93f9
  Stored in directory: /private/var/folders/zj/hy934vnj5xs68zv6w4b_f6s40000gn/T/pip-ephem-wheel-cache-x66n48lz/wheels/df/1c/0a/ab8052acc2f11b332db5a7d778754ce1f2da723056cef5d7d5
  Building wheel for requirements-parser (setup.py): started
  Building wheel for requirements-parser (setup.py): finished with status 'done'
  Created wheel for requirements-parser: filename=requirements_parser-0.2.0-py3-none-any.whl size=7688 sha256=08fe0acc14dda5c973036b3cd6c17e4870f701f748c6bdc1d3a21205e9a48630
  Stored in directory: /private/var/folders/zj/hy934vnj5xs68zv6w4b_f6s40000gn/T/pip-ephem-wheel-cache-x66n48lz/wheels/97/3a/c8/0a5a6ac041c7b83638f97b4a69b2cf6e76a03a1643d3926f5b
  Successfully built cyclonedx-python-lib requirements-parser
Installing collected packages: types-toml, types-setuptools, toml, setuptools, requirements-parser, packageurl-python, cyclonedx-python-lib
  Attempting uninstall: setuptools
    Found existing installation: setuptools 57.4.0
    Uninstalling setuptools-57.4.0:
      Successfully uninstalled setuptools-57.4.0
      Successfully installed cyclonedx-python-lib-0.11.1 packageurl-python-0.9.6 requirements-parser-0.2.0 setuptools-50.3.2 toml-0.10.2 types-setuptools-57.4.4 types-toml-0.10.1
", process.stderr=b''

So it downgrades the already installed setuptools, setting up the conflict with progress. I'm not really sure why it does that, given that the newer version is compatible with the version range and we haven't asked it to downgrade.

@woodruffw
Copy link
Member Author

Being a little more clever about sdist filtering reduced the resolution error down to two:

[
    RequirementInformation(
        requirement=<Requirement('setuptools==57.4.0')>,
        parent=<progress==1.6 wheel=False>
    ),
    RequirementInformation(
        requirement=<Requirement('setuptools<51.0.0,>=50.3.2')>,
        parent=<cyclonedx-python-lib==0.11.1 wheel=True>
    )
]

@di
Copy link
Sponsor Member

di commented Dec 2, 2021

   RequirementInformation(
       requirement=<Requirement('setuptools==57.4.0')>,
       parent=<progress==1.6 wheel=False>
   ),

Where is this coming from? That's a really tight pin that I don't see anywhere in progress's source distribution for that release

@woodruffw
Copy link
Member Author

woodruffw commented Dec 2, 2021

Where is this coming from?

That's a really good question; I'm not 100% sure yet.

Our install step for progress boils down to python -m pip install progress-1.6.tar.gz inside of a virtual environment, so we shouldn't be doing anything particularly unusual.

Here's the pip install step as it runs:

DEBUG:pip_audit._virtual_env:ran: ['/var/folders/zj/hy934vnj5xs68zv6w4b_f6s40000gn/T/tmpadmaoxgu/bin/python3', '-m', 'pip', 'install', 'progress==1.6'], stdout=b"Collecting progress==1.6
  Downloading progress-1.6.tar.gz (7.8 kB)
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'done'
  Building wheels for collected packages: progress
  Building wheel for progress (setup.py): started
  Building wheel for progress (setup.py): finished with status 'done'
  Created wheel for progress: filename=progress-1.6-py3-none-any.whl size=9628 sha256=6c6ea87b7ca81e99c45aa07897185d02aa948cd8d0d1bc15446fd2164d0bd21c
  Stored in directory: /Users/william/Library/Caches/pip/wheels/a2/68/5f/c339b20a41659d856c93ccdce6a33095493eb82c3964aac5a1
  Successfully built progress
Installing collected packages: progress
Successfully installed progress-1.6
", stderr=b''

@woodruffw woodruffw marked this pull request as ready for review December 3, 2021 22:22
@woodruffw
Copy link
Member Author

This is ready for review: it doesn't fix the impossible resolution bug, but I want to get it out of the way so that we can make other (performance-minded) improvements to this part of the code.

@woodruffw woodruffw added the performance Something isn't as fast or responsive as it should be. label Dec 3, 2021
Copy link
Contributor

@tetsuo-cpp tetsuo-cpp left a comment

Choose a reason for hiding this comment

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

LGTM!

names = t.getnames()
pkg_name = names[0]
t.extractall(pkg_dir)
sdist = Path(pkg_dir) / self.filename.name
Copy link
Contributor

Choose a reason for hiding this comment

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

Haha that's awesome. I didn't realise you could do that.

@tetsuo-cpp tetsuo-cpp merged commit a8f9fa2 into main Dec 5, 2021
@tetsuo-cpp tetsuo-cpp deleted the ww/dist-bug branch December 5, 2021 03:10
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Dec 7, 2021
## [1.1.0]

### Added

* CLI: The `--path <PATH>` flag has been added, allowing users to limit
  dependency discovery to one or more paths (specified separately)
  when `pip-audit` is invoked in environment mode
  ([#148](pypa/pip-audit#148))

* CLI: The `pip-audit` CLI can now be accessed through `python -m pip_audit`.
  All functionality is identical to the functionality provided by the
  `pip-audit` entrypoint
  ([#173](pypa/pip-audit#173))

* CLI: The `--verbose` flag has been added, allowing users to receive more
  more verbose output from `pip-audit`. Supplying the `--verbose` flag
  overrides the `PIP_AUDIT_LOGLEVEL` environment variable and is equivalent to
  setting it to `debug`
  ([#185](pypa/pip-audit#185))

### Changed

* CLI: `pip-audit` now clears its spinner bar from the terminal upon
  completion, preventing visual confusion
  ([#174](pypa/pip-audit#174))

### Fixed

* Dependency sources: a crash caused by `platform.python_version` returning
  an version string that couldn't be parsed as a PEP-440 version was fixed
  ([#175](pypa/pip-audit#175))

* Dependency sources: a crash caused by incorrect assumptions about
  the structure of source distributions was fixed
  ([#166](pypa/pip-audit#166))

* Vulnerability sources: a performance issue on Windows caused by cache failures
  was fixed ([#178](pypa/pip-audit#178))

## [1.0.1] - 2021-12-02

### Fixed

* CLI: The `--desc` flag no longer requires a following argument. If passed
  as a bare option, `--desc` is equivalent to `--desc on`
  ([#153](pypa/pip-audit#153))

* Dependency resolution: The PyPI-based dependency resolver no longer throws
  an uncaught exception on package resolution errors; instead, the package
  is marked as skipped and an appropriate warning or fatal error (in
  `--strict` mode) is produced
  ([#162](pypa/pip-audit#162))

* CLI: When providing the `--cache-dir` flag, the command to read the pip cache
  directory is no longer executed. Previously this was always executed and
  could result into failure when the command fails. In CI environments, the
  default `~/.cache` directory is typically not writable by the build user and
  this meant that the `python -m pip cache dir` would fail before this fix,
  even if the `--cache-dir` flag was provided.
  ([#161](pypa/pip-audit#161))

## [1.0.0] - 2021-12-01

### Added

* This is the first stable release of `pip-audit`! The CLI is considered
  stable from this point on, and all changes will comply with
  [Semantic Versioning](https://semver.org/)

## [0.0.9] - 2021-12-01

### Added

* CLI: Skipped dependencies are now listed in the output of `pip-audit`,
  for supporting output formats
  ([#145](pypa/pip-audit#145))
* CLI: `pip-audit` now supports a "strict" mode (enabled with `-S` or
  `--strict`) that fails if the audit if any individual dependency cannot be
  resolved or audited. The default behavior is still to skip any individual
  dependency errors ([#146](pypa/pip-audit#146))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:dep-sources Dependency sources performance Something isn't as fast or responsive as it should be.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dependency resolution fails to install cyclonedx-python-lib-0.11.1
3 participants