Warn on requirements.txt-provided arguments in uv run et al (#5364)

## Summary

Also applies to `uv tool run` and `uv tool install`.

Closes https://github.com/astral-sh/uv/issues/5349.
This commit is contained in:
Charlie Marsh 2024-07-23 16:34:01 -04:00 committed by GitHub
parent ada2b2bc29
commit 25c7599030
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 380 additions and 84 deletions

View file

@ -386,6 +386,11 @@ impl<'a> IndexLocations {
self.flat_index.iter()
}
/// Return the `--no-index` flag.
pub fn no_index(&self) -> bool {
self.no_index
}
/// Clone the index locations into a [`IndexUrls`] instance.
pub fn index_urls(&'a self) -> IndexUrls {
IndexUrls {

View file

@ -3,7 +3,6 @@ use tracing::debug;
use cache_key::digest;
use distribution_types::Resolution;
use pypi_types::Requirement;
use uv_cache::{Cache, CacheBucket};
use uv_client::Connectivity;
use uv_configuration::{Concurrency, PreviewMode};
@ -30,7 +29,7 @@ impl CachedEnvironment {
/// Get or create an [`CachedEnvironment`] based on a given set of requirements and a base
/// interpreter.
pub(crate) async fn get_or_create(
requirements: Vec<Requirement>,
spec: RequirementsSpecification,
interpreter: Interpreter,
settings: &ResolverInstallerSettings,
state: &SharedState,
@ -41,8 +40,6 @@ impl CachedEnvironment {
cache: &Cache,
printer: Printer,
) -> anyhow::Result<Self> {
let spec = RequirementsSpecification::from_requirements(requirements);
// When caching, always use the base interpreter, rather than that of the virtual
// environment.
let interpreter = if let Some(interpreter) = interpreter.to_base_interpreter(cache)? {

View file

@ -26,7 +26,7 @@ use uv_resolver::{
FlatIndex, OptionsBuilder, PythonRequirement, RequiresPython, ResolutionGraph, ResolverMarkers,
};
use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy};
use uv_warnings::warn_user;
use uv_warnings::{warn_user, warn_user_once};
use uv_workspace::Workspace;
use crate::commands::pip::operations::Modifications;
@ -405,6 +405,8 @@ pub(crate) async fn resolve_environment<'a>(
cache: &Cache,
printer: Printer,
) -> anyhow::Result<ResolutionGraph> {
warn_on_requirements_txt_setting(&spec, settings);
let ResolverSettingsRef {
index_locations,
index_strategy,
@ -418,6 +420,16 @@ pub(crate) async fn resolve_environment<'a>(
build_options,
} = settings;
// Respect all requirements from the provided sources.
let RequirementsSpecification {
project,
requirements,
constraints,
overrides,
source_trees,
..
} = spec;
// Determine the tags, markers, and interpreter to use for resolution.
let tags = interpreter.tags()?;
let markers = interpreter.markers();
@ -490,12 +502,12 @@ pub(crate) async fn resolve_environment<'a>(
// Resolve the requirements.
Ok(pip::operations::resolve(
spec.requirements,
spec.constraints,
spec.overrides,
requirements,
constraints,
overrides,
dev,
spec.source_trees,
spec.project,
source_trees,
project,
&extras,
preferences,
EmptyInstalledPackages,
@ -644,6 +656,8 @@ pub(crate) async fn update_environment(
cache: &Cache,
printer: Printer,
) -> anyhow::Result<PythonEnvironment> {
warn_on_requirements_txt_setting(&spec, settings.as_ref().into());
let ResolverInstallerSettings {
index_locations,
index_strategy,
@ -659,14 +673,20 @@ pub(crate) async fn update_environment(
build_options,
} = settings;
// Respect all requirements from the provided sources.
let RequirementsSpecification {
project,
requirements,
constraints,
overrides,
source_trees,
..
} = spec;
// Check if the current environment satisfies the requirements
let site_packages = SitePackages::from_environment(&venv)?;
if spec.source_trees.is_empty()
&& reinstall.is_none()
&& upgrade.is_none()
&& spec.overrides.is_empty()
{
match site_packages.satisfies(&spec.requirements, &spec.constraints)? {
if source_trees.is_empty() && reinstall.is_none() && upgrade.is_none() && overrides.is_empty() {
match site_packages.satisfies(&requirements, &constraints)? {
// If the requirements are already satisfied, we're done.
SatisfiesResult::Fresh {
recursive_requirements,
@ -756,12 +776,12 @@ pub(crate) async fn update_environment(
// Resolve the requirements.
let resolution = match pip::operations::resolve(
spec.requirements,
spec.constraints,
spec.overrides,
requirements,
constraints,
overrides,
dev,
spec.source_trees,
spec.project,
source_trees,
project,
&extras,
preferences,
site_packages.clone(),
@ -816,3 +836,62 @@ pub(crate) async fn update_environment(
Ok(venv)
}
/// Warn if the user provides (e.g.) an `--index-url` in a requirements file.
fn warn_on_requirements_txt_setting(
spec: &RequirementsSpecification,
settings: ResolverSettingsRef<'_>,
) {
let RequirementsSpecification {
index_url,
extra_index_urls,
no_index,
find_links,
no_binary,
no_build,
..
} = spec;
if settings.index_locations.no_index() {
// Nothing to do, we're ignoring the URLs anyway.
} else if *no_index {
warn_user_once!("Ignoring `--no-index` from requirements file. Instead, use the `--no-index` command-line argument, or set `no-index` in a `uv.toml` or `pyproject.toml` file.");
} else {
if let Some(index_url) = index_url {
if settings.index_locations.index() != Some(index_url) {
warn_user_once!(
"Ignoring `--index-url` from requirements file: `{}`. Instead, use the `--index-url` command-line argument, or set `index-url` in a `uv.toml` or `pyproject.toml` file.",
index_url
);
}
}
for extra_index_url in extra_index_urls {
if !settings
.index_locations
.extra_index()
.contains(extra_index_url)
{
warn_user_once!(
"Ignoring `--extra-index-url` from requirements file: `{}`. Instead, use the `--extra-index-url` command-line argument, or set `extra-index-url` in a `uv.toml` or `pyproject.toml` file.`",
extra_index_url
);
}
}
for find_link in find_links {
if !settings.index_locations.flat_index().contains(find_link) {
warn_user_once!(
"Ignoring `--find-links` from requirements file: `{}`. Instead, use the `--find-links` command-line argument, or set `find-links` in a `uv.toml` or `pyproject.toml` file.`",
find_link
);
}
}
}
if !no_binary.is_none() && settings.build_options.no_binary() != no_binary {
warn_user_once!("Ignoring `--no-binary` setting from requirements file. Instead, use the `--no-binary` command-line argument, or set `no-binary` in a `uv.toml` or `pyproject.toml` file.");
}
if !no_build.is_none() && settings.build_options.no_build() != no_build {
warn_user_once!("Ignoring `--no-binary` setting from requirements file. Instead, use the `--no-build` command-line argument, or set `no-build` in a `uv.toml` or `pyproject.toml` file.");
}
}

View file

@ -133,8 +133,9 @@ pub(crate) async fn run(
.into_iter()
.map(Requirement::from)
.collect();
let spec = RequirementsSpecification::from_requirements(requirements);
let environment = CachedEnvironment::get_or_create(
requirements,
spec,
interpreter,
&settings,
&state,

View file

@ -1,54 +1,8 @@
use distribution_types::{InstalledDist, Name};
use pypi_types::Requirement;
use uv_cache::Cache;
use uv_client::{BaseClientBuilder, Connectivity};
use uv_configuration::{Concurrency, PreviewMode};
use uv_installer::SitePackages;
use uv_python::{Interpreter, PythonEnvironment};
use uv_requirements::{RequirementsSource, RequirementsSpecification};
use uv_python::PythonEnvironment;
use uv_tool::entrypoint_paths;
use crate::commands::{project, SharedState};
use crate::printer::Printer;
use crate::settings::ResolverInstallerSettings;
/// Resolve any [`UnnamedRequirements`].
pub(super) async fn resolve_requirements(
requirements: &[RequirementsSource],
interpreter: &Interpreter,
settings: &ResolverInstallerSettings,
state: &SharedState,
preview: PreviewMode,
connectivity: Connectivity,
concurrency: Concurrency,
native_tls: bool,
cache: &Cache,
printer: Printer,
) -> anyhow::Result<Vec<Requirement>> {
let client_builder = BaseClientBuilder::new()
.connectivity(connectivity)
.native_tls(native_tls);
// Parse the requirements.
let spec =
RequirementsSpecification::from_simple_sources(requirements, &client_builder).await?;
// Resolve the parsed requirements.
project::resolve_names(
spec.requirements,
interpreter,
settings,
state,
preview,
connectivity,
concurrency,
native_tls,
cache,
printer,
)
.await
}
/// Return all packages which contain an executable with the given name.
pub(super) fn matching_packages(
name: &str,

View file

@ -8,7 +8,7 @@ use itertools::Itertools;
use owo_colors::OwoColorize;
use tracing::{debug, warn};
use distribution_types::Name;
use distribution_types::{Name, UnresolvedRequirementSpecification};
use pypi_types::Requirement;
use uv_cache::Cache;
use uv_client::{BaseClientBuilder, Connectivity};
@ -28,7 +28,7 @@ use uv_tool::{entrypoint_paths, find_executable_directory, InstalledTools, Tool,
use uv_warnings::{warn_user, warn_user_once};
use crate::commands::reporters::PythonDownloadReporter;
use crate::commands::tool::common::resolve_requirements;
use crate::commands::{
project::{resolve_environment, resolve_names, sync_environment, update_environment},
tool::common::matching_packages,
@ -138,13 +138,21 @@ pub(crate) async fn install(
.unwrap()
};
// Combine the `from` and `with` requirements.
// 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?
};
// Resolve the `--from` and `--with` requirements.
let requirements = {
let mut requirements = Vec::with_capacity(1 + with.len());
requirements.push(from.clone());
requirements.extend(
resolve_requirements(
with,
resolve_names(
spec.requirements.clone(),
&interpreter,
&settings,
&state,
@ -236,9 +244,15 @@ pub(crate) async fn install(
}
}
// Resolve the requirements.
let state = SharedState::default();
let spec = RequirementsSpecification::from_requirements(requirements.clone());
// Create a `RequirementsSpecification` from the resolved requirements, to avoid re-resolving.
let spec = RequirementsSpecification {
requirements: requirements
.iter()
.cloned()
.map(UnresolvedRequirementSpecification::from)
.collect(),
..spec
};
// TODO(zanieb): Build the environment in the cache directory then copy into the tool directory.
// This lets us confirm the environment is valid before removing an existing install. However,

View file

@ -28,7 +28,8 @@ use uv_tool::{entrypoint_paths, InstalledTools};
use uv_warnings::{warn_user, warn_user_once};
use crate::commands::reporters::PythonDownloadReporter;
use crate::commands::tool::common::resolve_requirements;
use crate::commands::project::resolve_names;
use crate::commands::{
project, project::environment::CachedEnvironment, tool::common::matching_packages,
};
@ -332,13 +333,21 @@ async fn get_or_create_environment(
.unwrap()
};
// Combine the `from` and `with` requirements.
// 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?
};
// Resolve the `--from` and `--with` requirements.
let requirements = {
let mut requirements = Vec::with_capacity(1 + with.len());
requirements.push(from.clone());
requirements.extend(
resolve_requirements(
with,
resolve_names(
spec.requirements.clone(),
&interpreter,
settings,
&state,
@ -388,11 +397,20 @@ async fn get_or_create_environment(
}
}
// Create a `RequirementsSpecification` from the resolved requirements, to avoid re-resolving.
let spec = RequirementsSpecification {
requirements: requirements
.into_iter()
.map(UnresolvedRequirementSpecification::from)
.collect(),
..spec
};
// TODO(zanieb): When implementing project-level tools, discover the project and check if it has the tool.
// TODO(zanieb): Determine if we should layer on top of the project environment if it is present.
let environment = CachedEnvironment::get_or_create(
requirements,
spec,
interpreter,
settings,
&state,

View file

@ -1478,7 +1478,7 @@ pub(crate) struct ResolverSettings {
pub(crate) build_options: BuildOptions,
}
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub(crate) struct ResolverSettingsRef<'a> {
pub(crate) index_locations: &'a IndexLocations,
pub(crate) index_strategy: IndexStrategy,

View file

@ -718,3 +718,54 @@ fn run_requirements_txt() -> Result<()> {
Ok(())
}
/// Ignore and warn when (e.g.) the `--index-url` argument is a provided `requirements.txt`.
#[test]
fn run_requirements_txt_arguments() -> Result<()> {
let context = TestContext::new("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! { r#"
[project]
name = "foo"
version = "1.0.0"
requires-python = ">=3.8"
dependencies = ["typing_extensions"]
"#
})?;
let test_script = context.temp_dir.child("main.py");
test_script.write_str(indoc! { r"
import typing_extensions
"
})?;
// Requesting an unsatisfied requirement should install it.
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! { r"
--index-url https://test.pypi.org/simple
idna
"
})?;
uv_snapshot!(context.filters(), context.run().arg("--with-requirements").arg(requirements_txt.as_os_str()).arg("main.py"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv run` is experimental and may change without warning
Resolved 2 packages in [TIME]
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
+ foo==1.0.0 (from file://[TEMP_DIR]/)
+ typing-extensions==4.10.0
warning: Ignoring `--index-url` from requirements file: `https://test.pypi.org/simple`. Instead, use the `--index-url` command-line argument, or set `index-url` in a `uv.toml` or `pyproject.toml` file.
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ idna==3.6
"###);
Ok(())
}

View file

@ -7,6 +7,7 @@ use assert_fs::{
assert::PathAssert,
fixture::{FileTouch, FileWriteStr, PathChild},
};
use indoc::indoc;
use insta::assert_snapshot;
use predicates::prelude::predicate;
@ -1373,6 +1374,134 @@ fn tool_install_requirements_txt() {
});
}
/// Ignore and warn when (e.g.) the `--index-url` argument is a provided `requirements.txt`.
#[test]
fn tool_install_requirements_txt_arguments() {
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");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt
.write_str(indoc! { r"
--index-url https://test.pypi.org/simple
idna
"
})
.unwrap();
// Install `black`
uv_snapshot!(context.filters(), context.tool_install()
.arg("black")
.arg("--with-requirements")
.arg("requirements.txt")
.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
warning: Ignoring `--index-url` from requirements file: `https://test.pypi.org/simple`. Instead, use the `--index-url` command-line argument, or set `index-url` in a `uv.toml` or `pyproject.toml` file.
Resolved 7 packages in [TIME]
Prepared 7 packages in [TIME]
Installed 7 packages in [TIME]
+ black==24.3.0
+ click==8.1.7
+ idna==3.6
+ 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",
"idna",
]
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});
// Don't warn, though, if the index URL is the same as the default or as settings.
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt
.write_str(indoc! { r"
--index-url https://pypi.org/simple
idna
"
})
.unwrap();
// Install `black`
uv_snapshot!(context.filters(), context.tool_install()
.arg("black")
.arg("--with-requirements")
.arg("requirements.txt")
.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 2 executables: black, blackd
"###);
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt
.write_str(indoc! { r"
--index-url https://test.pypi.org/simple
idna
"
})
.unwrap();
// Install `flask`
uv_snapshot!(context.filters(), context.tool_install()
.arg("flask")
.arg("--with-requirements")
.arg("requirements.txt")
.arg("--index-url")
.arg("https://test.pypi.org/simple")
.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 8 packages in [TIME]
Prepared 8 packages in [TIME]
Installed 8 packages in [TIME]
+ blinker==1.7.0
+ click==8.1.7
+ flask==3.0.2
+ idna==2.7
+ itsdangerous==2.1.2
+ jinja2==3.1.3
+ markupsafe==2.1.5
+ werkzeug==3.0.1
Installed 1 executable: flask
"###);
}
/// Test upgrading an already installed tool.
#[test]
fn tool_install_upgrade() {

View file

@ -2,8 +2,8 @@
use assert_cmd::prelude::*;
use assert_fs::prelude::*;
use common::{uv_snapshot, TestContext};
use indoc::indoc;
mod common;
@ -663,3 +663,51 @@ fn tool_run_requirements_txt() {
+ werkzeug==3.0.1
"###);
}
/// Ignore and warn when (e.g.) the `--index-url` argument is a provided `requirements.txt`.
#[test]
fn tool_run_requirements_txt_arguments() {
let context = TestContext::new("3.12").with_filtered_counts();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt
.write_str(indoc! { r"
--index-url https://test.pypi.org/simple
idna
"
})
.unwrap();
// We treat arguments before the command as uv arguments
uv_snapshot!(context.filters(), context.tool_run()
.arg("--with-requirements")
.arg("requirements.txt")
.arg("flask")
.arg("--version")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
.env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
Python 3.12.[X]
Flask 3.0.2
Werkzeug 3.0.1
----- stderr -----
warning: `uv tool run` is experimental and may change without warning
warning: Ignoring `--index-url` from requirements file: `https://test.pypi.org/simple`. Instead, use the `--index-url` command-line argument, or set `index-url` in a `uv.toml` or `pyproject.toml` file.
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ blinker==1.7.0
+ click==8.1.7
+ flask==3.0.2
+ idna==3.6
+ itsdangerous==2.1.2
+ jinja2==3.1.3
+ markupsafe==2.1.5
+ werkzeug==3.0.1
"###);
}