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

Extra requirement support (extras_require) #1363

Merged
merged 17 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
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
42 changes: 26 additions & 16 deletions piptools/scripts/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@
from ..repositories import LocalRequirementsRepository, PyPIRepository
from ..repositories.base import BaseRepository
from ..resolver import Resolver
from ..utils import UNSAFE_PACKAGES, dedup, is_pinned_requirement, key_from_ireq
from ..utils import (
UNSAFE_PACKAGES,
dedup,
is_pinned_requirement,
key_from_ireq,
req_check_markers,
)
from ..writer import OutputWriter

DEFAULT_REQUIREMENTS_FILE = "requirements.in"
Expand Down Expand Up @@ -61,6 +67,12 @@ def _get_default_option(option_name: str) -> Any:
is_flag=True,
help="Clear any caches upfront, rebuild from scratch",
)
@click.option(
"--extra",
"extras",
multiple=True,
help="names of extras_require to install",
Copy link
Member

Choose a reason for hiding this comment

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

  1. s/n/N
  2. If this is supposed to install only extras but not the default runtime deps, can this be renamed into --extra-only?
  3. If it's supposed to merge those extras with the main runtime deps list, can we add an additional flag --extras-only/--exclude-default-deps or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The last one, default dependencies are always selected. I don't think --exclude-default-deps makes sense, I'd make a separate discussion for it. AFAIK, setuptools/poetry/flit/pip doesn't have a way to opt-out installing the default dependencies, so I see no reason why pip-tools should.

Copy link
Member

Choose a reason for hiding this comment

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

It's actually a long-standing feature request in pip and I think there's been no opposition, just nobody had time to come up with an implementation. The main motivation was that people often supply test/dev/docs extras for different processes they have and some of them don't really require the dist itself to be installed. Of course, there's another camp saying that this sort of deps should be put elsewhere because they aren't necessary for the runtime and there should be env pins anyway.
But if pip-tools were to implement this flag, it'd facilitate a hybrid of these two approaches with the direct open-ended deps in the dist metadata configs and the pins existing in separate constraints files.
But yes, this could probably be brought up in a separate discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm from that camp too. I have a separate environment for every tool. However, most of the tools still need dependencies: pytest for sure, mypy for type-checking of third-party libs usage, pylint for inference-based tasks. In my projects, only flake8 works nicely without knowing the project dependencies. And I keep a separate requirements.txt for it. Yep, makes the root of the project a bit messier but works. This is why I made dephell a long time ago, to handle all this zoo :)

)
@click.option(
"-f",
"--find-links",
Expand Down Expand Up @@ -206,22 +218,23 @@ def cli(
dry_run: bool,
pre: bool,
rebuild: bool,
find_links: Tuple[str],
extras: Tuple[str, ...],
find_links: Tuple[str, ...],
index_url: str,
extra_index_url: Tuple[str],
extra_index_url: Tuple[str, ...],
cert: Optional[str],
client_cert: Optional[str],
trusted_host: Tuple[str],
trusted_host: Tuple[str, ...],
header: bool,
emit_trusted_host: bool,
annotate: bool,
upgrade: bool,
upgrade_packages: Tuple[str],
upgrade_packages: Tuple[str, ...],
output_file: Optional[LazyFile],
allow_unsafe: bool,
generate_hashes: bool,
reuse_hashes: bool,
src_files: Tuple[str],
src_files: Tuple[str, ...],
max_rounds: int,
build_isolation: bool,
emit_find_links: bool,
Expand Down Expand Up @@ -337,6 +350,7 @@ def cli(
###

constraints = []
setup_file_found = False
for src_file in src_files:
is_setup_file = os.path.basename(src_file) in METADATA_FILENAMES
if src_file == "-":
Expand All @@ -360,6 +374,7 @@ def cli(
req.comes_from = comes_from
constraints.extend(reqs)
elif is_setup_file:
setup_file_found = True
dist = meta.load(os.path.dirname(os.path.abspath(src_file)))
comes_from = f"{dist.metadata.get_all('Name')[0]} ({src_file})"
constraints.extend(
Expand All @@ -378,6 +393,10 @@ def cli(
)
)

if not setup_file_found:
msg = "--extra has effect only with setup.py and PEP-517 input formats"
raise click.BadParameter(msg)

primary_packages = {
key_from_ireq(ireq) for ireq in constraints if not ireq.constraint
}
Expand All @@ -387,16 +406,7 @@ def cli(
ireq for key, ireq in upgrade_install_reqs.items() if key in allowed_upgrades
)

# Filter out pip environment markers which do not match (PEP496)
constraints = [
req
for req in constraints
if req.markers is None
# We explicitly set extra=None to filter out optional requirements
# since evaluating an extra marker with no environment raises UndefinedEnvironmentName
# (see https://packaging.pypa.io/en/latest/markers.html#usage)
or req.markers.evaluate({"extra": None})
]
constraints = [req for req in constraints if req_check_markers(req, extras=extras)]
orsinium marked this conversation as resolved.
Show resolved Hide resolved

log.debug("Using indexes:")
with log.indentation():
Expand Down
13 changes: 13 additions & 0 deletions piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,19 @@ def is_pinned_requirement(ireq: InstallRequirement) -> bool:
return spec.operator in {"==", "==="} and not spec.version.endswith(".*")


def req_check_markers(ireq: InstallRequirement, extras: Tuple[str, ...]) -> bool:
"""
1. Check if the environment markers match (PEP-496).
2. Check if the requirement isn't extra or is included into extras to install.
"""
if not ireq.markers or ireq.markers.evaluate({"extra": None}):
return True
for extra in extras:
if ireq.markers.evaluate({"extra": extra}):
return True
return False


def as_tuple(ireq: InstallRequirement) -> Tuple[str, str, Tuple[str, ...]]:
"""
Pulls out the (name: str, version:str, extras:(str)) tuple from
Expand Down
36 changes: 36 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,39 @@ def _make_sdist(package_dir, dist_dir, *args):
return run_setup_file(package_dir, "sdist", "--dist-dir", str(dist_dir), *args)

return _make_sdist


@pytest.fixture
def make_module(tmpdir):
"""
Make a metadata file with the given name and content and a fake module.
"""

def _make_module(fname, content):
path = os.path.join(tmpdir, "sample_lib")
os.mkdir(path)
path = os.path.join(tmpdir, "sample_lib", "__init__.py")
with open(path, "w") as stream:
stream.write("'example module'\n__version__ = '1.2.3'")
path = os.path.join(tmpdir, fname)
with open(path, "w") as stream:
stream.write(dedent(content))
return path

return _make_module


@pytest.fixture
def fake_dists(tmpdir, make_package, make_wheel):
orsinium marked this conversation as resolved.
Show resolved Hide resolved
dists_path = os.path.join(tmpdir, "dists")
pkgs = [
make_package("small-fake-a", version="0.1"),
make_package("small-fake-b", version="0.2"),
make_package("small-fake-c", version="0.3"),
make_package("small-fake-d", version="0.4"),
make_package("small-fake-e", version="0.5"),
make_package("small-fake-f", version="0.6"),
]
for pkg in pkgs:
make_wheel(pkg, dists_path)
return dists_path
Loading