Skip to content

Commit c0a7570

Browse files
authored
🐛 fix(extras): scope optional deps to requesting edge (#608)
Running `pipdeptree --extras explicit` in an environment where one dependency asks for `urllib3[socks]` printed `PySocks` under every `urllib3` in the tree, including the `requests` and `botocore` branches that only ask for a plain `urllib3`. 🔁 The reverse view had the mirror problem: `pipdeptree -r -p PySocks` walked out to every `urllib3` dependent rather than the lone `selenium` that pulled in the `socks` extra. Closes #607. A `PackageDAG` keys by node, so `urllib3` is a single node with one shared child list, while an extra belongs to the edge that requested it. Resolving `urllib3[socks]` appended `PySocks` to that shared list, and since the renderers look children up by node key it leaked onto every parent. An extra edge is path-dependent and a node-keyed graph has nowhere to store that, so the fix records on each optional edge the extra that gates it and applies the gate while walking children. Forward, the walk drops a scoped edge unless the requiring parent asked for the extra; in reverse, once a descent passes through an extra only the dependents that asked for it survive. Extras that belong to the package rather than one edge, such as `--extras active` or an explicit `--packages foo[extra]`, carry no gate and show everywhere. A text render revisits a node once per path that reaches it, so the gate consults a cached set of the nodes that carry a scoped extra and leaves every other node on its original allocation-free lookup. Package builds without scoped extras keep their previous cost. One deliberate choice worth a look: a package listed at the top level still shows its extra dependencies, since nothing gates a root and the edge exists. Only the nested occurrences under a non-requesting parent lose the spurious child.
1 parent 5237236 commit c0a7570

7 files changed

Lines changed: 204 additions & 36 deletions

File tree

src/pipdeptree/_models/dag.py

Lines changed: 114 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from collections.abc import Iterator, Mapping
66
from enum import Enum, auto
77
from fnmatch import fnmatch
8+
from functools import cached_property
89
from itertools import chain
910
from typing import TYPE_CHECKING, Literal
1011

@@ -130,16 +131,38 @@ def get_node_as_parent(self, node_key: str) -> DistPackage | None:
130131
except KeyError:
131132
return None
132133

133-
def get_children(self, node_key: str) -> list[ReqPackage]:
134+
def get_children(self, node_key: str, parent: DistPackage | ReqPackage | None = None) -> list[ReqPackage]:
134135
"""
135136
Get child nodes for a node by its key.
136137
137138
:param node_key: key of the node to get children of
139+
:param parent: the edge the node was reached through; gates edge-scoped extras so ``PySocks`` shows only
140+
beneath the ``urllib3[socks]`` that asked for it. ``None`` skips gating, which suits top-level nodes and
141+
the graph renderers
138142
:returns: child nodes
139143
140144
"""
141145
node = self.get_node_as_parent(node_key)
142-
return self._obj[node] if node else []
146+
if node is None:
147+
return []
148+
children = self._obj[node]
149+
if isinstance(parent, ReqPackage):
150+
if node_key in self._scoped_parent_keys:
151+
return _gate_requested(children, parent.requested_extras)
152+
return children
153+
if isinstance(parent, DistPackage) and parent.req is not None and (scoped := parent.req.scoped_extra):
154+
return _gate_dependents(children, scoped)
155+
return children
156+
157+
@cached_property
158+
def _scoped_parent_keys(self) -> frozenset[str]:
159+
# Node keys whose children include an edge-scoped extra; only these pay the forward-gating cost. Rendering
160+
# revisits a node once per path, so skipping the per-call allocation for the common ungated node matters.
161+
return frozenset(
162+
node.key
163+
for node, children in self._obj.items()
164+
if any(isinstance(c, ReqPackage) and c.scoped_extra for c in children)
165+
)
143166

144167
def filter_nodes( # noqa: C901, PLR0912
145168
self,
@@ -373,6 +396,28 @@ def reverse(self) -> PackageDAG: # ty: ignore[invalid-method-override]
373396
return PackageDAG(dict(forward_dag))
374397

375398

399+
def _gate_requested(children: list[ReqPackage], requested_extras: frozenset[str]) -> list[ReqPackage]:
400+
"""Forward gate: keep an extra-scoped edge only when the requiring edge asked for that extra."""
401+
requested = {canonicalize_name(e) for e in requested_extras}
402+
return [
403+
c
404+
for c in children
405+
if not isinstance(c, ReqPackage) or c.scoped_extra is None or canonicalize_name(c.scoped_extra) in requested
406+
]
407+
408+
409+
def _gate_dependents(children: list[ReqPackage], extra: str) -> list[ReqPackage]:
410+
"""Reverse gate: having descended into a package through ``extra``, keep only dependents that asked for it."""
411+
wanted = canonicalize_name(extra)
412+
return [
413+
c
414+
for c in children
415+
if not isinstance(c, DistPackage)
416+
or c.req is None
417+
or wanted in {canonicalize_name(e) for e in c.req.requested_extras}
418+
]
419+
420+
376421
def _expand_requested_extras(
377422
idx: dict[str, DistPackage], requested_extras: Mapping[str, set[str]] | None
378423
) -> dict[str, set[str]]:
@@ -396,46 +441,84 @@ def _resolve_extras(
396441
requested_extras: dict[str, set[str]] | None = None,
397442
) -> None:
398443
"""Add extra/optional dependencies to the DAG in-place."""
399-
extras_needed = _seed_extras(pkg_deps, idx, extras)
444+
seed, unconditional = _seed_extras(pkg_deps, idx, extras)
400445
for key, wanted in (requested_extras or {}).items():
401-
extras_needed.setdefault(key, set()).update(wanted)
402-
processed: dict[str, set[str]] = {}
403-
# The same (parent, child, extra) triple can be reached through multiple req.extras propagation
404-
# paths across rounds; without dedup it would be appended once per path.
405-
seen_edges: set[tuple[str, str, str]] = set()
406-
while extras_needed:
407-
next_round: dict[str, set[str]] = {}
408-
for pkg_key, wanted in extras_needed.items():
409-
new_extras = wanted - processed.get(pkg_key, set())
410-
if not new_extras:
446+
seed.setdefault(key, set()).update(wanted)
447+
# ``--packages foo[extra]`` is a direct request, so surface it wherever ``foo`` appears.
448+
unconditional.update((key, extra) for extra in wanted)
449+
_ExtrasExpander(pkg_deps, idx, unconditional).expand(seed)
450+
451+
452+
class _ExtrasExpander:
453+
"""
454+
Append extra/optional dependencies to a node-keyed DAG, breadth-first over ``req.extras`` chains.
455+
456+
Each appended edge records the ``scoped_extra`` gating it, unless its ``(pkg_key, extra)`` is unconditional
457+
(active-satisfied or ``--packages`` requested). The renderer reads that tag to place an optional dependency only
458+
beneath the parent that asked for the extra instead of under every occurrence of the provider.
459+
"""
460+
461+
def __init__(
462+
self,
463+
pkg_deps: dict[DistPackage, list[ReqPackage]],
464+
idx: dict[str, DistPackage],
465+
unconditional: set[tuple[str, str]],
466+
) -> None:
467+
self._pkg_deps = pkg_deps
468+
self._idx = idx
469+
self._unconditional = unconditional
470+
# The same (parent, child, extra) triple can be reached through multiple req.extras propagation paths across
471+
# rounds; without dedup it would be appended once per path.
472+
self._seen_edges: set[tuple[str, str, str]] = set()
473+
self._processed: dict[str, set[str]] = {}
474+
475+
def expand(self, pending: dict[str, set[str]]) -> None:
476+
while pending:
477+
next_round: dict[str, set[str]] = {}
478+
for pkg_key, wanted in pending.items():
479+
if new_extras := wanted - self._processed.get(pkg_key, set()):
480+
self._processed.setdefault(pkg_key, set()).update(new_extras)
481+
self._attach(pkg_key, new_extras, next_round)
482+
pending = next_round
483+
484+
def _attach(self, pkg_key: str, new_extras: set[str], next_round: dict[str, set[str]]) -> None:
485+
dist_pkg = self._idx.get(pkg_key)
486+
if dist_pkg is None or dist_pkg not in self._pkg_deps:
487+
return
488+
for req, extra_name, dep_key in dist_pkg.requires_for_extras(frozenset(new_extras)):
489+
if (dist := self._idx.get(dep_key)) is None:
411490
continue
412-
processed.setdefault(pkg_key, set()).update(new_extras)
413-
dist_pkg = idx.get(pkg_key)
414-
if dist_pkg is None or dist_pkg not in pkg_deps:
491+
edge_key = (dist_pkg.key, dist.key, extra_name)
492+
if edge_key in self._seen_edges:
415493
continue
416-
for req, extra_name, dep_key in dist_pkg.requires_for_extras(frozenset(new_extras)):
417-
if (dist := idx.get(dep_key)) is None:
418-
continue
419-
edge_key = (dist_pkg.key, dist.key, extra_name)
420-
if edge_key in seen_edges:
421-
continue
422-
seen_edges.add(edge_key)
423-
req.name = dist.project_name
424-
pkg_deps[dist_pkg].append(ReqPackage(req, dist, extra=extra_name))
425-
if req.extras:
426-
next_round.setdefault(dist.key, set()).update(req.extras)
427-
extras_needed = next_round
494+
self._seen_edges.add(edge_key)
495+
req.name = dist.project_name
496+
unconditional = (pkg_key, extra_name) in self._unconditional
497+
scoped_extra = None if unconditional else extra_name
498+
self._pkg_deps[dist_pkg].append(ReqPackage(req, dist, extra=extra_name, scoped_extra=scoped_extra))
499+
if req.extras:
500+
next_round.setdefault(dist.key, set()).update(req.extras)
501+
if unconditional:
502+
# An unconditional extra pulls its own sub-extras unconditionally too.
503+
self._unconditional.update((dist.key, sub) for sub in req.extras)
428504

429505

430506
def _seed_extras(
431507
pkg_deps: dict[DistPackage, list[ReqPackage]], idx: dict[str, DistPackage], extras: ExtrasMode
432-
) -> dict[str, set[str]]:
433-
"""Collect the extras to resolve: requested extras, plus satisfiable ones in active mode."""
508+
) -> tuple[dict[str, set[str]], set[tuple[str, str]]]:
509+
"""
510+
Collect the extras to resolve: explicit ``name[extra]`` requests, plus satisfiable ones in active mode.
511+
512+
Returns the extras keyed by package and the set of ``(pkg_key, extra)`` pairs that are unconditional, i.e.
513+
active-satisfied rather than gated behind a specific requiring edge.
514+
"""
434515
extras_needed = _collect_explicit_extras(pkg_deps)
516+
unconditional: set[tuple[str, str]] = set()
435517
if extras == "active":
436518
for pkg_key, satisfied in _collect_satisfied_extras(pkg_deps, idx).items():
437519
extras_needed.setdefault(pkg_key, set()).update(satisfied)
438-
return extras_needed
520+
unconditional.update((pkg_key, extra) for extra in satisfied)
521+
return extras_needed, unconditional
439522

440523

441524
def _collect_explicit_extras(pkg_deps: dict[DistPackage, list[ReqPackage]]) -> dict[str, set[str]]:

src/pipdeptree/_models/package.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,22 @@ class ReqPackage(Package):
258258

259259
UNKNOWN_VERSION = "?"
260260

261-
def __init__(self, obj: Requirement, dist: DistPackage | None = None, extra: str | None = None) -> None:
261+
def __init__(
262+
self,
263+
obj: Requirement,
264+
dist: DistPackage | None = None,
265+
extra: str | None = None,
266+
*,
267+
scoped_extra: str | None = None,
268+
) -> None:
262269
super().__init__(obj.name)
263270
self._obj = obj
264271
self.dist = dist
265272
self.extra = extra
273+
# The extra this edge is gated behind, or None to surface it everywhere. Set only for an optional dependency
274+
# reached through an explicit ``name[extra]`` request (e.g. ``urllib3[socks]``); extras pulled in by
275+
# ``--extras active`` or ``--packages foo[extra]`` stay None. PackageDAG.get_children reads it to gate output.
276+
self.scoped_extra = scoped_extra
266277

267278
def render_as_root(self, *, frozen: bool) -> str:
268279
if not frozen:
@@ -287,6 +298,11 @@ def version_spec(self) -> str | None:
287298
specs = sorted(map(str, self._obj.specifier), reverse=True) # `reverse` makes '>' prior to '<'
288299
return ",".join(specs) if specs else None
289300

301+
@property
302+
def requested_extras(self) -> frozenset[str]:
303+
"""Extras the requiring package asked for on this edge (e.g. ``urllib3[socks]`` -> ``{'socks'}``)."""
304+
return frozenset(self._obj.extras)
305+
290306
@property
291307
def edge_label(self) -> str:
292308
version = self.version_spec or "any"

src/pipdeptree/_render/json_tree.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def aux(
6464

6565
d["dependencies"] = [
6666
aux(c, parent=node, cur_chain=[*cur_chain, c.project_name])
67-
for c in tree.get_children(node.key)
67+
for c in tree.get_children(node.key, node)
6868
if c.project_name not in cur_chain
6969
]
7070

src/pipdeptree/_render/rich_text.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def _build_tree( # noqa: PLR0913
7070
if depth >= max_depth:
7171
return
7272

73-
children = tree.get_children(node.key)
73+
children = tree.get_children(node.key, node)
7474
for child in children:
7575
if child.project_name in cur_chain:
7676
continue

src/pipdeptree/_render/text.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ def aux( # noqa: PLR0913, PLR0917
101101

102102
result = [node_str]
103103

104-
children = tree.get_children(node.key)
104+
children = tree.get_children(node.key, node)
105105
children_strings = [
106106
aux(
107107
c,
@@ -151,7 +151,7 @@ def aux(
151151
result = [node_str]
152152
children = [
153153
aux(c, node, indent=indent + 2, cur_chain=[*cur_chain, c.project_name], depth=depth + 1)
154-
for c in tree.get_children(node.key)
154+
for c in tree.get_children(node.key, node)
155155
if c.project_name not in cur_chain and depth + 1 <= max_depth
156156
]
157157
result += list(chain.from_iterable(children))

tests/_models/test_dag.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,44 @@ def test_dag_extras_annotates_extra_name(make_mock_dist: MockDistMaker) -> None:
306306
assert all(dep.extra == "signedtoken" for dep in extra_deps)
307307

308308

309+
def _build_socks_dag(make_mock_dist: MockDistMaker, *, extras: ExtrasMode = "explicit") -> PackageDAG:
310+
pkgs = [
311+
make_mock_dist("selenium", "4.35.0", requires=["urllib3[socks]>=2.5.0,<3.0"]),
312+
make_mock_dist("requests", "2.33.1", requires=["urllib3>=1.26,<3"]),
313+
make_mock_dist("urllib3", "2.7.0", requires=["pysocks ; extra == 'socks'"], provides_extras=["socks"]),
314+
make_mock_dist("pysocks", "1.7.1"),
315+
]
316+
return PackageDAG.from_pkgs(pkgs, extras=extras)
317+
318+
319+
def _edge_into(dag: PackageDAG, parent_key: str, child_key: str) -> ReqPackage:
320+
return next(dep for dep in dag.get_children(parent_key) if dep.key == child_key)
321+
322+
323+
@pytest.mark.parametrize(
324+
("extras", "via", "shown"),
325+
[
326+
pytest.param("explicit", "requests", False, id="explicit-non-requesting-parent-hides"),
327+
pytest.param("explicit", "selenium", True, id="explicit-requesting-parent-shows"),
328+
pytest.param("explicit", None, True, id="explicit-no-parent-shows-all"),
329+
pytest.param("active", "requests", True, id="active-shows-under-every-parent"),
330+
],
331+
)
332+
def test_dag_edge_scoped_extra_visibility(
333+
make_mock_dist: MockDistMaker, extras: ExtrasMode, via: str | None, shown: bool
334+
) -> None:
335+
dag = _build_socks_dag(make_mock_dist, extras=extras)
336+
parent = _edge_into(dag, via, "urllib3") if via else None
337+
assert ("pysocks" in {c.key for c in dag.get_children("urllib3", parent)}) is shown
338+
339+
340+
def test_dag_reverse_edge_scoped_extra_keeps_only_requesting_dependent(make_mock_dist: MockDistMaker) -> None:
341+
reversed_dag = _build_socks_dag(make_mock_dist).reverse()
342+
urllib3_dist = next(d for d in reversed_dag.get_children("pysocks") if d.key == "urllib3")
343+
dependents = {d.key for d in reversed_dag.get_children("urllib3", urllib3_dist)}
344+
assert dependents == {"selenium"}
345+
346+
309347
def _build_requested_extras_pkgs(make_mock_dist: MockDistMaker) -> list[Distribution]:
310348
return [
311349
make_mock_dist(

tests/render/test_text.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,3 +605,34 @@ def test_render_text_with_extras(
605605
render_text(dag, max_depth=float("inf"), encoding=encoding)
606606
output = capsys.readouterr().out
607607
assert "extra: signedtoken" in output
608+
609+
610+
def _socks_dag(make_mock_dist: MockDistMaker) -> PackageDAG:
611+
pkgs = [
612+
make_mock_dist("selenium", "4.35.0", requires=["urllib3[socks]>=2.5.0,<3.0"]),
613+
make_mock_dist("requests", "2.33.1", requires=["urllib3>=1.26,<3"]),
614+
make_mock_dist("urllib3", "2.7.0", requires=["pysocks ; extra == 'socks'"], provides_extras=["socks"]),
615+
make_mock_dist("pysocks", "1.7.1"),
616+
]
617+
return PackageDAG.from_pkgs(pkgs, extras="explicit")
618+
619+
620+
def test_render_text_edge_scoped_extra_only_under_requesting_parent(
621+
capsys: pytest.CaptureFixture[str], make_mock_dist: MockDistMaker
622+
) -> None:
623+
render_text(_socks_dag(make_mock_dist), max_depth=float("inf"), encoding="utf-8", list_all=False)
624+
lines = capsys.readouterr().out.splitlines()
625+
requests_block = lines[lines.index("requests==2.33.1") : lines.index("selenium==4.35.0")]
626+
selenium_block = lines[lines.index("selenium==4.35.0") :]
627+
assert not any("pysocks" in line for line in requests_block)
628+
assert any("pysocks" in line for line in selenium_block)
629+
630+
631+
def test_render_text_reverse_edge_scoped_extra_prunes_unrelated_dependents(
632+
capsys: pytest.CaptureFixture[str], make_mock_dist: MockDistMaker
633+
) -> None:
634+
reversed_dag = _socks_dag(make_mock_dist).reverse().filter_nodes(["pysocks"], None)
635+
render_text(reversed_dag, max_depth=float("inf"), encoding="utf-8", list_all=False)
636+
output = capsys.readouterr().out
637+
assert "selenium" in output
638+
assert "requests" not in output

0 commit comments

Comments
 (0)