-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add uv python install --default
#8650
Conversation
457a318
to
4579f3f
Compare
783e70d
to
00873e2
Compare
4579f3f
to
7e14476
Compare
5280eab
to
0598d60
Compare
bb90cbd
to
ae37a24
Compare
032950f
to
43e7779
Compare
Pulling out of #8650 for readability. Trying to clean this up to simplify extensions in the future. This is not a strict refactor, there are behavioral changes here. - Adds some structs for managing state. - Addresses some likely inconsistent behavior for weird edge-cases. - We fill platform information before checking if a request is satisfied. - We error earlier if we can't find a download for the request, i.e., even if you somehow have it installed. - Only reports versions as uninstalled if a download actually replaces them. - Moves some of the default output to tracing messages. - Even if an installation was already satisfied, we'll check that it is setup properly
2981c8d
to
4e4d3cd
Compare
4e4d3cd
to
40c88cd
Compare
af592db
to
25ad2fe
Compare
Thanks, I can look into fixing that. |
I am a bit confused how the default versions are replaced. I know I can force replace them by adding --force, but if I don't, what is replaced feels a bit off. Starting from scratch, install and set 3.12.6 as default
OK! New version released, install and set 3.12.7 as default
OK! New minor version released, install and set 3.13 as default.
hmm? |
That looks wrong indeed :) thanks for giving it a try. |
Should --default maybe always imply --force? From scratch again:
The install command returns success because it has successfully made sure there is a 3.12 version installed. However, in my mind, it "failed" to set 3.12 as default. |
|
c1619ee
to
c697111
Compare
That's fixed now. |
--default works as I expected now! --force seems to both be needed to replace executables not managed by uv as well as allowing minor versions managed by uv to be downgraded.
|
The latter is intentional. Do you think we should make the log more visible explaining that behavior? |
I think it makes perfect sense and I don't think it needs to be clarified. More information can also confuse. I think I was mostly confused by your comment above that --force was for replacing executables not managed by uv. I just had never encountered that before. |
These were erroneously being filtered, interfering with the snapshots in #8650
44b018a
to
fc46546
Compare
fc46546
to
c2b582a
Compare
In a follow-up, I'd like to change the executable display to include all the executables available for the version not just the ones we added in this invocation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good, just a few comments.
@@ -302,114 +312,136 @@ pub(crate) async fn install( | |||
.expect("We should have a bin directory with preview enabled") | |||
.as_path(); | |||
|
|||
let target = bin.join(installation.key().versioned_executable_name()); | |||
let targets = if (default || is_default_install) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is_default_install
here? I thought that meant something distinct from default
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you run
uv python install
(without arguments), we include the --default flag implicitly to populate python and python3 for the "default" install version.
When you run uv python install
without arguments, it's a "default install". The idea is for that bootstrapping experience to get you a complete installation. Using the --default
flag in subsequent operations would be used to change the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(If you've previously installed Python with uv, using uv python install
without arguments has no effect)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, ok. But if there's a .python-version
file, it does not count as a "default" install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not really sure if it makes sense but I would find it surprising for my "default" version to be set when installing the Python versions needed for a project, e.g., I noticed this for uv. It's pretty niche though.
target.simplified_display() | ||
); | ||
for target in targets { | ||
let target = bin.join(target); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is everything from here down mostly just an indent of the existing logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, and some tweaks from rebases though I've reduced those a couple times.
This pull request is best viewed with whitespace hidden
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Trust you to act on feedback as you see fit!)
Should be addressed in d3d94ce |
d3d94ce
to
d95fe76
Compare
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.5.5` -> `0.5.6` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.5.6`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#056) [Compare Source](astral-sh/uv@0.5.5...0.5.6) ##### Enhancements - Add `--dry-run` to `uv pip uninstall` ([#​9557](astral-sh/uv#9557)) - Allow `--constraints` and `--overrides` in `uv tool install` ([#​9547](astral-sh/uv#9547)) - Display removed Python executables on uninstall ([#​9459](astral-sh/uv#9459)) - Warn when keyring has no password for `uv publish` ([#​8827](astral-sh/uv#8827)) - Add suggested action when `.python-version` pin is incompatible with the project ([#​9590](astral-sh/uv#9590)) - Improve error messages for mismatches in `tool.uv.sources` ([#​9482](astral-sh/uv#9482)) - Use constraints in trace rather than irrelevant `requires-python` ([#​9529](astral-sh/uv#9529)) ##### Preview features - Add `uv python install --default` ([#​8650](astral-sh/uv#8650)) - Fix Python executable installation when multiple patch versions are requested ([#​9607](astral-sh/uv#9607)) - Build backend: Revamp `include` / `exclude` ([#​9525](astral-sh/uv#9525)) - Build backend: Add fast path ([#​9556](astral-sh/uv#9556)) - Build backend: Add functions to collect file list ([#​9602](astral-sh/uv#9602)) - Build backend: Default excludes ([#​9552](astral-sh/uv#9552)) - Build backend: Refactoring before list ([#​9558](astral-sh/uv#9558)) - Build backend: Warn when visiting over 10k files ([#​9523](astral-sh/uv#9523)) ##### Configuration - Make `check-url` available in configuration files ([#​9032](astral-sh/uv#9032)) ##### Performance - Avoid adding non-extra package with extra dependencies ([#​9540](astral-sh/uv#9540)) - Avoid cloning `String` in marker evaluation ([#​9598](astral-sh/uv#9598)) ##### Rust API - `uv-pep508`: Add more methods for simplifying `extra`-related expressions ([#​9469](astral-sh/uv#9469)) ##### Bug fixes - Allow `file:` URLs to include package names ([#​9493](astral-sh/uv#9493)) - Avoid using IDs across PubGrub states ([#​9538](astral-sh/uv#9538)) - Consistently enforce requested-vs.-built metadata when retrieving wheels ([#​9484](astral-sh/uv#9484)) - Do not show empty version specifier in `uv tool list` ([#​9605](astral-sh/uv#9605)) - Include Git member information when getting metadata from cache ([#​9388](astral-sh/uv#9388)) - Include base installation directory in uv run PATH ([#​9585](astral-sh/uv#9585)) - Insert backslash when appending to system drive ([#​9488](astral-sh/uv#9488)) - Normalize paths when lowering Git dependencies ([#​9595](astral-sh/uv#9595)) - Omit origin when comparing requirements ([#​9570](astral-sh/uv#9570)) - Override `manylinux_compatible` with `--python-platform` ([#​9526](astral-sh/uv#9526)) - Pass extra when evaluating lockfile markers ([#​9539](astral-sh/uv#9539)) - Propagate markers for recursive extras in resolver ([#​9509](astral-sh/uv#9509)) - Respect path dependencies within Git dependencies ([#​9594](astral-sh/uv#9594)) - Support recursive extras with marker in `pip compile -r pyproject.toml` ([#​9535](astral-sh/uv#9535)) - Don't emit unpinned warning for proxy packages ([#​9497](astral-sh/uv#9497)) - Fix `--refresh-package` flag mentioned as `--refresh-dependency` ([#​9486](astral-sh/uv#9486)) - Handle Windows AV/EDR file locks during script installations ([#​9543](astral-sh/uv#9543)) - Re-enable conflicting extra/group tests and fix regression from [#​9540](astral-sh/uv#9540) ([#​9582](astral-sh/uv#9582)) ##### Documentation - Add missing word to docs for `run.md` ([#​9527](astral-sh/uv#9527)) - Add policies reference section and license document ([#​9367](astral-sh/uv#9367)) - Fix typo in entry point docs ([#​9491](astral-sh/uv#9491)) - Fix up version in prior uninstall instructions ([#​9485](astral-sh/uv#9485)) - Mention `uv pip` behavior in build system note ([#​9586](astral-sh/uv#9586)) - Update build failures document ([#​9584](astral-sh/uv#9584)) - Correct wording for multiple sources section ([#​9504](astral-sh/uv#9504)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
This pull request is best viewed with whitespace hidden
Adds a
--default
flag touv python install
in preview. This includes apython
andpython{major}
executable in addition to thepython{major}.{minor}
executable. We will replace uv-managed executables, but externally managed executables require the--force
flag to overwrite.If you run
uv python install
(without arguments), we include the--default
flag implicitly to populatepython
andpython3
for the "default" install version.In the future, we should add a warning if the installed executable isn't at the front of the PATH.