Skip to content

Python: Honor the legacy dependency --project selector - #9674

Closed
LI (ktz03) wants to merge 1 commit into
microsoft:mainfrom
ktz03:fix/dependency-project-alias-20261024
Closed

LI (ktz03) wants to merge 1 commit into
microsoft:mainfrom
ktz03:fix/dependency-project-alias-20261024

Conversation

@ktz03

Copy link
Copy Markdown
Contributor

Motivation & Context

add_dependency_to_project.py retains --project as a hidden compatibility alias, but --project core --dependency httpx exits with an argparse error. The required flag is attached only to the separate -P/--package action, so setting the shared destination through the alias does not satisfy it.

Description & Review Guide

  • What are the major changes? Validate the shared selector destination after parsing instead of requiring only the canonical action. Add eight focused regression cases.
  • What is the impact of these changes? The documented compatibility alias works by itself. Canonical spellings, missing-selector/dependency errors, hidden-alias help output, last-value behavior when both spellings are used, and the existing uv add delegation are preserved.
  • What do you want reviewers to focus on? The is None check preserves existing handling of explicit empty values. The focused tests call the actual wrapper main(), with package resolution and subprocess execution mocked. The baseline has one failure (legacy alias alone) and seven passing controls; the fix passes all eight. No real uv add, dependency mutation, SDK/provider call, build, or full workspace suite was run.

Related Issue

N/A - trivial change. This corrects the existing source's explicit compatibility promise without changing selector resolution, dependency handling, or subprocess execution.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Codex: code, regression tests, and this PR description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI 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.

🟡 Changes recommended

The new regression tests are outside CI test discovery, so they do not provide automated protection against reintroducing the alias failure.

1 open issue
What changed in this PR

Fixes the Python dependency-management wrapper so the hidden legacy --project selector works without requiring --package.

Changes:

  • Validates the shared selector value after parsing, preserving explicit empty-value handling.
  • Adds eight regression cases covering selector spellings, precedence, required arguments, hidden help output, and uv add delegation.
File Description
python/​scripts/​dependencies/​tests/​test_add_dependency_to_project.py Adds mocked command-line regression tests.
python/​scripts/​dependencies/​add_dependency_to_project.py Moves selector validation after parsing to support the legacy alias.

Review effort: Balanced

Comment on lines +36 to +38
def test_main_accepts_selector_spellings(
monkeypatch: pytest.MonkeyPatch, selector_args: list[str], expected_selector: str
) -> None:

This branch was previously deployed

1 inactive deployment
github-app-auth — d8715aab Deployed Oct 24, 2026 by ktz03 via team_check #7395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants