Skip to content

Commit

Permalink
Surface extras and group conflicts in uv export (#9365)
Browse files Browse the repository at this point in the history
## Summary

Closes #9364.
  • Loading branch information
charliermarsh authored Nov 22, 2024
1 parent 536d038 commit 1744a9b
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 39 deletions.
9 changes: 7 additions & 2 deletions crates/uv/src/commands/project/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use uv_workspace::{DiscoveryOptions, MemberDiscovery, VirtualProject, Workspace}
use crate::commands::pip::loggers::DefaultResolveLogger;
use crate::commands::project::lock::{do_safe_lock, LockMode};
use crate::commands::project::{
default_dependency_groups, DependencyGroupsTarget, ProjectError, ProjectInterpreter,
default_dependency_groups, detect_conflicts, DependencyGroupsTarget, ProjectError,
ProjectInterpreter,
};
use crate::commands::{diagnostics, ExitStatus, OutputWriter, SharedState};
use crate::printer::Printer;
Expand Down Expand Up @@ -93,6 +94,7 @@ pub(crate) async fn export(

// Determine the default groups to include.
let defaults = default_dependency_groups(project.current_project().pyproject_toml())?;
let dev = dev.with_defaults(defaults);

// Determine the lock mode.
let interpreter;
Expand Down Expand Up @@ -153,6 +155,9 @@ pub(crate) async fn export(
Err(err) => return Err(err.into()),
};

// Validate that the set of requested extras and development groups are compatible.
detect_conflicts(&lock, &extras, &dev)?;

// Identify the installation target.
let target = if all_packages {
InstallTarget::Workspace {
Expand All @@ -179,7 +184,7 @@ pub(crate) async fn export(
let export = RequirementsTxtExport::from_lock(
target,
&extras,
&dev.with_defaults(defaults),
&dev,
editable,
hashes,
&install_options,
Expand Down
45 changes: 43 additions & 2 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use tracing::debug;
use uv_cache::Cache;
use uv_client::{BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClientBuilder};
use uv_configuration::{
Concurrency, Constraints, DevGroupsSpecification, ExtrasSpecification, GroupsSpecification,
LowerBound, Reinstall, TrustedHost, Upgrade,
Concurrency, Constraints, DevGroupsManifest, DevGroupsSpecification, ExtrasSpecification,
GroupsSpecification, LowerBound, Reinstall, TrustedHost, Upgrade,
};
use uv_dispatch::BuildDispatch;
use uv_distribution::DistributionDatabase;
Expand Down Expand Up @@ -1630,6 +1630,47 @@ pub(crate) fn default_dependency_groups(
}
}

/// Validate that we aren't trying to install extras or groups that
/// are declared as conflicting.
#[allow(clippy::result_large_err)]
pub(crate) fn detect_conflicts(
lock: &Lock,
extras: &ExtrasSpecification,
dev: &DevGroupsManifest,
) -> Result<(), ProjectError> {
// Note that we need to collect all extras and groups that match in
// a particular set, since extras can be declared as conflicting with
// groups. So if extra `x` and group `g` are declared as conflicting,
// then enabling both of those should result in an error.
let conflicts = lock.conflicts();
for set in conflicts.iter() {
let mut conflicts: Vec<ConflictPackage> = vec![];
for item in set.iter() {
if item
.extra()
.map(|extra| extras.contains(extra))
.unwrap_or(false)
{
conflicts.push(item.conflict().clone());
}
if item
.group()
.map(|group| dev.contains(group))
.unwrap_or(false)
{
conflicts.push(item.conflict().clone());
}
}
if conflicts.len() >= 2 {
return Err(ProjectError::ConflictIncompatibility(
set.clone(),
conflicts,
));
}
}
Ok(())
}

/// Warn if the user provides (e.g.) an `--index-url` in a requirements file.
fn warn_on_requirements_txt_setting(
spec: &RequirementsSpecification,
Expand Down
39 changes: 4 additions & 35 deletions crates/uv/src/commands/project/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ use uv_installer::SitePackages;
use uv_normalize::PackageName;
use uv_pep508::{MarkerTree, Requirement, VersionOrUrl};
use uv_pypi_types::{
ConflictPackage, LenientRequirement, ParsedArchiveUrl, ParsedGitUrl, ParsedUrl,
VerbatimParsedUrl,
LenientRequirement, ParsedArchiveUrl, ParsedGitUrl, ParsedUrl, VerbatimParsedUrl,
};
use uv_python::{PythonDownloads, PythonEnvironment, PythonPreference, PythonRequest};
use uv_resolver::{FlatIndex, InstallTarget};
Expand All @@ -36,7 +35,7 @@ use crate::commands::pip::operations;
use crate::commands::pip::operations::Modifications;
use crate::commands::project::lock::{do_safe_lock, LockMode};
use crate::commands::project::{
default_dependency_groups, DependencyGroupsTarget, ProjectError, SharedState,
default_dependency_groups, detect_conflicts, DependencyGroupsTarget, ProjectError, SharedState,
};
use crate::commands::{diagnostics, project, ExitStatus};
use crate::printer::Printer;
Expand Down Expand Up @@ -304,38 +303,8 @@ pub(super) async fn do_sync(
));
}

// Validate that we aren't trying to install extras or groups that
// are declared as conflicting. Note that we need to collect all
// extras and groups that match in a particular set, since extras
// can be declared as conflicting with groups. So if extra `x` and
// group `g` are declared as conflicting, then enabling both of
// those should result in an error.
let conflicts = target.lock().conflicts();
for set in conflicts.iter() {
let mut conflicts: Vec<ConflictPackage> = vec![];
for item in set.iter() {
if item
.extra()
.map(|extra| extras.contains(extra))
.unwrap_or(false)
{
conflicts.push(item.conflict().clone());
}
if item
.group()
.map(|group| dev.contains(group))
.unwrap_or(false)
{
conflicts.push(item.conflict().clone());
}
}
if conflicts.len() >= 2 {
return Err(ProjectError::ConflictIncompatibility(
set.clone(),
conflicts,
));
}
}
// Validate that the set of requested extras and development groups are compatible.
detect_conflicts(target.lock(), extras, dev)?;

// Determine the markers to use for resolution.
let marker_env = venv.interpreter().resolver_marker_environment();
Expand Down
9 changes: 9 additions & 0 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2360,6 +2360,15 @@ fn lock_conflicting_extra_basic() -> Result<()> {
exit_code: 2
----- stdout -----

----- stderr -----
error: extra `project1`, extra `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`}
"###);
// As should exporting them.
uv_snapshot!(context.filters(), context.export().arg("--frozen").arg("--all-extras"), @r###"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: extra `project1`, extra `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`}
"###);
Expand Down

0 comments on commit 1744a9b

Please sign in to comment.