Skip to content

Commit

Permalink
feat: add support for --no-extra flag and setting (#9387)
Browse files Browse the repository at this point in the history
<!--  
Thank you for contributing to uv! To help us review effectively, please
ensure that:
- The pull request includes a summary of the change.  
- The title is descriptive and concise.  
- Relevant issues are referenced where applicable.  
-->

## Summary

Resolves #9333  

This pull request introduces support for the `--no-extra` command-line
flag and the corresponding `no-extra` UV setting.

### Behavior
- When `--all-extras` is supplied, the specified extras in `--no-extra`
will be excluded from the installation.
- If `--all-extras` is not supplied, `--no-extra` has no effect and is
safely ignored.

## Test Plan

Since `ExtrasSpecification::from_args` and
`ExtrasSpecification::extra_names` are the most important parts in the
implementation, I added the following tests in the
`uv-configuration/src/extras.rs` module:

- **`test_no_extra_full`**: Verifies behavior when `no_extra` includes
the entire list of extras.
- **`test_no_extra_partial`**: Tests partial exclusion, ensuring only
specified extras are excluded.
- **`test_no_extra_empty`**: Confirms that no extras are excluded if
`no_extra` is empty.
- **`test_no_extra_excessive`**: Ensures the implementation ignores
`no_extra` values that don't match any available extras.
- **`test_no_extra_without_all_extras`**: Validates that `no_extra` has
no effect when `--all-extras` is not supplied.
- **`test_no_extra_without_package_extras`**: Confirms correct behavior
when no extras are available in the package.
- **`test_no_extra_duplicates`**: Verifies that duplicate entries in
`pkg_extras` or `no_extra` do not cause errors.

---------

Co-authored-by: Charlie Marsh <[email protected]>
  • Loading branch information
alan910127 and charliermarsh authored Nov 24, 2024
1 parent c63616c commit e485dfd
Show file tree
Hide file tree
Showing 12 changed files with 297 additions and 32 deletions.
18 changes: 18 additions & 0 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2599,6 +2599,12 @@ pub struct RunArgs {
#[arg(long, conflicts_with = "extra")]
pub all_extras: bool,

/// Exclude the specified optional dependencies, if `--all-extras` is supplied.
///
/// May be provided multiple times.
#[arg(long)]
pub no_extra: Vec<ExtraName>,

#[arg(long, overrides_with("all_extras"), hide = true)]
pub no_all_extras: bool,

Expand Down Expand Up @@ -2836,6 +2842,12 @@ pub struct SyncArgs {
#[arg(long, conflicts_with = "extra")]
pub all_extras: bool,

/// Exclude the specified optional dependencies, if `--all-extras` is supplied.
///
/// May be provided multiple times.
#[arg(long)]
pub no_extra: Vec<ExtraName>,

#[arg(long, overrides_with("all_extras"), hide = true)]
pub no_all_extras: bool,

Expand Down Expand Up @@ -3418,6 +3430,12 @@ pub struct ExportArgs {
#[arg(long, conflicts_with = "extra")]
pub all_extras: bool,

/// Exclude the specified optional dependencies, if `--all-extras` is supplied.
///
/// May be provided multiple times.
#[arg(long)]
pub no_extra: Vec<ExtraName>,

#[arg(long, overrides_with("all_extras"), hide = true)]
pub no_all_extras: bool,

Expand Down
143 changes: 141 additions & 2 deletions crates/uv-configuration/src/extras.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rustc_hash::FxHashSet;
use uv_normalize::ExtraName;

#[derive(Debug, Default, Clone)]
Expand All @@ -6,16 +7,25 @@ pub enum ExtrasSpecification {
None,
All,
Some(Vec<ExtraName>),
Exclude(FxHashSet<ExtraName>),
}

impl ExtrasSpecification {
/// Determine the extras specification to use based on the command-line arguments.
pub fn from_args(all_extras: bool, extra: Vec<ExtraName>) -> Self {
if all_extras {
pub fn from_args(
all_extras: bool,
no_extra: Vec<ExtraName>,
mut extra: Vec<ExtraName>,
) -> Self {
if all_extras && !no_extra.is_empty() {
ExtrasSpecification::Exclude(FxHashSet::from_iter(no_extra))
} else if all_extras {
ExtrasSpecification::All
} else if extra.is_empty() {
ExtrasSpecification::None
} else {
// If a package is included in both `no_extra` and `extra`, it should be excluded.
extra.retain(|name| !no_extra.contains(name));
ExtrasSpecification::Some(extra)
}
}
Expand All @@ -26,10 +36,139 @@ impl ExtrasSpecification {
ExtrasSpecification::All => true,
ExtrasSpecification::None => false,
ExtrasSpecification::Some(extras) => extras.contains(name),
ExtrasSpecification::Exclude(excluded) => !excluded.contains(name),
}
}

pub fn is_empty(&self) -> bool {
matches!(self, ExtrasSpecification::None)
}

pub fn extra_names<'a, Names>(&'a self, all_names: Names) -> ExtrasIter<'a, Names>
where
Names: Iterator<Item = &'a ExtraName>,
{
match self {
ExtrasSpecification::All => ExtrasIter::All(all_names),
ExtrasSpecification::None => ExtrasIter::None,
ExtrasSpecification::Some(extras) => ExtrasIter::Some(extras.iter()),
ExtrasSpecification::Exclude(excluded) => ExtrasIter::Exclude(all_names, excluded),
}
}
}

/// An iterator over the extra names to include.
#[derive(Debug)]
pub enum ExtrasIter<'a, Names: Iterator<Item = &'a ExtraName>> {
None,
All(Names),
Some(std::slice::Iter<'a, ExtraName>),
Exclude(Names, &'a FxHashSet<ExtraName>),
}

impl<'a, Names: Iterator<Item = &'a ExtraName>> Iterator for ExtrasIter<'a, Names> {
type Item = &'a ExtraName;

fn next(&mut self) -> Option<Self::Item> {
match self {
Self::All(names) => names.next(),
Self::None => None,
Self::Some(extras) => extras.next(),
Self::Exclude(names, excluded) => {
for name in names.by_ref() {
if !excluded.contains(name) {
return Some(name);
}
}
None
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;

macro_rules! extras {
() => (
Vec::new()
);
($($x:expr),+ $(,)?) => (
vec![$(ExtraName::new($x.into()).unwrap()),+]
)
}

#[test]
fn test_no_extra_full() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-2"];
let no_extra = extras!["dev", "docs", "extra-1", "extra-2"];
let spec = ExtrasSpecification::from_args(true, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras![]);
}

#[test]
fn test_no_extra_partial() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-2"];
let no_extra = extras!["extra-1", "extra-2"];
let spec = ExtrasSpecification::from_args(true, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras!["dev", "docs"]);
}

#[test]
fn test_no_extra_empty() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-2"];
let no_extra = extras![];
let spec = ExtrasSpecification::from_args(true, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras!["dev", "docs", "extra-1", "extra-2"]);
}

#[test]
fn test_no_extra_excessive() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-2"];
let no_extra = extras!["does-not-exists"];
let spec = ExtrasSpecification::from_args(true, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras!["dev", "docs", "extra-1", "extra-2"]);
}

#[test]
fn test_no_extra_without_all_extras() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-2"];
let no_extra = extras!["extra-1", "extra-2"];
let spec = ExtrasSpecification::from_args(false, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras![]);
}

#[test]
fn test_no_extra_without_package_extras() {
let pkg_extras = extras![];
let no_extra = extras!["extra-1", "extra-2"];
let spec = ExtrasSpecification::from_args(true, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras![]);
}

#[test]
fn test_no_extra_duplicates() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-1", "extra-2"];
let no_extra = extras!["extra-1", "extra-2"];
let spec = ExtrasSpecification::from_args(true, no_extra, vec![]);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras!["dev", "docs"]);
}

#[test]
fn test_no_extra_extra() {
let pkg_extras = extras!["dev", "docs", "extra-1", "extra-2"];
let no_extra = extras!["extra-1", "extra-2"];
let extra = extras!["extra-1", "extra-2", "docs"];
let spec = ExtrasSpecification::from_args(false, no_extra, extra);
let result: Vec<_> = spec.extra_names(pkg_extras.iter()).cloned().collect();
assert_eq!(result, extras!["docs"]);
}
}
12 changes: 6 additions & 6 deletions crates/uv-requirements/src/source_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> {
let origin = RequirementOrigin::Project(path.to_path_buf(), metadata.name.clone());

// Determine the extras to include when resolving the requirements.
let extras = match self.extras {
ExtrasSpecification::All => metadata.provides_extras.as_slice(),
ExtrasSpecification::None => &[],
ExtrasSpecification::Some(extras) => extras,
};
let extras: Vec<_> = self
.extras
.extra_names(metadata.provides_extras.iter())
.cloned()
.collect();

// Determine the appropriate requirements to return based on the extras. This involves
// evaluating the `extras` expression in any markers, but preserving the remaining marker
Expand All @@ -103,7 +103,7 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> {
.into_iter()
.map(|requirement| Requirement {
origin: Some(origin.clone()),
marker: requirement.marker.simplify_extras(extras),
marker: requirement.marker.simplify_extras(&extras),
..requirement
})
.collect();
Expand Down
14 changes: 2 additions & 12 deletions crates/uv-resolver/src/lock/requirements_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,8 @@ impl<'lock> RequirementsTxtExport<'lock> {

// Push its dependencies on the queue.
queue.push_back((dist, None));
match extras {
ExtrasSpecification::None => {}
ExtrasSpecification::All => {
for extra in dist.optional_dependencies.keys() {
queue.push_back((dist, Some(extra)));
}
}
ExtrasSpecification::Some(extras) => {
for extra in extras {
queue.push_back((dist, Some(extra)));
}
}
for extra in extras.extra_names(dist.optional_dependencies.keys()) {
queue.push_back((dist, Some(extra)));
}
}

Expand Down
14 changes: 2 additions & 12 deletions crates/uv-resolver/src/lock/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,8 @@ impl<'env> InstallTarget<'env> {
if dev.prod() {
// Push its dependencies on the queue.
queue.push_back((dist, None));
match extras {
ExtrasSpecification::None => {}
ExtrasSpecification::All => {
for extra in dist.optional_dependencies.keys() {
queue.push_back((dist, Some(extra)));
}
}
ExtrasSpecification::Some(extras) => {
for extra in extras {
queue.push_back((dist, Some(extra)));
}
}
for extra in extras.extra_names(dist.optional_dependencies.keys()) {
queue.push_back((dist, Some(extra)));
}
}

Expand Down
10 changes: 10 additions & 0 deletions crates/uv-settings/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,16 @@ pub struct PipOptions {
"#
)]
pub all_extras: Option<bool>,
/// Exclude the specified optional dependencies if `all-extras` is supplied.
#[option(
default = "[]",
value_type = "list[str]",
example = r#"
all-extras = true
no-extra = ["dev", "docs"]
"#
)]
pub no_extra: Option<Vec<ExtraName>>,
/// Ignore package dependencies, instead only add those packages explicitly listed
/// on the command line to the resulting the requirements file.
#[option(
Expand Down
8 changes: 8 additions & 0 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ impl RunSettings {
let RunArgs {
extra,
all_extras,
no_extra,
no_all_extras,
dev,
no_dev,
Expand Down Expand Up @@ -323,6 +324,7 @@ impl RunSettings {
frozen,
extras: ExtrasSpecification::from_args(
flag(all_extras, no_all_extras).unwrap_or_default(),
no_extra,
extra.unwrap_or_default(),
),
dev: DevGroupsSpecification::from_args(
Expand Down Expand Up @@ -884,6 +886,7 @@ impl SyncSettings {
let SyncArgs {
extra,
all_extras,
no_extra,
no_all_extras,
dev,
no_dev,
Expand Down Expand Up @@ -922,6 +925,7 @@ impl SyncSettings {
frozen,
extras: ExtrasSpecification::from_args(
flag(all_extras, no_all_extras).unwrap_or_default(),
no_extra,
extra.unwrap_or_default(),
),
dev: DevGroupsSpecification::from_args(
Expand Down Expand Up @@ -1306,6 +1310,7 @@ impl ExportSettings {
prune,
extra,
all_extras,
no_extra,
no_all_extras,
dev,
no_dev,
Expand Down Expand Up @@ -1342,6 +1347,7 @@ impl ExportSettings {
prune,
extras: ExtrasSpecification::from_args(
flag(all_extras, no_all_extras).unwrap_or_default(),
no_extra,
extra.unwrap_or_default(),
),
dev: DevGroupsSpecification::from_args(
Expand Down Expand Up @@ -2490,6 +2496,7 @@ impl PipSettings {
strict,
extra,
all_extras,
no_extra,
no_deps,
allow_empty_requirements,
resolution,
Expand Down Expand Up @@ -2601,6 +2608,7 @@ impl PipSettings {
),
extras: ExtrasSpecification::from_args(
args.all_extras.combine(all_extras).unwrap_or_default(),
args.no_extra.combine(no_extra).unwrap_or_default(),
args.extra.combine(extra).unwrap_or_default(),
),
dependency_mode: if args.no_deps.combine(no_deps).unwrap_or_default() {
Expand Down
Loading

0 comments on commit e485dfd

Please sign in to comment.