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 1 commit
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
5 changes: 4 additions & 1 deletion crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2254,10 +2254,13 @@ pub struct ToolInstallArgs {
/// The package to install commands from.
pub package: String,

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

/// The package to install commands from.
///
/// This option is provided for parity with `uv tool run`, but is redundant with `package`.
#[arg(long, hide = true)]
#[arg(long, hide = true, conflicts_with("editable"))]
pub from: Option<String>,

/// Include the following extra requirements.
Expand Down
24 changes: 17 additions & 7 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 Down Expand Up @@ -121,8 +125,19 @@ pub(crate) async fn install(

from_requirement
} else {
let requirements = if editable {
RequirementsSpecification::from_source(
&RequirementsSource::Editable(package),
&client_builder,
)
.await?
.requirements
} else {
vec![RequirementsSpecification::parse_package(&package)?]
};

resolve_names(
vec![RequirementsSpecification::parse_package(&package)?],
requirements,
&interpreter,
&settings,
&state,
Expand All @@ -139,12 +154,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 @@ -672,6 +672,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 @@ -299,6 +299,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 @@ -307,6 +308,7 @@ impl ToolInstallSettings {
pub(crate) fn resolve(args: ToolInstallArgs, filesystem: Option<FilesystemOptions>) -> Self {
let ToolInstallArgs {
package,
editable,
from,
with,
with_requirements,
Expand All @@ -327,6 +329,7 @@ impl ToolInstallSettings {
.collect(),
python,
force,
editable,
refresh: Refresh::from(refresh),
settings: ResolverInstallerSettings::combine(
resolver_installer_options(installer, build),
Expand Down
28 changes: 28 additions & 0 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,34 @@ fn tool_install_version() {
"###);
}

/// Test an editable install 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`
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: false
exit_code: 1
----- stdout -----
No executables are provided by `black`

----- 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)
"###);
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
Loading