Skip to content

Commit 5378a6e

Browse files
authored
Merge pull request #12194 from cosmicexplorer/build-tracker-comments
Add lots of comments on the function of BuildTracker
2 parents d8cd93f + 4a853ea commit 5378a6e

File tree

7 files changed

+71
-23
lines changed

7 files changed

+71
-23
lines changed

news/12194.trivial.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add lots of comments to the ``BuildTracker``.

src/pip/_internal/distributions/base.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import abc
2+
from typing import Optional
23

34
from pip._internal.index.package_finder import PackageFinder
45
from pip._internal.metadata.base import BaseDistribution
@@ -19,12 +20,23 @@ class AbstractDistribution(metaclass=abc.ABCMeta):
1920
2021
- we must be able to create a Distribution object exposing the
2122
above metadata.
23+
24+
- if we need to do work in the build tracker, we must be able to generate a unique
25+
string to identify the requirement in the build tracker.
2226
"""
2327

2428
def __init__(self, req: InstallRequirement) -> None:
2529
super().__init__()
2630
self.req = req
2731

32+
@abc.abstractproperty
33+
def build_tracker_id(self) -> Optional[str]:
34+
"""A string that uniquely identifies this requirement to the build tracker.
35+
36+
If None, then this dist has no work to do in the build tracker, and
37+
``.prepare_distribution_metadata()`` will not be called."""
38+
raise NotImplementedError()
39+
2840
@abc.abstractmethod
2941
def get_metadata_distribution(self) -> BaseDistribution:
3042
raise NotImplementedError()

src/pip/_internal/distributions/installed.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Optional
2+
13
from pip._internal.distributions.base import AbstractDistribution
24
from pip._internal.index.package_finder import PackageFinder
35
from pip._internal.metadata import BaseDistribution
@@ -10,6 +12,10 @@ class InstalledDistribution(AbstractDistribution):
1012
been computed.
1113
"""
1214

15+
@property
16+
def build_tracker_id(self) -> Optional[str]:
17+
return None
18+
1319
def get_metadata_distribution(self) -> BaseDistribution:
1420
assert self.req.satisfied_by is not None, "not actually installed"
1521
return self.req.satisfied_by

src/pip/_internal/distributions/sdist.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import logging
2-
from typing import Iterable, Set, Tuple
2+
from typing import Iterable, Optional, Set, Tuple
33

44
from pip._internal.build_env import BuildEnvironment
55
from pip._internal.distributions.base import AbstractDistribution
@@ -18,6 +18,12 @@ class SourceDistribution(AbstractDistribution):
1818
generated, either using PEP 517 or using the legacy `setup.py egg_info`.
1919
"""
2020

21+
@property
22+
def build_tracker_id(self) -> Optional[str]:
23+
"""Identify this requirement uniquely by its link."""
24+
assert self.req.link
25+
return self.req.link.url_without_fragment
26+
2127
def get_metadata_distribution(self) -> BaseDistribution:
2228
return self.req.get_dist()
2329

src/pip/_internal/distributions/wheel.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Optional
2+
13
from pip._vendor.packaging.utils import canonicalize_name
24

35
from pip._internal.distributions.base import AbstractDistribution
@@ -15,6 +17,10 @@ class WheelDistribution(AbstractDistribution):
1517
This does not need any preparation as wheels can be directly unpacked.
1618
"""
1719

