Skip to content
Open
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
1 change: 1 addition & 0 deletions e2e/ci_bootstrap_suite.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ run_test "bootstrap_prerelease"
run_test "bootstrap_cache"
run_test "bootstrap_sdist_only"
run_test "bootstrap_multiple_versions"
run_test "bootstrap_max_release_age"

test_section "bootstrap test-mode tests"
run_test "mode_resolution"
Expand Down
98 changes: 98 additions & 0 deletions e2e/test_bootstrap_max_release_age.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#!/bin/bash
# -*- indent-tabs-mode: nil; tab-width: 2; sh-indentation: 2; -*-

# Test bootstrap with --max-release-age flag
# Tests that old versions are filtered out by the max release age window
# and that the filter also applies to build dependencies

SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
source "$SCRIPTDIR/common.sh"

# certifi PyPI upload timeline (actual upload_time from PyPI JSON API):
#
# certifi 2025.11.12 2025-11-12 (should be filtered — too old)
# certifi 2026.1.4 2026-01-04 (should be filtered — too old)
# certifi 2026.2.25 2026-02-25 (should be included — recent enough)
# certifi 2026.4.22 2026-04-22 (should be included — recent enough)
#
# Compute --max-release-age so certifi 2026.2.25 is inside the window
# but certifi 2026.1.4 is outside. We anchor on certifi 2026.2.25's
# upload date and add a buffer.
MAX_AGE=$(python3 -c "
from datetime import date
# Age of certifi 2026.2.25 (uploaded 2026-02-25) + 10 day buffer
age = (date.today() - date(2026, 2, 25)).days + 10
print(age)
")

echo "Using --max-release-age=$MAX_AGE"

fromager \
--log-file="$OUTDIR/bootstrap.log" \
--error-log-file="$OUTDIR/fromager-errors.log" \
--sdists-repo="$OUTDIR/sdists-repo" \
--wheels-repo="$OUTDIR/wheels-repo" \
--work-dir="$OUTDIR/work-dir" \
bootstrap \
--multiple-versions \
--max-release-age="$MAX_AGE" \
'certifi>=2025.11,<=2026.5'

# Verify that recent versions were built (within age window)
echo ""
echo "Checking for expected versions..."
for version in 2026.2.25 2026.4.22; do
if find "$OUTDIR/wheels-repo/downloads/" -name "certifi-$version-*.whl" | grep -q .; then
echo "✓ Found wheel for certifi $version (within max-release-age window)"
else
echo "✗ Missing wheel for certifi $version"
echo "ERROR: certifi $version should be within the max-release-age window"
echo ""
echo "Found wheels:"
find "$OUTDIR/wheels-repo/downloads/" -name 'certifi-*.whl'
exit 1
fi
done
Comment on lines +44 to +55
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Non-deterministic if a newer certifi ships.

The version range 'certifi>=2025.11,<=2026.5' plus the "within window" assertion will still pass if PyPI publishes certifi 2026.4.30 later, but won't catch it. More importantly, if a future build-dep (setuptools) version gets a bump outside the age window, the setuptools-*.whl check on line 90 could start failing. Consider pinning the upper bound tighter (e.g., <=2026.4.22) so the test's expectations stay deterministic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/test_bootstrap_max_release_age.sh` around lines 44 - 55, The test is
non-deterministic because the current requirement range
'certifi>=2025.11,<=2026.5' allows newer releases that can slip past the "within
window" assertions; update the hardcoded versions iterated by the for loop (the
`version` list checked by find "$OUTDIR/wheels-repo/downloads/" for
'certifi-*.whl') and tighten the upper bound in the requirement string to a
fixed cutoff (e.g., change '<=2026.5' to '<=2026.4.22') so the test consistently
expects specific certifi wheels; also ensure any related wheel checks (like the
setuptools check referenced around the later find pattern) use the same
tightened upper bound to keep expectations deterministic.


# Verify that old versions were filtered out
echo ""
echo "Checking that old versions were filtered..."
UNEXPECTED=""
for version in 2025.11.12 2026.1.4; do
if find "$OUTDIR/wheels-repo/downloads/" -name "certifi-$version-*.whl" | grep -q .; then
echo "✗ Found wheel for certifi $version — should have been filtered by max-release-age"
UNEXPECTED="$UNEXPECTED $version"
else
echo "✓ certifi $version correctly filtered out by max-release-age"
fi
done

if [ -n "$UNEXPECTED" ]; then
echo ""
echo "ERROR: --max-release-age should have excluded:$UNEXPECTED"
exit 1
fi

# Verify that max-release-age filtering was applied (check log)
echo ""
echo "Checking log for max-release-age filtering..."
if grep -q "published within.*days" "$OUTDIR/bootstrap.log"; then
echo "✓ Log confirms max-release-age filtering was applied"
else
echo "✗ No max-release-age filtering found in log"
exit 1
fi

# Verify that build dependencies were also resolved within the window
# setuptools is the build dependency for certifi
echo ""
echo "Checking that build dependencies were resolved..."
if find "$OUTDIR/wheels-repo/downloads/" -name "setuptools-*.whl" | grep -q .; then
echo "✓ setuptools was built (build dependency of certifi)"
else
echo "✗ setuptools was not built — build dependency resolution may have failed"
exit 1
fi

echo ""
echo "SUCCESS: --max-release-age correctly filtered old versions and resolved build dependencies"
9 changes: 9 additions & 0 deletions e2e/test_bootstrap_multiple_versions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ cat > "$constraints_file" <<EOF
flit-core>=3.9,<3.12
EOF

# Compute --max-release-age dynamically: days since tomli 2.0.0 was uploaded
# to PyPI (2021-12-13) plus a buffer, so the oldest version is always included.
MAX_AGE=$(python3 -c "
from datetime import date
age = (date.today() - date(2021, 12, 13)).days
print(age + 30)
")

# Use tomli with a version range that matches exactly 3 versions (2.0.0, 2.0.1, 2.0.2)
# tomli has no runtime dependencies, making it fast to bootstrap
# It uses flit-core as build backend, and we allow multiple flit-core versions
Expand All @@ -31,6 +39,7 @@ fromager \
--constraints-file="$constraints_file" \
bootstrap \
--multiple-versions \
--max-release-age="$MAX_AGE" \
'tomli>=2.0,<=2.0.2'

# Check that wheels were built
Expand Down
5 changes: 4 additions & 1 deletion src/fromager/bootstrap_requirement_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ def _resolve(
sdist_server_url=resolver.PYPI_SERVER_URL,
req_type=req_type,
)
return resolver.find_all_matching_from_provider(provider, req)
max_age_cutoff = resolver._compute_max_age_cutoff(self.ctx)
return resolver.find_all_matching_from_provider(
provider, req, max_age_cutoff=max_age_cutoff
)

def get_cached_resolution(
self,
Expand Down
28 changes: 28 additions & 0 deletions src/fromager/commands/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ def _get_requirements_from_args(
default=False,
help="Bootstrap all matching versions instead of only the highest version",
)
@click.option(
"--max-release-age",
"max_release_age",
type=click.IntRange(min=0),
default=0,
help="Reject package versions published more than this many days ago (0 disables the check). Required with --multiple-versions.",
)
@click.argument("toplevel", nargs=-1)
@click.pass_obj
def bootstrap(
Expand All @@ -121,6 +128,7 @@ def bootstrap(
skip_constraints: bool,
test_mode: bool,
multiple_versions: bool,
max_release_age: int,
toplevel: list[str],
) -> None:
"""Compute and build the dependencies of a set of requirements recursively
Expand Down Expand Up @@ -156,6 +164,10 @@ def bootstrap(
)

if multiple_versions:
if max_release_age <= 0:
raise click.UsageError(
"--max-release-age is required when using --multiple-versions"
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.

Why is the age value required?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without --max-release-age, --multiple-versions would attempt to build every version that matches the requirement specifier. For a package like setuptools>=60, that could mean dozens or hundreds of versions going back years. The age limit acts as a guardrail to keep the build scope manageable.

Let me know if you think I should make it optional

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 definitely advisable. I'm not sure I want to make it required. We could require a user to provide some value, but allow 0 as an explicit choice, I guess. Let's see what others think.

)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
logger.info(
"multiple versions mode enabled: will bootstrap all matching versions"
)
Expand All @@ -168,6 +180,13 @@ def bootstrap(
)
skip_constraints = True

if max_release_age > 0:
wkctx.set_max_release_age(max_release_age)
logger.info(
"max release age: rejecting versions older than %d days",
max_release_age,
)

pre_built = wkctx.settings.list_pre_built()
if pre_built:
logger.info("treating %s as pre-built wheels", sorted(pre_built))
Expand Down Expand Up @@ -492,6 +511,13 @@ def write_constraints_file(
default=False,
help="Bootstrap all matching versions instead of only the highest version",
)
@click.option(
"--max-release-age",
"max_release_age",
type=click.IntRange(min=0),
default=0,
help="Reject package versions published more than this many days ago (0 disables the check). Required with --multiple-versions.",
)
@click.argument("toplevel", nargs=-1)
@click.pass_obj
@click.pass_context
Expand All @@ -506,6 +532,7 @@ def bootstrap_parallel(
force: bool,
max_workers: int | None,
multiple_versions: bool,
max_release_age: int,
toplevel: list[str],
) -> None:
"""Bootstrap and build-parallel
Expand Down Expand Up @@ -533,6 +560,7 @@ def bootstrap_parallel(
sdist_only=True,
skip_constraints=skip_constraints,
multiple_versions=multiple_versions,
max_release_age=max_release_age,
toplevel=toplevel,
)

Expand Down
10 changes: 10 additions & 0 deletions src/fromager/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(
settings_dir: pathlib.Path | None = None,
wheel_server_url: str = "",
cooldown: Cooldown | None = None,
max_release_age: datetime.timedelta | None = None,
):
if active_settings is None:
active_settings = packagesettings.Settings(
Expand Down Expand Up @@ -113,6 +114,15 @@ def __init__(
self._parallel_builds = False

self.cooldown: Cooldown | None = cooldown
self._max_release_age: datetime.timedelta | None = max_release_age

@property
def max_release_age(self) -> datetime.timedelta | None:
return self._max_release_age

def set_max_release_age(self, days: int) -> None:
"""Set the maximum release age in days."""
self._max_release_age = datetime.timedelta(days=days)

def enable_parallel_builds(self) -> None:
self._parallel_builds = True
Expand Down
69 changes: 66 additions & 3 deletions src/fromager/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ def resolve(
ignore_platform=ignore_platform,
)
provider.cooldown = resolve_package_cooldown(ctx, req)
results = find_all_matching_from_provider(provider, req)
max_age_cutoff = _compute_max_age_cutoff(ctx)
results = find_all_matching_from_provider(
provider, req, max_age_cutoff=max_age_cutoff
)
return results[0]


Expand Down Expand Up @@ -167,6 +170,24 @@ def resolve_package_cooldown(
)


def _compute_max_age_cutoff(
ctx: context.WorkContext,
) -> datetime.datetime | None:
"""Compute the cutoff time for max release age filtering.

Returns the oldest acceptable upload time, or None if disabled.
Uses the cooldown's bootstrap_time for consistency across a single run.
"""
if ctx.max_release_age is None:
return None
bootstrap_time = (
ctx.cooldown.bootstrap_time
if ctx.cooldown is not None
else datetime.datetime.now(datetime.UTC)
)
return bootstrap_time - ctx.max_release_age


def extract_filename_from_url(url: str) -> str:
"""Extract filename from URL and decode it."""
path = urlparse(url).path
Expand Down Expand Up @@ -203,13 +224,20 @@ def ending(self, state: typing.Any) -> None:


def find_all_matching_from_provider(
provider: BaseProvider, req: Requirement
provider: BaseProvider,
req: Requirement,
max_age_cutoff: datetime.datetime | None = None,
) -> list[tuple[str, Version]]:
"""Find all matching candidates from provider without full dependency resolution.

This function collects ALL candidates that match the requirement, rather than
performing full dependency resolution to find a single best candidate.

Args:
provider: The provider to query for candidates.
req: The requirement to match.
max_age_cutoff: If set, reject candidates published before this time.

Returns list of (url, version) tuples sorted by version (highest first).

IMPORTANT: This bypasses resolvelib's full resolver to collect all matching
Expand Down Expand Up @@ -242,10 +270,45 @@ def find_all_matching_from_provider(
f"Unable to resolve requirement specifier {req} with constraint {constraint} using {provider_desc}: {original_msg}"
) from err

# Materialize candidates so we can iterate more than once if filtering
candidates_list = list(candidates)
logger.info(
"%s: found %d candidate(s) matching %s",
req.name,
len(candidates_list),
req,
)

if max_age_cutoff is not None:
max_age_days = (datetime.datetime.now(datetime.UTC) - max_age_cutoff).days
filtered = [
c
for c in candidates_list
if c.upload_time is None or c.upload_time >= max_age_cutoff
]
Comment on lines +282 to +288
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

upload_time is None handling diverges from is_blocked_by_cooldown.

For providers with supports_upload_time=True (e.g. PyPI), is_blocked_by_cooldown fails closed on missing upload_time (line 684-691), but here we always keep such candidates. If a PyPI candidate genuinely lacks metadata, the cooldown side rejects it while the max-age side admits it — same candidate, opposite verdicts in the same run.

Probably fine in practice (PyPI rarely omits upload_time), but worth aligning — either gate on provider.supports_upload_time here too, or note the intentional asymmetry in the docstring.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/fromager/resolver.py` around lines 276 - 281, The max-age filter
currently keeps candidates with upload_time None which diverges from
is_blocked_by_cooldown's behavior for providers that declare
supports_upload_time=True; update the max-age filtering logic (the block using
max_age_cutoff and candidates_list) to only treat missing upload_time as
acceptable when the candidate.provider.supports_upload_time is False (i.e., if
provider.supports_upload_time is True, treat upload_time None as failing the
max-age check), or alternatively add a clear docstring comment on the max-age
filter explaining the intentional asymmetry; reference max_age_cutoff,
candidates_list, upload_time, and is_blocked_by_cooldown when making the change
so the behavior is aligned or explicitly documented.

if filtered:
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.

We're going to want some structure to the logging as we add more filtering features. So maybe we log the full number of versions found, then for each filter step how many remain and the criteria used to do the filtering. Something like this, for example:

INFO have 100 versions of foo>=1.0.0
INFO have 10 version of foo>=1.0.0 older than 30 days
INFO have 1 version of foo>=1.0.0 at least 14 days old

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

makes sense. I have attempted to add some structure to logging in my latest push

logger.info(
"%s: have %d candidate(s) of %s published within %d days",
req.name,
len(filtered),
req,
max_age_days,
)
candidates_list = filtered
else:
logger.warning(
"%s: all %d candidate(s) of %s are older than %d days, "
"keeping all to avoid empty resolution",
req.name,
len(candidates_list),
req,
max_age_days,
)
Comment on lines +298 to +306
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent fallback defeats the filter's purpose.

When every candidate is older than the cutoff, the filter is bypassed and all old versions are returned anyway. A user who set --max-release-age=N explicitly asked not to build those. Falling back silently (with only a warning log) can lead to building the very stale releases the flag was meant to exclude — the exact problem issue #1037 calls out for packages like setuptools.

Consider failing loudly instead, so the caller decides:

🔧 Suggested change
-        if filtered:
-            dropped = len(candidates_list) - len(filtered)
-            if dropped > 0:
-                logger.info(
-                    "%s: filtered %d/%d versions by max release age",
-                    req.name,
-                    dropped,
-                    len(candidates_list),
-                )
-            candidates_list = filtered
-        else:
-            logger.warning(
-                "%s: all %d versions are older than max release age cutoff, "
-                "keeping all versions to avoid empty resolution",
-                req.name,
-                len(candidates_list),
-            )
+        dropped = len(candidates_list) - len(filtered)
+        if dropped:
+            logger.info(
+                "%s: filtered %d/%d versions by max release age",
+                req.name,
+                dropped,
+                len(candidates_list),
+            )
+        if not filtered:
+            raise resolvelib.resolvers.ResolverException(
+                f"all {len(candidates_list)} candidate(s) for {req} were "
+                f"published before the max-release-age cutoff"
+            )
+        candidates_list = filtered

At minimum, this is worth a design-intent note in the function docstring so reviewers/users understand the deliberate "fail open" behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/fromager/resolver.py` around lines 292 - 298, The filter that checks
release age currently falls back to keeping all candidates and only logs a
warning, which defeats the --max-release-age filter; in the resolver.py function
that filters candidates by max release age (the block referencing req.name and
candidates_list) change the behavior to fail loudly: instead of returning all
candidates when every candidate is older than the cutoff, raise a clear
exception (e.g., a ValueError or a custom ResolutionError) so callers can decide
how to proceed; additionally, update the function docstring to state the
explicit "fail on cutoff" policy and mention the raised exception type so
callers know to catch it.


# Convert candidates to list of (url, version) tuples
# Candidates are sorted by version (highest first) by BaseProvider.find_matches()
# which calls sorted(candidates, key=attrgetter("version", "build_tag"), reverse=True)
return [(candidate.url, candidate.version) for candidate in candidates]
return [(c.url, c.version) for c in candidates_list]


def get_project_from_pypi(
Expand Down
5 changes: 4 additions & 1 deletion src/fromager/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ def resolve_source(
)

# Get all matching candidates from provider
results = resolver.find_all_matching_from_provider(provider, req)
max_age_cutoff = resolver._compute_max_age_cutoff(ctx)
results = resolver.find_all_matching_from_provider(
provider, req, max_age_cutoff=max_age_cutoff
)

# Return highest version (first in sorted list)
url, version = results[0]
Expand Down
Loading
Loading