Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add uv tool install #4492

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Add uv tool install #4492

merged 1 commit into from
Jun 26, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Jun 24, 2024 •
edited
Loading

This is the minimal "working" implementation. In summary, we:

  • Resolve the requested requirements
  • Create an environment at $UV_STATE_DIR/tools/$name
  • Inspect the dist-info for the main requirement to determine its entry points scripts
  • Link the entry points from a user-executable directory ($XDG_BIN_HOME) to the environment bin
  • Create an entry at $UV_STATE_DIR/tools/tools.toml tracking the user's request

The idea with tools.toml is that it allows us to perform upgrades and syncs, retaining the original user request (similar to declarations in a pyproject.toml). I imagine using a similar schema in the pyproject.toml in the future if/when we add project-levle tools. I'm also considering exposing tools.toml in the standard uv configuration directory instead of the state directory, but it seems nice to tuck it away for now while we iterate on it. Installing a tool won't perform a sync of other tool environments, we'll probably have an explicit uv tool sync command for that?

I've split out todos into follow-up pull requests:

Closes #4485

@zanieb zanieb force-pushed the zb/tool-install branch 2 times, most recently from 4358b81 to 502c1da Compare June 24, 2024 23:33
@zanieb zanieb added preview Experimental behavior labels Jun 25, 2024
@zanieb zanieb mentioned this pull request Jun 25, 2024
Merged
zanieb
zanieb commented Jun 25, 2024
View reviewed changes
crates/uv-tool/src/lib.rs Outdated Show resolved Hide resolved
@zanieb zanieb force-pushed the zb/tool-install branch 2 times, most recently from db4e62d to 5bfdbbe Compare June 25, 2024 04:20
This was referenced Jun 25, 2024
Merged
Merged
@zanieb zanieb force-pushed the zb/tool-install branch 2 times, most recently from 3967fe7 to ceec118 Compare June 25, 2024 12:44
zanieb added a commit that referenced this pull request Jun 25, 2024
@zanieb
Isolate the tests from the real home directory (#4508)
e25cbee 
Nice to have for #4492 and seems like a good idea in general to avoid
mutating a developer's machine.
@zanieb zanieb force-pushed the zb/tool-install branch 2 times, most recently from de0c954 to 49d3532 Compare June 25, 2024 13:09
konstin
konstin approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

A single tools.toml for all tools sounds like a way that could break all installed tool by corrupting a single file, what e.g. about $UV_STATE_DIR/tools/$name/tool.toml for each tool?

Should we add versioning to the toml file(s) to allow breaking changes in the format?

We should add locking to prevent concurrent modification of both tools and toml files; we already have an abstraction for this with LockedFile::acquire(tool_dir.join(".lock"), tool_dir.user_display()).

crates/uv-cli/src/lib.rs Outdated Show resolved Hide resolved
crates/uv-tool/src/lib.rs Outdated Show resolved Hide resolved
crates/uv-tool/src/lib.rs Show resolved Hide resolved
crates/uv/src/commands/tool/install.rs Outdated Show resolved Hide resolved
@zanieb
Copy link
Member Author

zanieb commented Jun 25, 2024

A single tools.toml for all tools sounds like a way that could break all installed tool by corrupting a single file, what e.g. about $UV_STATE_DIR/tools/$name/tool.toml for each tool?

Seems reasonable, though I wanted a central file for bulk operations. I guess we can read every directory instead pretty easily. It won't break the tools though it'd just stop you from installing more.

Should we add versioning to the toml file(s) to allow breaking changes in the format?

Perhaps eventually, but it doesn't seem critical right now. Do you think we need to immediately?

We should add locking to prevent concurrent modification of both tools and toml files; we already have an abstraction for this with LockedFile::acquire(tool_dir.join(".lock"), tool_dir.user_display()).

Sounds good to me.

zanieb
zanieb commented Jun 25, 2024
View reviewed changes
crates/uv-tool/src/lib.rs Outdated Show resolved Hide resolved
zanieb
zanieb commented Jun 25, 2024
View reviewed changes
crates/uv-tool/src/lib.rs Outdated Show resolved Hide resolved
@konstin
Copy link
Member

konstin commented Jun 25, 2024

Perhaps eventually, but it doesn't seem critical right now. Do you think we need to immediately?

I'd put a version = 1 on top, if we never use it it's ~free and if we bump the version later all old versions error immediately.

@zanieb
Copy link
Member Author

zanieb commented Jun 25, 2024

I'm hesitating on multiple tool files. I eventually want people to be able to define their tools in a centralized file rather than in invocations.

@zanieb
Copy link
Member Author

zanieb commented Jun 25, 2024 •
edited
Loading

I'm going to skip adding a version in this pull request, but I'm happy to do it next. I just want to think more carefully about how it works.

@zanieb zanieb marked this pull request as ready for review June 25, 2024 14:20
charliermarsh pushed a commit that referenced this pull request Jun 25, 2024
@zanieb
Update project::update_environment to respect reinstall options (#4502
e242cdf 
)

While working on #4492 I noticed
that `--reinstall-package` was not actually respected by
`update_environment`, it exited early due to satisfied requirements.

Before

```

❯ cargo run -q -- tool install black -v --reinstall-package tomli
...
DEBUG All requirements satisfied: black | click>=8.0.0 | mypy-extensions>=0.4.3 | packaging>=22.0 | pathspec>=0.9.0 | platformdirs>=2 | tomli>=1.1.0 ; python_version < '3.11' | typing-extensions>=4.0.1 ; python_version < '3.11'
```

After

```
❯ cargo run -q -- tool install black -v --reinstall-package tomli
...
Uninstalled 1 package in 0.99ms
Installed 1 package in 4ms
 - tomli==2.0.1
 + tomli==2.0.1
```
charliermarsh
charliermarsh approved these changes Jun 26, 2024
View reviewed changes
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Looks good! My only concern here is around tools.toml, because it feels under-explored right now and (as far as I can tell) is unused at the moment. Will we always keep it in sync? What are the motivating use-cases?

@zanieb zanieb merged commit c9657b0 into main Jun 26, 2024
47 checks passed
@zanieb zanieb deleted the zb/tool-install branch June 26, 2024 15:24
@charliermarsh
Copy link
Member

Rock on

@zanieb zanieb mentioned this pull request Jun 26, 2024
Merged
zanieb added a commit that referenced this pull request Jun 26, 2024
@zanieb
Add test cases for uv tool install (#4509)
dc40814 
Adds test cases for functionality in #4492.

Includes #4520 which was needed to pass CI.
charliermarsh pushed a commit that referenced this pull request Jun 26, 2024
@zanieb
Move from a shared tools.toml to separated tool receipts (#4560)
fc681ec 
Refactors the installed tool metadata per commentary in #4492 

We now store a `uv-receipt.toml` per tool install instead of a single
`tools.toml`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

@konstin konstin konstin approved these changes

Assignees
No one assigned
Labels
preview Experimental behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add uv tool install
3 participants
@zanieb @konstin @charliermarsh