Skip to content

fix: recognize additional dependency manifests and lockfiles - #450

Open
JohnsonLyu wants to merge 1 commit into
darnitdevorg:mainfrom
JohnsonLyu:fix/repro-deps-manifests-446
Open

JohnsonLyu wants to merge 1 commit into
darnitdevorg:mainfrom
JohnsonLyu:fix/repro-deps-manifests-446

Conversation

@JohnsonLyu

Copy link
Copy Markdown

Closes #446.

Summary

Expand repro_deps_pinned filename recognition for additional dependency manifests and lock files.

Added manifest recognition for:

  • pyproject.toml
  • Pipfile
  • environment.yml
  • environment.yaml

Added lock file recognition for:

  • pdm.lock
  • conda-lock.yml
  • pnpm-lock.yaml
  • bun.lockb

This intentionally causes repositories with one of the newly recognized manifests but no recognized lock file to return FAIL instead of INCONCLUSIVE.

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

Framework Changes Checklist

If this PR modifies the darnit framework (packages/darnit/):

  • Updated framework spec (docs/architecture/framework-design.md) if behavior changed
  • Ran uv run python scripts/validate_sync.py --verbose and it passes

Control/TOML Changes Checklist

If this PR modifies controls or TOML configuration:

  • Control metadata defined in TOML (not Python code)
  • SARIF fields (description, severity, help_url) included where appropriate
  • Ran validation to confirm TOML schema compliance

Testing

  • Tests pass locally (uv run pytest tests/ -v)
  • Added tests for new functionality (if applicable)
  • Linting passes (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

  • No AI assistance was used
  • AI assistance was used

Used Cursor for code navigation and review.

Additional Notes

This change only expands recognized filenames and does not change the existing decision logic.

Signed-off-by: Johnson Lyu <cl7084@nyu.edu>
@Marc-cn

Marc-cn commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

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.
Behaviour on real repos matches what you describe:

tqdm       WARN -> FAIL   (pyproject.toml now recognised; no lock file)
requests   FAIL -> FAIL   (picks up pyproject.toml alongside setup.py)
flask      PASS -> PASS   (uv.lock, unchanged)
in-toto    FAIL -> FAIL   (unchanged)

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.
LGTM.
@mlieberman85 to merge

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

repro_deps_pinned does not recognize pyproject.toml or conda environment files

3 participants