[fix] use -P instead of --upgrade for uv pip so it doesnt upgrade deps as well #338
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c38931ddf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR modifies the uv pip install behavior to only upgrade the target package without upgrading its dependencies. The change switches from using the --upgrade flag to using the -P <package_name> flag for uv, while keeping pip unchanged.
Changes:
- Modified
get_install_command()to accept apackage_nameparameter and use-Pflag foruvinstead of--upgrade - Updated the private build install logic to use
-Pflag with normalized package name foruv - Updated
_build_install_command()to use-Pflag for simple index installations withuv
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tool: Package manager to use ('uv' or 'pip') | ||
| wheel_url: URL to the wheel file | ||
| no_upgrade: If True, don't include --upgrade flag (preserves locked dependencies) | ||
| package_name: Package name for targeted upgrade with -P flag (uv only) |
There was a problem hiding this comment.
The docstring says the package_name parameter is for "targeted upgrade with -P flag (uv only)", but it doesn't specify whether the name should be normalized or not. Consider clarifying that this should be the normalized package name (as it is in practice at line 2311 where normalized_name is passed), or alternatively, perform the normalization inside this function to ensure consistency.
| package_name: Package name for targeted upgrade with -P flag (uv only) | |
| package_name: Normalized package name for targeted upgrade with -P flag (uv only) | |
| (should match the normalized name used elsewhere, e.g. lowercase as in PEP 503) |
| if with_tool == "uv": | ||
| cmd_parts = ["uv", "pip", "install", str(wheel_path)] | ||
| cmd_parts = ["uv", "pip", "install"] | ||
| if not no_upgrade: | ||
| # Use -P to only upgrade this package, not its dependencies | ||
| cmd_parts.extend(["-P", normalized_name]) | ||
| cmd_parts.append(str(wheel_path)) | ||
| else: | ||
| cmd_parts = ["pip", "install", str(wheel_path)] | ||
| if not no_upgrade: | ||
| cmd_parts.insert(-1, "--upgrade") | ||
| if not no_upgrade: | ||
| cmd_parts.append("--upgrade") |
There was a problem hiding this comment.
The --upgrade flag is placed after the wheel path (line 1601), but in the get_install_command function (lines 1277-1279), the flag is correctly placed before the wheel URL. This inconsistency means the command structure differs between this code path and the one using get_install_command. While pip typically accepts flags in either position, this creates unnecessary inconsistency. Consider using get_install_command(with_tool, str(wheel_path), normalized_name, no_upgrade) instead of manually building the command, which would ensure consistency with the rest of the codebase and reduce code duplication.
Note
Switches
uv pip installbehavior to use-P <package>instead of--upgrade, ensuring only the target environment is upgraded and not its dependencies.get_install_commandnow acceptspackage_nameand usesuv ... -P <package>; callers updated to pass normalized names_build_install_commandupdated to use-Pfor uv and keep--upgradefor pipWritten by Cursor Bugbot for commit 2c38931. This will update automatically on new commits. Configure here.