Skip to content

Fix optional file dependencies - #982

Open
harbinresearcher wants to merge 1 commit into
python-poetry:mainfrom
harbinresearcher:codex/fix-optional-file-dependencies
Open

harbinresearcher wants to merge 1 commit into
python-poetry:mainfrom
harbinresearcher:codex/fix-optional-file-dependencies

Conversation

@harbinresearcher

@harbinresearcher harbinresearcher commented Oct 3, 2026 •

Copy link
Copy Markdown

Resolves: python-poetry/poetry#10426

Factory.create_dependency() reads optional from a dependency table but does not pass it to FileDependency in the file branch. The equivalent path branch already passes it. This leaves an explicitly optional file dependency required and activated.

Pass the existing flag through in the file branch. The parameterized regression covers file and path with the flag omitted, false, and true, including the resulting activation state. No changes to lock-file format or extras handling.

Validation:

  • Before the fix: the file / optional=true regression fails; five controls pass.
  • After the fix: 6 regression cases pass; 168 factory tests pass.
  • Full suite on Windows / Python 3.12.14: 2960 passed, 3 skipped, 9 integration tests deselected.
  • Separately enabled integration suite: 9 passed.
  • Ruff 0.16.4, mypy, pre-commit on changed files, and wheel/sdist build pass.
  • A local Poetry CLI smoke test with an optional wheel writes optional=false before the fix and optional=true after it; install --dry-run without the extra skips the dependency.

The unmodified full-suite baseline had one unrelated test_path_to_url_win failure when run from a non-ASCII working directory. Running the final suite from an ASCII temporary working directory avoids that environment-dependent assertion; no test was disabled or changed for it.

Developed with Codex assistance; the regression, full suite, integration suite, and CLI smoke checks were run locally.

  • Added tests for changed code.
  • Updated documentation for changed code. Not needed: this restores the existing option; no interface change.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. This changes whether file and path dependencies are marked optional, which can alter dependency resolution and whether those packages are installed or activated. Reverting restores the previous behavior, but lockfiles or already-installed dependency state may need to be regenerated or cleaned up.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

This branch has not been deployed

No deployments
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.

optional=true not respected for local file dependency, but works with local path dependency

1 participant