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

Warn when project-specific settings are passed to non-project uv run commands #5977

Merged
merged 1 commit into from
Aug 9, 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
16 changes: 12 additions & 4 deletions crates/uv/src/commands/project/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,7 @@ async fn init_project(
printer: Printer,
) -> Result<()> {
// Discover the current workspace, if it exists.
let workspace = if no_workspace {
None
} else {
// Attempt to find a workspace root.
let workspace = {
let parent = path.parent().expect("Project path has no parent");
match Workspace::discover(
parent,
Expand All @@ -188,6 +185,17 @@ async fn init_project(
}
};

// Ignore the current workspace, if `--no-workspace` was provided.
let workspace = if no_workspace {
// If the user runs with `--no-workspace` and we can't find a workspace, warn.
if workspace.is_none() {
warn_user_once!("`--no-workspace` was provided, but no workspace was found");
}
None
} else {
workspace
};

// Add a `requires-python` field to the `pyproject.toml`.
let requires_python = if let Some(request) = python.as_deref() {
// (1) Explicit request from user
Expand Down
78 changes: 75 additions & 3 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,42 @@ pub(crate) async fn run(
// Discover and sync the base environment.
let temp_dir;
let base_interpreter = if let Some(script_interpreter) = script_interpreter {
// If we found a PEP 723 script and the user provided a project-only setting, warn.
if !extras.is_empty() {
warn_user_once!("Extras are not supported for Python scripts with inline metadata");
}
if !dev {
warn_user_once!("`--no-dev` is not supported for Python scripts with inline metadata");
}
if no_project {
warn_user_once!(
"`--no-project` is a no-op for Python scripts with inline metadata, which always run in isolation"
);
}
if package.is_some() {
warn_user_once!(
"`--package` is a no-op for Python scripts with inline metadata, which always run in isolation"
);
}
if locked {
warn_user_once!(
"`--locked` is a no-op for Python scripts with inline metadata, which always run in isolation"
);
}
if frozen {
warn_user_once!(
"`--frozen` is a no-op for Python scripts with inline metadata, which always run in isolation"
);
}
if isolated {
warn_user_once!(
"`--isolated` is a no-op for Python scripts with inline metadata, which always run in isolation"
);
}

script_interpreter
} else {
let project = if no_project {
None
} else if let Some(package) = package {
let project = if let Some(package) = package {
// We need a workspace, but we don't need to have a current package, we can be e.g. in
// the root of a virtual workspace and then switch into the selected package.
Some(VirtualProject::Project(
Expand All @@ -209,6 +240,47 @@ pub(crate) async fn run(
}
};

let project = if no_project {
// If the user runs with `--no-project` and we can't find a project, warn.
if project.is_none() {
warn_user_once!("`--no-project` was provided, but no project was found");
}

// If the user ran with `--no-project` and provided a project-only setting, warn.
if !extras.is_empty() {
warn_user_once!("Extras have no effect when used alongside `--no-project`");
}
if !dev {
warn_user_once!("`--no-dev` has no effect when used alongside `--no-project`");
}
if locked {
warn_user_once!("`--locked` has no effect when used alongside `--no-project`");
}
if frozen {
warn_user_once!("`--frozen` has no effect when used alongside `--no-project`");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We could enforce these specific warnings via Clap, and make them errors. But the others must be done at runtime, because we don't know if we'll discover a project when we first parse the arguments. So I just did them all as warnings for consistency.


None
} else {
// If we can't find a project and the user provided a project-only setting, warn.
if project.is_none() {
if !extras.is_empty() {
warn_user_once!("Extras have no effect when used outside of a project");
}
if !dev {
warn_user_once!("`--no-dev` has no effect when used outside of a project");
}
if locked {
warn_user_once!("`--locked` has no effect when used outside of a project");
}
if frozen {
warn_user_once!("`--frozen` has no effect when used outside of a project");
}
}

project
};

let interpreter = if let Some(project) = project {
if let Some(project_name) = project.project_name() {
debug!(
Expand Down
41 changes: 41 additions & 0 deletions crates/uv/tests/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,47 @@ fn init_no_workspace() -> Result<()> {
Ok(())
}

/// Warn if the user provides `--no-workspace` outside of a workspace.
#[test]
fn init_no_workspace_warning() -> Result<()> {
let context = TestContext::new("3.12");

uv_snapshot!(context.filters(), context.init().current_dir(&context.temp_dir).arg("--no-workspace").arg("--name").arg("project"), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
warning: `uv init` is experimental and may change without warning
warning: `--no-workspace` was provided, but no workspace was found
Initialized project `project`
"###);

let workspace = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
workspace, @r###"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
readme = "README.md"
requires-python = ">=3.12"
dependencies = []

[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"
"###
);
});

Ok(())
}

#[test]
fn init_project_inside_project() -> Result<()> {
let context = TestContext::new("3.12");
Expand Down
36 changes: 36 additions & 0 deletions crates/uv/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,18 @@ fn run_pep723_script() -> Result<()> {
Reading inline script metadata from: main.py
"###);

// Running a script with `--no-project` should warn.
uv_snapshot!(context.filters(), context.run().arg("--preview").arg("--no-project").arg("main.py"), @r###"
success: true
exit_code: 0
----- stdout -----
Hello, world!

----- stderr -----
Reading inline script metadata from: main.py
warning: `--no-project` is a no-op for Python scripts with inline metadata, which always run in isolation
"###);

Ok(())
}

Expand Down Expand Up @@ -1119,5 +1131,29 @@ fn run_no_project() -> Result<()> {
warning: `uv run` is experimental and may change without warning
"###);

// `run --no-project` should not (but it should still run in the same environment, as it would
// if there were no project at all).
uv_snapshot!(context.filters(), context.run().arg("--no-project").arg("python").arg("-c").arg("import sys; print(sys.executable)"), @r###"
success: true
exit_code: 0
----- stdout -----
[VENV]/[BIN]/python

----- stderr -----
warning: `uv run` is experimental and may change without warning
"###);

// `run --no-project --locked` should fail.
uv_snapshot!(context.filters(), context.run().arg("--no-project").arg("--locked").arg("python").arg("-c").arg("import sys; print(sys.executable)"), @r###"
success: true
exit_code: 0
----- stdout -----
[VENV]/[BIN]/python

----- stderr -----
warning: `uv run` is experimental and may change without warning
warning: `--locked` has no effect when used alongside `--no-project`
"###);

Ok(())
}
Loading