-
Notifications
You must be signed in to change notification settings - Fork 50
feat(bootstrap): add --max-release-age for multiple-versions mode #1089
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
|
|
||
| # 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" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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( | ||
|
|
@@ -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 | ||
|
|
@@ -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" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the age value required?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without Let me know if you think I should make it optional
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| logger.info( | ||
| "multiple versions mode enabled: will bootstrap all matching versions" | ||
| ) | ||
|
|
@@ -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)) | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
@@ -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, | ||
| ) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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] | ||
|
|
||
|
|
||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For providers with Probably fine in practice (PyPI rarely omits 🤖 Prompt for AI Agents |
||
| if filtered: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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 = filteredAt 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 |
||
|
|
||
| # 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( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 publishescertifi 2026.4.30later, but won't catch it. More importantly, if a future build-dep (setuptools) version gets a bump outside the age window, thesetuptools-*.whlcheck 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