20+
@property
21+
def build_tracker_id(self) -> Optional[str]:
22+
return None
23+
1824
def get_metadata_distribution(self) -> BaseDistribution:
1925
"""Loads the metadata from the wheel file into memory and returns a
2026
Distribution that uses it, not relying on the wheel file or

src/pip/_internal/operations/build/build_tracker.py

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,22 @@ def get_build_tracker() -> Generator["BuildTracker", None, None]:
5151
yield tracker
5252

5353

54+
class TrackerId(str):
55+
"""Uniquely identifying string provided to the build tracker."""
56+
57+
5458
class BuildTracker:
59+
"""Ensure that an sdist cannot request itself as a setup requirement.
60+
61+
When an sdist is prepared, it identifies its setup requirements in the
62+
context of ``BuildTracker.track()``. If a requirement shows up recursively, this
63+
raises an exception.
64+
65+
This stops fork bombs embedded in malicious packages."""
66+
5567
def __init__(self, root: str) -> None:
5668
self._root = root
57-
self._entries: Set[InstallRequirement] = set()
69+
self._entries: Dict[TrackerId, InstallRequirement] = {}
5870
logger.debug("Created build tracker: %s", self._root)
5971

6072
def __enter__(self) -> "BuildTracker":
@@ -69,16 +81,15 @@ def __exit__(
6981
) -> None:
7082
self.cleanup()
7183

72-
def _entry_path(self, link: Link) -> str:
73-
hashed = hashlib.sha224(link.url_without_fragment.encode()).hexdigest()
84+
def _entry_path(self, key: TrackerId) -> str:
85+
hashed = hashlib.sha224(key.encode()).hexdigest()
7486
return os.path.join(self._root, hashed)
7587

76-
def add(self, req: InstallRequirement) -> None:
88+
def add(self, req: InstallRequirement, key: TrackerId) -> None:
7789
"""Add an InstallRequirement to build tracking."""
7890

79-
assert req.link
8091
# Get the file to write information about this requirement.
81-
entry_path = self._entry_path(req.link)
92+
entry_path = self._entry_path(key)
8293

8394
# Try reading from the file. If it exists and can be read from, a build
8495
# is already in progress, so a LookupError is raised.
@@ -92,33 +103,37 @@ def add(self, req: InstallRequirement) -> None:
92103
raise LookupError(message)
93104

94105
# If we're here, req should really not be building already.
95-
assert req not in self._entries
106+
assert key not in self._entries
96107

97108
# Start tracking this requirement.
98109
with open(entry_path, "w", encoding="utf-8") as fp:
99110
fp.write(str(req))
100-
self._entries.add(req)
111+
self._entries[key] = req
101112

102113
logger.debug("Added %s to build tracker %r", req, self._root)
103114

104-
def remove(self, req: InstallRequirement) -> None:
115+
def remove(self, req: InstallRequirement, key: TrackerId) -> None:
105116
"""Remove an InstallRequirement from build tracking."""
106117

107-
assert req.link
108-
# Delete the created file and the corresponding entries.
109-
os.unlink(self._entry_path(req.link))
110-
self._entries.remove(req)
118+
# Delete the created file and the corresponding entry.
119+
os.unlink(self._entry_path(key))
120+
del self._entries[key]
111121

112122
logger.debug("Removed %s from build tracker %r", req, self._root)
113123

114124
def cleanup(self) -> None:
115-
for req in set(self._entries):
116-
self.remove(req)
125+
for key, req in list(self._entries.items()):
126+
self.remove(req, key)
117127

118128
logger.debug("Removed build tracker: %r", self._root)
119129

120130
@contextlib.contextmanager
121-
def track(self, req: InstallRequirement) -> Generator[None, None, None]:
122-
self.add(req)
131+
def track(self, req: InstallRequirement, key: str) -> Generator[None, None, None]:
132+
"""Ensure that `key` cannot install itself as a setup requirement.
133+
134+
:raises LookupError: If `key` was already provided in a parent invocation of
135+
the context introduced by this method."""
136+
tracker_id = TrackerId(key)
137+
self.add(req, tracker_id)
123138
yield
124-
self.remove(req)
139+
self.remove(req, tracker_id)

src/pip/_internal/operations/prepare.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,12 @@ def _get_prepared_distribution(
6565
) -> BaseDistribution:
6666
"""Prepare a distribution for installation."""
6767
abstract_dist = make_distribution_for_install_requirement(req)
68-
with build_tracker.track(req):
69-
abstract_dist.prepare_distribution_metadata(
70-
finder, build_isolation, check_build_deps
71-
)
68+
tracker_id = abstract_dist.build_tracker_id
69+
if tracker_id is not None:
70+
with build_tracker.track(req, tracker_id):
71+
abstract_dist.prepare_distribution_metadata(
72+
finder, build_isolation, check_build_deps
73+
)
7274
return abstract_dist.get_metadata_distribution()
7375

7476

0 commit comments

Comments
 (0)