Skip to content

Commit

Permalink
Apply combination logic to merge CLI and persistent configuration (as…
Browse files Browse the repository at this point in the history
…tral-sh#3618)

## Summary

If you have (e.g.) `extra-index-url` in your configuration file _and_
provide `--extra-index-url` on the command-line, we now merge the
options rather than ignoring those in the configuration file. As such,
merging the CLI and the persistent configuration is now semantically
identical to how we merge (project persistent configuration) with (user
persistent configuration).

Closes astral-sh#3541.
  • Loading branch information
charliermarsh authored May 20, 2024
1 parent f3965fe commit c32fb86
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 51 deletions.
124 changes: 77 additions & 47 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use uv_interpreter::{PythonVersion, Target};
use uv_normalize::PackageName;
use uv_requirements::ExtrasSpecification;
use uv_resolver::{AnnotationStyle, DependencyMode, ExcludeNewer, PreReleaseMode, ResolutionMode};
use uv_workspace::{PipOptions, Workspace};
use uv_workspace::{Combine, PipOptions, Workspace};

use crate::cli::{
ColorChoice, GlobalArgs, LockArgs, Maybe, PipCheckArgs, PipCompileArgs, PipFreezeArgs,
Expand Down Expand Up @@ -50,12 +50,12 @@ impl GlobalSettings {
args.color
},
native_tls: flag(args.native_tls, args.no_native_tls)
.or(workspace.and_then(|workspace| workspace.options.native_tls))
.combine(workspace.and_then(|workspace| workspace.options.native_tls))
.unwrap_or(false),
isolated: args.isolated,
preview: PreviewMode::from(
flag(args.preview, args.no_preview)
.or(workspace.and_then(|workspace| workspace.options.preview))
.combine(workspace.and_then(|workspace| workspace.options.preview))
.unwrap_or(false),
),
}
Expand Down Expand Up @@ -965,99 +965,129 @@ impl PipSharedSettings {

Self {
index_locations: IndexLocations::new(
args.index_url.or(index_url),
args.extra_index_url.or(extra_index_url).unwrap_or_default(),
args.find_links.or(find_links).unwrap_or_default(),
args.no_index.or(no_index).unwrap_or_default(),
args.index_url.combine(index_url),
args.extra_index_url
.combine(extra_index_url)
.unwrap_or_default(),
args.find_links.combine(find_links).unwrap_or_default(),
args.no_index.combine(no_index).unwrap_or_default(),
),
extras: ExtrasSpecification::from_args(
args.all_extras.or(all_extras).unwrap_or_default(),
args.extra.or(extra).unwrap_or_default(),
args.all_extras.combine(all_extras).unwrap_or_default(),
args.extra.combine(extra).unwrap_or_default(),
),
dependency_mode: if args.no_deps.or(no_deps).unwrap_or_default() {
dependency_mode: if args.no_deps.combine(no_deps).unwrap_or_default() {
DependencyMode::Direct
} else {
DependencyMode::Transitive
},
resolution: args.resolution.or(resolution).unwrap_or_default(),
prerelease: args.prerelease.or(prerelease).unwrap_or_default(),
output_file: args.output_file.or(output_file),
no_strip_extras: args.no_strip_extras.or(no_strip_extras).unwrap_or_default(),
no_annotate: args.no_annotate.or(no_annotate).unwrap_or_default(),
no_header: args.no_header.or(no_header).unwrap_or_default(),
custom_compile_command: args.custom_compile_command.or(custom_compile_command),
resolution: args.resolution.combine(resolution).unwrap_or_default(),
prerelease: args.prerelease.combine(prerelease).unwrap_or_default(),
output_file: args.output_file.combine(output_file),
no_strip_extras: args
.no_strip_extras
.combine(no_strip_extras)
.unwrap_or_default(),
no_annotate: args.no_annotate.combine(no_annotate).unwrap_or_default(),
no_header: args.no_header.combine(no_header).unwrap_or_default(),
custom_compile_command: args.custom_compile_command.combine(custom_compile_command),
annotation_style: args
.annotation_style
.or(annotation_style)
.combine(annotation_style)
.unwrap_or_default(),
connectivity: if args.offline.or(offline).unwrap_or_default() {
connectivity: if args.offline.combine(offline).unwrap_or_default() {
Connectivity::Offline
} else {
Connectivity::Online
},
index_strategy: args.index_strategy.or(index_strategy).unwrap_or_default(),
index_strategy: args
.index_strategy
.combine(index_strategy)
.unwrap_or_default(),
keyring_provider: args
.keyring_provider
.or(keyring_provider)
.combine(keyring_provider)
.unwrap_or_default(),
generate_hashes: args.generate_hashes.or(generate_hashes).unwrap_or_default(),
setup_py: if args.legacy_setup_py.or(legacy_setup_py).unwrap_or_default() {
generate_hashes: args
.generate_hashes
.combine(generate_hashes)
.unwrap_or_default(),
setup_py: if args
.legacy_setup_py
.combine(legacy_setup_py)
.unwrap_or_default()
{
SetupPyStrategy::Setuptools
} else {
SetupPyStrategy::Pep517
},
no_build_isolation: args
.no_build_isolation
.or(no_build_isolation)
.combine(no_build_isolation)
.unwrap_or_default(),
no_build: NoBuild::from_args(
args.only_binary.or(only_binary).unwrap_or_default(),
args.no_build.or(no_build).unwrap_or_default(),
args.only_binary.combine(only_binary).unwrap_or_default(),
args.no_build.combine(no_build).unwrap_or_default(),
),
config_setting: args.config_settings.or(config_settings).unwrap_or_default(),
python_version: args.python_version.or(python_version),
python_platform: args.python_platform.or(python_platform),
exclude_newer: args.exclude_newer.or(exclude_newer),
no_emit_package: args.no_emit_package.or(no_emit_package).unwrap_or_default(),
emit_index_url: args.emit_index_url.or(emit_index_url).unwrap_or_default(),
emit_find_links: args.emit_find_links.or(emit_find_links).unwrap_or_default(),
config_setting: args
.config_settings
.combine(config_settings)
.unwrap_or_default(),
python_version: args.python_version.combine(python_version),
python_platform: args.python_platform.combine(python_platform),
exclude_newer: args.exclude_newer.combine(exclude_newer),
no_emit_package: args
.no_emit_package
.combine(no_emit_package)
.unwrap_or_default(),
emit_index_url: args
.emit_index_url
.combine(emit_index_url)
.unwrap_or_default(),
emit_find_links: args
.emit_find_links
.combine(emit_find_links)
.unwrap_or_default(),
emit_marker_expression: args
.emit_marker_expression
.or(emit_marker_expression)
.combine(emit_marker_expression)
.unwrap_or_default(),
emit_index_annotation: args
.emit_index_annotation
.or(emit_index_annotation)
.combine(emit_index_annotation)
.unwrap_or_default(),
link_mode: args.link_mode.combine(link_mode).unwrap_or_default(),
require_hashes: args
.require_hashes
.combine(require_hashes)
.unwrap_or_default(),
link_mode: args.link_mode.or(link_mode).unwrap_or_default(),
require_hashes: args.require_hashes.or(require_hashes).unwrap_or_default(),
python: args.python.or(python),
system: args.system.or(system).unwrap_or_default(),
python: args.python.combine(python),
system: args.system.combine(system).unwrap_or_default(),
break_system_packages: args
.break_system_packages
.or(break_system_packages)
.combine(break_system_packages)
.unwrap_or_default(),
target: args.target.or(target).map(Target::from),
no_binary: NoBinary::from_args(args.no_binary.or(no_binary).unwrap_or_default()),
target: args.target.combine(target).map(Target::from),
no_binary: NoBinary::from_args(args.no_binary.combine(no_binary).unwrap_or_default()),
compile_bytecode: args
.compile_bytecode
.or(compile_bytecode)
.combine(compile_bytecode)
.unwrap_or_default(),
strict: args.strict.or(strict).unwrap_or_default(),
strict: args.strict.combine(strict).unwrap_or_default(),
concurrency: Concurrency {
downloads: args
.concurrent_downloads
.or(concurrent_downloads)
.combine(concurrent_downloads)
.map(NonZeroUsize::get)
.unwrap_or(Concurrency::DEFAULT_DOWNLOADS),
builds: args
.concurrent_builds
.or(concurrent_builds)
.combine(concurrent_builds)
.map(NonZeroUsize::get)
.unwrap_or_else(Concurrency::threads),
installs: args
.concurrent_installs
.or(concurrent_installs)
.combine(concurrent_installs)
.map(NonZeroUsize::get)
.unwrap_or_else(Concurrency::threads),
},
Expand Down
82 changes: 78 additions & 4 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8027,9 +8027,8 @@ requires-python = ">3.8"

/// Install a package via `--extra-index-url`.
///
/// If the package exists exist on the "extra" index, but at an incompatible version, the
/// resolution should fail by default (even though a compatible version exists on the "primary"
/// index).
/// If the package exists on the "extra" index, but at an incompatible version, the resolution
/// should fail by default (even though a compatible version exists on the "primary" index).
#[test]
fn compile_index_url_first_match() -> Result<()> {
let context = TestContext::new("3.12");
Expand Down Expand Up @@ -8690,13 +8689,88 @@ fn resolve_configuration() -> Result<()> {
"###
);

// Write out a `--find-links` entry.
// Add an extra index URL entry to the `pyproject.toml` file.
pyproject.write_str(indoc::indoc! {r#"
[project]
name = "example"
version = "0.0.0"
[tool.uv.pip]
index-url = "https://test.pypi.org/simple"
extra-index-url = ["https://pypi.org/simple"]
"#})?;

// Resolution should succeed, since the PyPI index is preferred.
uv_snapshot!(context.compile()
.arg("requirements.in"), @r###"
success: true
exit_code: 0
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in
anyio==4.3.0
# via -r requirements.in
idna==3.6
# via anyio
sniffio==1.3.1
# via anyio
----- stderr -----
Resolved 3 packages in [TIME]
"###
);

// Providing an additional index URL on the command-line should fail, since it will be
// preferred (but the test index alone can't satisfy the requirements).
uv_snapshot!(context.compile()
.arg("requirements.in")
.arg("--extra-index-url")
.arg("https://test.pypi.org/simple"), @r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because only idna<2.8 is available and anyio==3.5.0 depends on idna>=2.8, we can conclude that anyio==3.5.0 cannot be used.
And because only the following versions of anyio are available:
anyio<=3.0.0
anyio==3.5.0
and you require anyio, we can conclude that the requirements are unsatisfiable.
"###
);

// If we allow the resolver to use _any_ index, it should succeed, since it now has _both_
// the test and PyPI indexes in its `--extra-index-url`.
uv_snapshot!(context.compile()
.arg("requirements.in")
.arg("--extra-index-url")
.arg("https://test.pypi.org/simple")
.arg("--index-strategy")
.arg("unsafe-best-match"), @r###"
success: true
exit_code: 0
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in --index-strategy unsafe-best-match
anyio==4.3.0
# via -r requirements.in
idna==3.6
# via anyio
sniffio==1.3.1
# via anyio
----- stderr -----
Resolved 3 packages in [TIME]
"###
);

// Write out a `--find-links` entry.
pyproject.write_str(indoc::indoc! {r#"
[project]
name = "example"
version = "0.0.0"
[tool.uv.pip]
no-index = true
find-links = ["https://download.pytorch.org/whl/torch_stable.html"]
Expand Down

0 comments on commit c32fb86

Please sign in to comment.