fix: recognize additional dependency manifests and lockfiles - #450
JohnsonLyu wants to merge 1 commit into
Conversation
Signed-off-by: Johnson Lyu <cl7084@nyu.edu>
|
Thanks @JohnsonLyu,tested on Linux, rebased onto current main (which now includes #443, so the requirements.txt-reading change landed after your branch point). Rebases cleanly, 213 reproducibility tests pass. The tqdm move is the interesting one and it's a real improvement, before, tqdm had no recognised manifest at all, so it fell through to "Could not automatically verify". That was a false negative; FAIL is the honest answer. One follow-up, not for this PR: #443 made the control read requirements.txt contents, but pyproject.toml is still judged by filename alone. A project with everything pinned in pyproject and no lock file now FAILs without the file being opened. Worth a separate issue for reading pyproject.toml the way requirements.txt is read. |
mlieberman85
left a comment
There was a problem hiding this comment.
The lock files are right and were genuinely missing. Trial-merged into current main: merges clean, 3215 tests pass.
One fix needed. pyproject.toml is judged by name, so a file with no [project] section at all -- [tool.ruff] and nothing else -- reports "Dependency manifests found but no lock files".
pypa/pip is the concrete case, and it is the repo that motivated #446: no dependencies key, dynamic = ["version"] only, no lock file. It vendors everything, so there is nothing to pin, and this moves it from an unhelpful WARN to a confident wrong answer.
Feature 037 already settled this for requirements.txt (FR-011): a file declaring no dependencies is treated as absent. Same rule applies here.
Pipfile and environment.yml have the same hole but far fewer dependency-free instances in practice -- your call whether to fold them in.
| loose_manifests = { | ||
| "requirements.txt": "pip requirements", | ||
| "setup.py": "setuptools", | ||
| "pyproject.toml": "pyproject (Python)", |
There was a problem hiding this comment.
Needs a content check. tomllib is stdlib and already used in the repo.
def _pyproject_declares_dependencies(path: Path) -> bool:
try:
data = tomllib.loads(path.read_text(encoding="utf-8"))
except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError):
return True # unreadable is not "declares nothing"
project = data.get("project", {})
return bool(
project.get("dependencies")
or project.get("optional-dependencies")
or "dependencies" in project.get("dynamic", [])
)The dynamic clause matters: without it, any project keeping its dependencies in setup.py reads as declaring none.
| result = repro_deps_pinned_handler({}, make_ctx(tmp_path)) | ||
| assert result.status == HandlerResultStatus.FAIL | ||
|
|
||
| def test_fail_with_only_pyproject_toml(self, tmp_path: Path) -> None: |
There was a problem hiding this comment.
This fixture has no dependencies, so it is the case that should be treated as absent. Worth two tests: a [project] with real dependencies asserting FAIL, and this one asserting INCONCLUSIVE.
Closes #446.
Summary
Expand
repro_deps_pinnedfilename recognition for additional dependency manifests and lock files.Added manifest recognition for:
pyproject.tomlPipfileenvironment.ymlenvironment.yamlAdded lock file recognition for:
pdm.lockconda-lock.ymlpnpm-lock.yamlbun.lockbThis intentionally causes repositories with one of the newly recognized manifests but no recognized lock file to return
FAILinstead ofINCONCLUSIVE.Type of Change
Framework Changes Checklist
If this PR modifies the darnit framework (
packages/darnit/):docs/architecture/framework-design.md) if behavior changeduv run python scripts/validate_sync.py --verboseand it passesControl/TOML Changes Checklist
If this PR modifies controls or TOML configuration:
Testing
uv run pytest tests/ -v)uv run ruff check .)Targeted tests:
14 passed.Full test suite was also run on Windows; unrelated Windows-specific path and encoding failures remain.
AI assistance
Used Cursor for code navigation and review.
Additional Notes
This change only expands recognized filenames and does not change the existing decision logic.