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

_service/pypi.py: Do not execute pip cache command when a custom directory is provided #161

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

JeffreyVdb
Copy link
Contributor

In a scenario where the pip cache directory isn't available because the
user doesn't have permissions; pip will error with this message:

ERROR: pip cache commands cannot function since cache is disabled.

This currently cannot be mitigated by specifying the --cache-dir flag
because the python -m pip cache dir command is execute before this
predicate is evaluated. By using an assignment expression (PEP 572), we
can evaluate whether the expression evaluates to a truthy value in the
elif branch (saving it's return value) AFTER first evaluating the
custom directory value.

@tetsuo-cpp tetsuo-cpp self-requested a review December 2, 2021 09:38
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.

Great catch @JeffreyVdb! Thanks for this.

if custom_cache_dir is not None:
return str(custom_cache_dir)
elif pip_cache_dir is not None: # pragma: no cover
elif (
pip_cache_dir := _get_pip_cache() if _PIP_VERSION >= _MINIMUM_PIP_VERSION else None
Copy link
Contributor

Choose a reason for hiding this comment

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

We're trying to support Python 3.6 and later so we'll need to avoid the :=.

Copy link
Contributor Author

@JeffreyVdb JeffreyVdb Dec 2, 2021

Choose a reason for hiding this comment

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

Fixed in: f3f2bcf

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.

Nice! This looks good to me.

@tetsuo-cpp tetsuo-cpp requested a review from woodruffw December 2, 2021 12:47
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM as well. @JeffreyVdb, would you mind committing a CHANGELOG entry as well, under the "Fixed" heading for the upcoming release? You can use the already present entries as a style reference 🙂

In a scenario where the pip cache directory isn't available because the
user doesn't have permissions, pip will error with this message:

```
ERROR: pip cache commands can not function since cache is disabled.
```

This currently cannot be mitigated by specifying the `--cache-dir` flag
because the `python -m pip cache dir` command is execute before this
predicate is evaluated. By using an assignment expression (PEP 572), we
can evaluate whether the expression evaluates to a truthy value in the
`elif` branch (saving it's return value) AFTER first evaluating the
custom directory value.
Assignment expressions are only supported from Python 3.8 and upwards.
Because the if statement consists out of early returns, we can create
another if statement and assign the pip cache directory before the if
statement and after the check for a custom directory has happened.
@JeffreyVdb JeffreyVdb force-pushed the evaluate-pip-cache-dir-after-custom-check branch from f3f2bcf to 6698f25 Compare December 2, 2021 15:59
@JeffreyVdb
Copy link
Contributor Author

LGTM as well. @JeffreyVdb, would you mind committing a CHANGELOG entry as well, under the "Fixed" heading for the upcoming release? You can use the already present entries as a style reference 🙂

Rebased and added an entry to the CHANGELOG. Hopefully that's sufficient information.

@di di merged commit bca886a into pypa:main Dec 2, 2021
@di
Copy link
Member

di commented Dec 2, 2021

Thanks!

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants