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.

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

Editable installs for uv tool #5454

Merged
merged 4 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2283,6 +2283,9 @@ pub struct ToolInstallArgs {
/// The package to install commands from.
pub package: String,

#[arg(short, long)]
pub editable: bool,

/// The package to install commands from.
///
/// This option is provided for parity with `uv tool run`, but is redundant with `package`.
Expand Down
33 changes: 25 additions & 8 deletions crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::settings::ResolverInstallerSettings;
/// Install a tool.
pub(crate) async fn install(
package: String,
editable: bool,
from: Option<String>,
with: &[RequirementsSource],
python: Option<String>,
Expand Down Expand Up @@ -82,6 +83,9 @@ pub(crate) async fn install(

// Initialize any shared state.
let state = SharedState::default();
let client_builder = BaseClientBuilder::new()
.connectivity(connectivity)
.native_tls(native_tls);

// Resolve the `from` requirement.
let from = if let Some(from) = from {
Expand All @@ -91,9 +95,18 @@ pub(crate) async fn install(
bail!("Package requirement (`{from}`) provided with `--from` conflicts with install request (`{package}`)", from = from.cyan(), package = package.cyan())
};

let source = if editable {
RequirementsSource::Editable(from)
} else {
RequirementsSource::Package(from)
};
let requirements = RequirementsSpecification::from_source(&source, &client_builder)
.await?
.requirements;

let from_requirement = {
resolve_names(
vec![RequirementsSpecification::parse_package(&from)?],
requirements,
&interpreter,
&settings,
&state,
Expand Down Expand Up @@ -121,8 +134,17 @@ pub(crate) async fn install(

from_requirement
} else {
let source = if editable {
RequirementsSource::Editable(package.clone())
} else {
RequirementsSource::Package(package.clone())
};
let requirements = RequirementsSpecification::from_source(&source, &client_builder)
.await?
.requirements;

resolve_names(
vec![RequirementsSpecification::parse_package(&package)?],
requirements,
&interpreter,
&settings,
&state,
Expand All @@ -139,12 +161,7 @@ pub(crate) async fn install(
};

// Read the `--with` requirements.
let spec = {
let client_builder = BaseClientBuilder::new()
.connectivity(connectivity)
.native_tls(native_tls);
RequirementsSpecification::from_simple_sources(with, &client_builder).await?
};
let spec = RequirementsSpecification::from_simple_sources(with, &client_builder).await?;

// Resolve the `--from` and `--with` requirements.
let requirements = {
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ async fn run(cli: Cli) -> Result<ExitStatus> {

commands::tool_install(
args.package,
args.editable,
args.from,
&requirements,
args.python,
Expand Down
3 changes: 3 additions & 0 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ pub(crate) struct ToolInstallSettings {
pub(crate) refresh: Refresh,
pub(crate) settings: ResolverInstallerSettings,
pub(crate) force: bool,
pub(crate) editable: bool,
}

impl ToolInstallSettings {
Expand All @@ -310,6 +311,7 @@ impl ToolInstallSettings {
pub(crate) fn resolve(args: ToolInstallArgs, filesystem: Option<FilesystemOptions>) -> Self {
let ToolInstallArgs {
package,
editable,
from,
with,
with_requirements,
Expand All @@ -330,6 +332,7 @@ impl ToolInstallSettings {
.collect(),
python,
force,
editable,
refresh: Refresh::from(refresh),
settings: ResolverInstallerSettings::combine(
resolver_installer_options(installer, build),
Expand Down
230 changes: 230 additions & 0 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,236 @@ fn tool_install_version() {
"###);
}

/// Test an editable installation of a tool.
#[test]
fn tool_install_editable() {
let context = TestContext::new("3.12").with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

// Install `black` as an editable package.
uv_snapshot!(context.filters(), context.tool_install()
.arg("-e")
.arg(context.workspace_root.join("scripts/packages/black_editable"))
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str())
.env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
warning: `uv tool install` is experimental and may change without warning
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ black==0.1.0 (from file://[WORKSPACE]/scripts/packages/black_editable)
Installed 1 executable: black
"###);

tool_dir.child("black").assert(predicate::path::is_dir());
tool_dir
.child("black")
.child("uv-receipt.toml")
.assert(predicate::path::exists());

let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX));
assert!(executable.exists());

// On Windows, we can't snapshot an executable file.
#[cfg(not(windows))]
insta::with_settings!({
filters => context.filters(),
}, {
// Should run black in the virtual environment
assert_snapshot!(fs_err::read_to_string(&executable).unwrap(), @r###"
#![TEMP_DIR]/tools/black/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from black import main
if __name__ == "__main__":
sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
sys.exit(main())
"###);

});

insta::with_settings!({
filters => context.filters(),
}, {
// We should have a tool receipt
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black @ file://[WORKSPACE]/scripts/packages/black_editable"]
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
]
"###);
});

uv_snapshot!(context.filters(), Command::new("black").arg("--version").env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
Hello world!

----- stderr -----
"###);

// Request `black`. It should retain the current installation.
// TODO(charlie): This is arguably incorrect, especially because the tool receipt removes the
// file path.
uv_snapshot!(context.filters(), context.tool_install()
.arg("black")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str())
.env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
warning: `uv tool install` is experimental and may change without warning
Installed 1 executable: black
"###);

insta::with_settings!({
filters => context.filters(),
}, {
// We should have a tool receipt
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black"]
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
]
"###);
});

// Request `black` at a different version. It should install a new version.
uv_snapshot!(context.filters(), context.tool_install()
.arg("black")
.arg("--from")
.arg("black==24.2.0")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str())
.env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
warning: `uv tool install` is experimental and may change without warning
Resolved 6 packages in [TIME]
Prepared 6 packages in [TIME]
Uninstalled 1 package in [TIME]
Installed 6 packages in [TIME]
- black==0.1.0 (from file://[WORKSPACE]/scripts/packages/black_editable)
+ black==24.2.0
+ click==8.1.7
+ mypy-extensions==1.0.0
+ packaging==24.0
+ pathspec==0.12.1
+ platformdirs==4.2.0
Installed 2 executables: black, blackd
"###);

insta::with_settings!({
filters => context.filters(),
}, {
// We should have a tool receipt
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black==24.2.0"]
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});
}

/// Test an editable installation of a tool using `--from`.
#[test]
fn tool_install_editable_from() {
let context = TestContext::new("3.12").with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

// Install `black` as an editable package.
uv_snapshot!(context.filters(), context.tool_install()
.arg("black")
.arg("-e")
.arg("--from")
.arg(context.workspace_root.join("scripts/packages/black_editable"))
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str())
.env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
warning: `uv tool install` is experimental and may change without warning
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ black==0.1.0 (from file://[WORKSPACE]/scripts/packages/black_editable)
Installed 1 executable: black
"###);

tool_dir.child("black").assert(predicate::path::is_dir());
tool_dir
.child("black")
.child("uv-receipt.toml")
.assert(predicate::path::exists());

let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX));
assert!(executable.exists());

// On Windows, we can't snapshot an executable file.
#[cfg(not(windows))]
insta::with_settings!({
filters => context.filters(),
}, {
// Should run black in the virtual environment
assert_snapshot!(fs_err::read_to_string(&executable).unwrap(), @r###"
#![TEMP_DIR]/tools/black/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from black import main
if __name__ == "__main__":
sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
sys.exit(main())
"###);

});

insta::with_settings!({
filters => context.filters(),
}, {
// We should have a tool receipt
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black @ file://[WORKSPACE]/scripts/packages/black_editable"]
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
]
"###);
});

uv_snapshot!(context.filters(), Command::new("black").arg("--version").env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
Hello world!

----- stderr -----
"###);
Copy link
Member

Choose a reason for hiding this comment

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

Can we show the receipt here?

Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a test for uv tool install back after uv tool install -e ./scripts/packages/black_editable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we show the receipt here?

uv tool install exits with a failure for the black_editable package since it has no executables. So no receipt gets created in this scenario.

Copy link
Member

Choose a reason for hiding this comment

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

Can we add an executable to it?

Copy link
Contributor Author

@blueraft blueraft Jul 25, 2024

Choose a reason for hiding this comment

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

I've added a simple executable and added additional snapshot tests.

Can we also add a test for uv tool install back after uv tool install -e ./scripts/packages/black_editable?

I added this too, but it just installs black version from the PyPi server in this case, do you think that's the right behaviour here?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's because you're creating a new context here. If you remove that, it retains the install (but the behavior is kind of confusing -- #5489 helps).

}

/// Test installing a tool with `uv tool install --from`
#[test]
fn tool_install_from() {
Expand Down
8 changes: 6 additions & 2 deletions scripts/packages/black_editable/black/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
def a():
pass
def main():
print("Hello world!")


if __name__ == "__main__":
main()
3 changes: 3 additions & 0 deletions scripts/packages/black_editable/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ dependencies = []
requires-python = ">=3.11,<3.13"
license = {text = "MIT"}

[project.scripts]
black = "black:main"

[project.optional-dependencies]
colorama = ["colorama>=0.4.3"]
uvloop = ["uvloop>=0.15.2"]
Expand Down
Loading