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

requirement: Fix --skip-editable flag #499

Merged
merged 3 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions pip_audit/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,10 @@ def audit() -> None: # pragma: no cover
# within the RequirementSource instead of in-line here.
source = RequirementSource(
req_files,
ResolveLibResolver(
index_urls, args.timeout, args.cache_dir, args.skip_editable, state
),
ResolveLibResolver(index_urls, args.timeout, args.cache_dir, state),
require_hashes=args.require_hashes,
no_deps=args.no_deps,
skip_editable=args.skip_editable,
state=state,
)
elif args.project_path is not None:
Expand All @@ -430,9 +429,7 @@ def audit() -> None: # pragma: no cover
# Determine which kind of project file exists in the project path
source = _dep_source_from_project_path(
args.project_path,
ResolveLibResolver(
index_urls, args.timeout, args.cache_dir, args.skip_editable, state
),
ResolveLibResolver(index_urls, args.timeout, args.cache_dir, state),
state,
)
else:
Expand Down
9 changes: 9 additions & 0 deletions pip_audit/_dependency_source/requirement.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def __init__(
*,
require_hashes: bool = False,
no_deps: bool = False,
skip_editable: bool = False,
state: AuditState = AuditState(),
) -> None:
"""
Expand All @@ -64,12 +65,16 @@ def __init__(
dependency resolution is not performed and the inputs are checked
and treated as "frozen".

`skip_editable` controls whether requirements marked as "editable" are skipped.
By default, editable requirements are not skipped.

`state` is an `AuditState` to use for state callbacks.
"""
self._filenames = filenames
self._resolver = resolver
self._require_hashes = require_hashes
self._no_deps = no_deps
self._skip_editable = skip_editable
self.state = state
self._dep_cache: dict[Path, dict[Requirement, set[Dependency]]] = {}

Expand Down Expand Up @@ -105,6 +110,10 @@ def collect(self) -> Iterator[Dependency]:
"please specify them with #egg=your_package_name==your_package_version",
)
continue
if self._skip_editable and req.is_editable:
yield SkippedDependency(
name=req.name, skip_reason="requirement marked as editable"
)
if req.marker is None or req.marker.evaluate():
# This means we have a duplicate requirement for the same package
if req.name in req_names:
Expand Down
22 changes: 1 addition & 21 deletions pip_audit/_dependency_source/resolvelib/resolvelib.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@

import logging
from pathlib import Path
from typing import Union

from packaging.requirements import Requirement as _Requirement
from pip_api import Requirement as ParsedRequirement
from packaging.requirements import Requirement
from requests.exceptions import HTTPError
from resolvelib import BaseReporter, Resolver
from resolvelib.resolvers import ResolutionImpossible
Expand All @@ -32,10 +30,6 @@
PYPI_URL = "https://pypi.org/simple/"


# TODO: Replace with _Requirement | ParsedRequirement once our minimum is 3.10.
Requirement = Union[_Requirement, ParsedRequirement]


class ResolveLibResolver(DependencyResolver):
"""
An implementation of `DependencyResolver` that uses `resolvelib` as its
Expand All @@ -47,7 +41,6 @@ def __init__(
index_urls: list[str] = [PYPI_URL],
timeout: int | None = None,
cache_dir: Path | None = None,
skip_editable: bool = False,
state: AuditState = AuditState(),
) -> None:
"""
Expand All @@ -56,9 +49,6 @@ def __init__(
`timeout` and `cache_dir` are optional arguments for HTTP timeouts
and caching, respectively.

`skip_editable` controls whether requirements marked as "editable" are skipped.
By default, editable requirements are not skipped.

`state` is an `AuditState` to use for state callbacks.
"""
self.index_urls = index_urls
Expand All @@ -68,7 +58,6 @@ def __init__(
self.session = caching_session(cache_dir, use_pip=True)
self.state = state
self.reporter = BaseReporter()
self._skip_editable = skip_editable

def resolve(
self,
Expand All @@ -79,15 +68,6 @@ def resolve(
Resolve the given `Requirement` into a `Dependency` list.
"""

# HACK: `resolve` takes both `packaging.Requirement` and `pip_api.Requirement`,
# since the latter is a subclass. But only the latter knows whether the
# requirement is editable, so we need to check for it here.
if isinstance(req, ParsedRequirement):
if req.editable and self._skip_editable:
return [
SkippedDependency(name=req.name, skip_reason="requirement marked as editable")
]

provider = PyPIProvider(self.index_urls, req_hashes, self.session, self.timeout, self.state)
resolver: Resolver = Resolver(provider, self.reporter)

Expand Down
10 changes: 0 additions & 10 deletions test/dependency_source/resolvelib/test_resolvelib.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import requests
from packaging.requirements import Requirement
from packaging.version import Version
from pip_api import Requirement as ParsedRequirement
from requests.exceptions import HTTPError

from pip_audit._dependency_source import RequirementHashes, resolvelib
Expand Down Expand Up @@ -499,15 +498,6 @@ def get_multiple_index_package_mock(url):
assert resolved_deps[req] == [ResolvedDependency("flask", Version("0.5"))]


def test_resolvelib_skip_editable():
resolver = resolvelib.ResolveLibResolver(skip_editable=True)
req = ParsedRequirement("foo==1.0.0", editable=True, filename="stub", lineno=1)

deps = resolver.resolve(req, RequirementHashes()) # type: ignore
assert len(deps) == 1
assert deps[0] == SkippedDependency(name="foo", skip_reason="requirement marked as editable")


def test_resolvelib_no_links(monkeypatch):
# Simulate the project page containing no versions
data = ""
Expand Down
14 changes: 14 additions & 0 deletions test/dependency_source/test_requirement.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,20 @@ def test_requirement_source_non_editable_without_egg_fragment(monkeypatch):
)


@pytest.mark.online
Copy link
Member

Choose a reason for hiding this comment

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

Does this test need to be online? From the body it looks like it shouldn't need network access 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! I'll fix this in #488.

def test_requirement_source_editable_skip(monkeypatch):
source = requirement.RequirementSource(
[Path("requirements1.txt")], ResolveLibResolver(), skip_editable=True
)

monkeypatch.setattr(
pip_requirements_parser, "get_file_content", lambda _: "-e file:flask.py#egg=flask==2.0.1"
)

specs = list(source.collect())
assert SkippedDependency(name="flask", skip_reason="requirement marked as editable") in specs


def _check_fixes(
input_reqs: list[str],
expected_reqs: list[str],
Expand Down