Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
7 changes: 0 additions & 7 deletions e2e/constraints.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,2 @@
# This file is here in case we need to quickly add a constraint to
# fix CI jobs.

# setuptools-scm 10.0.2 added a build-system dependency on
# vcs-versioning, which itself requires hatchling. This creates a
# circular bootstrap dependency that causes e2e tests to fail because
# hatchling's wheel is not yet available on the local wheel server
# when vcs-versioning tries to install it. See #982.
setuptools-scm<10
16 changes: 9 additions & 7 deletions src/fromager/bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
logger = logging.getLogger(__name__)

# package name, extras, version, sdist/wheel
SeenKey = tuple[NormalizedName, tuple[str, ...], str, typing.Literal["sdist", "wheel"]]
SeenKey = tuple[NormalizedName, str, typing.Literal["sdist", "wheel"]]


@dataclasses.dataclass
Expand Down Expand Up @@ -1287,9 +1287,13 @@ def _sort_requirements(
def _resolved_key(
self, req: Requirement, version: Version, typ: typing.Literal["sdist", "wheel"]
) -> SeenKey:
"""Return a key for tracking whether a package has already been resolved.
Extras are intentionally excluded because a build is the same
regardless of extras.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not the same at all. That's the point of extras. If we don't include extras here then building torch appears to be the same as building torch[cuda] except it does not include the cuda dependencies.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. Going to close this PR.

"""
return (
canonicalize_name(req.name),
tuple(sorted(req.extras)),
str(version),
typ,
)
Expand Down Expand Up @@ -1331,11 +1335,9 @@ def _add_to_build_order(
prebuilt: bool = False,
constraint: Requirement | None = None,
) -> None:
# We only care if this version of this package has been built,
# and don't want to trigger building it twice. The "extras"
# value, included in the _resolved_key() output, can confuse
# that so we ignore itand build our own key using just the
# name and version.
# Deduplicate by (name, version) only. We don't distinguish
# sdist vs wheel here because a package should appear in the
# build order at most once per version.
key = (canonicalize_name(req.name), str(version))
if key in self._build_requirements:
return
Expand Down
20 changes: 17 additions & 3 deletions tests/test_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,32 @@ def test_seen(tmp_context: WorkContext) -> None:


def test_seen_extras(tmp_context: WorkContext) -> None:
"""Verify that extras are ignored for seen-checks.

A wheel build is the same regardless of extras, so
``testdist`` and ``testdist[extra]`` must be treated as
the same package.
"""
req1 = Requirement("testdist")
req2 = Requirement("testdist[extra]")
version = Version("1.2")
bt = bootstrapper.Bootstrapper(tmp_context)
assert not bt._has_been_seen(req1, version)
bt._mark_as_seen(req1, version)
assert bt._has_been_seen(req1, version)
assert not bt._has_been_seen(req2, version)
bt._mark_as_seen(req2, version)
assert bt._has_been_seen(req1, version)
# Same package with extras should also be seen
assert bt._has_been_seen(req2, version)

# Reverse: mark with extras, check without
bt2 = bootstrapper.Bootstrapper(tmp_context)
bt2._mark_as_seen(req2, version)
assert bt2._has_been_seen(req1, version)

# Extras are also ignored for sdist_only path
bt3 = bootstrapper.Bootstrapper(tmp_context)
bt3._mark_as_seen(req1, version, sdist_only=True)
assert bt3._has_been_seen(req2, version, sdist_only=True)


def test_seen_name_canonicalization(tmp_context: WorkContext) -> None:
req = Requirement("flit_core")
Expand Down
Loading