feat: auto-cap setuptools when setup.py uses removed APIs

[vshawrh]

Summary

• Auto-detect pkg_resources imports and dry_run keyword arguments in setup.py via AST parsing
• Append setuptools<82 or setuptools<81 to build-system requirements when removed APIs are detected
• No cap added for packages without setup.py or clean setup.py files

Closes #1263

Motivation

setuptools 81 removed distutils.spawn(dry_run=...) and distutils.dir_util.remove_tree(dry_run=...). setuptools 82 removed pkg_resources entirely. Many packages on PyPI still use these in their setup.py, causing build failures when Fromager resolves an uncapped setuptools.

Currently every Fromager deployment must handle this per-package, either via a plugin overriding get_build_system_dependencies or via project_override.update_build_requires. In the AIPCC wheels builder, 22 identical plugins exist solely to add this constraint, and 127 additional packages have no protection at all.

Implementation

The change adds a _get_setuptools_constraint() function to dependencies.py that:

1. Checks if setup.py exists in sdist_root_dir
2. Parses it with ast.parse (avoids false positives from comments/strings)
3. Walks the AST looking for pkg_resources imports and dry_run= keyword args
4. Returns the tighter constraint when both apply

default_get_build_system_dependencies calls this function and appends the constraint when needed.

Regression analysis

Scenario	Impact
Packages with no plugin	Gain automatic protection (net positive)
Packages with tighter update_build_requires (e.g., setuptools<80)	pip resolves to more restrictive cap. No conflict
Plugins that call default_get_build_system_dependencies internally	Inherit the cap automatically. Harmless duplicate if they also add their own
Plugins that fully replace the hook	Unaffected (default never called)
Packages without setup.py	No cap added. No change
build-system-requirements.txt cache	Generated fresh each build. No stale cache risk

Test plan

• test_pkg_resources_import -- detects import pkg_resources
• test_pkg_resources_from_import -- detects from pkg_resources import ...
• test_pkg_resources_submodule -- detects import pkg_resources.extern
• test_dry_run_keyword -- detects dry_run= keyword arg
• test_both_returns_tighter -- returns setuptools<81 when both found
• test_clean_setup_py -- returns None for clean setup.py
• test_no_setup_py -- returns None when no setup.py exists
• test_syntax_error -- returns None gracefully on syntax errors
• test_pkg_resources_in_string_not_detected -- no false positive from string literals
• test_default_build_system_deps_adds_setuptools_constraint -- integration test through full get_build_system_dependencies

[coderabbitai]

📝 Walkthrough

Walkthrough

Build-system dependency resolution now parses setup.py with the AST module, detects dry_run calls and pkg_resources imports, and appends the corresponding setuptools constraint. Missing or invalid files and unrelated occurrences produce no constraint. Tests cover the detection cases and verify integration with build-system dependency resolution.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes match #1263: AST detection for pkg_resources and dry_run, tighter cap precedence, and no-op behavior when absent.
Out of Scope Changes check	✅ Passed	No unrelated code changes are evident beyond the setuptools detection and test coverage for the linked objective.
Title check	✅ Passed	The title clearly summarizes the main change: auto-capping setuptools based on setup.py-detected removed APIs.
Description check	✅ Passed	The description matches the changeset and explains the AST-based setuptools cap logic, motivation, and tests.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fromager/dependencies.py`:
- Around line 164-167: Update the AST Call handling in the dependency analysis
so dry_run is recorded only when the call target resolves to a tracked
removed-API import, rather than for every keyword named dry_run. Preserve
existing removed-API detection and add a regression test confirming unrelated
local calls such as copy_assets are ignored.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 74bf20a1-8b86-435a-ba81-799fe50f64d2

📥 Commits

Reviewing files that changed from the base of the PR and between 3f11933 and f876e10.

📒 Files selected for processing (2)

• src/fromager/dependencies.py
• tests/test_dependencies.py

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Restrict dry_run detection to removed APIs.

Any local call such as copy_assets(dry_run=True) adds setuptools<81 despite not using setuptools functionality, potentially conflicting with a package’s declared setuptools requirement. Resolve the call target against tracked removed-API imports, and add a regression for an unrelated local dry_run parameter. Setuptools 81 removed setup.py dry-run support, not arbitrary Python keyword arguments. (setuptools.pypa.io)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fromager/dependencies.py` around lines 164 - 167, Update the AST Call
handling in the dependency analysis so dry_run is recorded only when the call
target resolves to a tracked removed-API import, rather than for every keyword
named dry_run. Preserve existing removed-API detection and add a regression test
confirming unrelated local calls such as copy_assets are ignored.

[dhellmann]

This is useful, but it feels somewhat specific to the set of packages we happen to be building right now. What sorts of options did you explore for making this a plugin or otherwise making it a more configurable behavior so that a user can choose to enable it or not?

[dhellmann]

What if there is already a setuptools requirement in the list? Shouldn't we modify that requirement instead of just adding another one?

[tiran]

Yes, the code has to carefully merge constraints. If the upstream project or our downstream project overrides set a lower ceiling, then the new code must not raise the ceiling.

[tiran]

Yes, the code has to carefully merge constraints. If the upstream project or our downstream project overrides set a lower ceiling, then the new code must not raise the ceiling.

[tiran]

The function should look into build_dir, not sdist_root_dir.

[vshawrh]

Closing in favor of a better approach: adding get_build_system_dependencies as a global hook point instead of putting setuptools-specific logic in core. New PR incoming.