fix handling of --all-groups and --no-default-groups flags (#11224)

This is a rewrite of the groups subsystem to have more clear semantics,
and some adjustments to the CLI flag constraints. In doing so, the
following bugs are fixed:

* `--no-default-groups --no-group foo` is no longer needlessly rejected
* `--all-groups --no-default-groups` now correctly evaluates to
`--all-groups` where previously it was erroneously being interpretted as
just `--no-default-groups`
* `--all-groups --only-dev` is now illegal, where previously it was
accepted and mishandled, as if it was a mythical `--only-all-groups`
flag

Fixes #10890
Closes #10891
This commit is contained in:
Aria Desires 2025-02-05 15:31:23 -05:00 committed by GitHub
parent 311a96bd28
commit 72d9361ce1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 1198 additions and 605 deletions

View file

@ -1,9 +1,304 @@
use std::borrow::Cow;
use either::Either;
use std::{borrow::Cow, sync::Arc};
use uv_normalize::{GroupName, DEV_DEPENDENCIES};
/// Manager of all dependency-group decisions and settings history.
///
/// This is an Arc mostly just to avoid size bloat on things that contain these.
#[derive(Debug, Default, Clone)]
pub struct DevGroupsSpecification(Arc<DevGroupsSpecificationInner>);
/// Manager of all dependency-group decisions and settings history.
#[derive(Debug, Default, Clone)]
pub struct DevGroupsSpecificationInner {
/// Groups to include.
include: IncludeGroups,
/// Groups to exclude (always wins over include).
exclude: Vec<GroupName>,
/// Whether an `--only` flag was passed.
///
/// If true, users of this API should refrain from looking at packages
/// that *aren't* specified by the dependency-groups. This is exposed
/// via [`DevGroupsSpecificationInner::prod`][].
only_groups: bool,
/// The "raw" flags/settings we were passed for diagnostics.
history: DevGroupsSpecificationHistory,
}
impl DevGroupsSpecification {
/// Create from history.
///
/// This is the "real" constructor, it's basically taking raw CLI flags but in
/// a way that's a bit nicer for other constructors to use.
fn from_history(history: DevGroupsSpecificationHistory) -> Self {
let DevGroupsSpecificationHistory {
dev_mode,
mut group,
mut only_group,
mut no_group,
all_groups,
no_default_groups,
mut defaults,
} = history.clone();
// First desugar --dev flags
match dev_mode {
Some(DevMode::Include) => group.push(DEV_DEPENDENCIES.clone()),
Some(DevMode::Only) => only_group.push(DEV_DEPENDENCIES.clone()),
Some(DevMode::Exclude) => no_group.push(DEV_DEPENDENCIES.clone()),
None => {}
}
// `group` and `only_group` actually have the same meanings: packages to include.
// But if `only_group` is non-empty then *other* packages should be excluded.
// So we just record whether it was and then treat the two lists as equivalent.
let only_groups = !only_group.is_empty();
// --only flags imply --no-default-groups
let default_groups = !no_default_groups && !only_groups;
let include = if all_groups {
// If this is set we can ignore group/only_group/defaults as irrelevant
// (`--all-groups --only-*` is rejected at the CLI level, don't worry about it).
IncludeGroups::All
} else {
// Merge all these lists, they're equivalent now
group.append(&mut only_group);
if default_groups {
group.append(&mut defaults);
}
IncludeGroups::Some(group)
};
Self(Arc::new(DevGroupsSpecificationInner {
include,
exclude: no_group,
only_groups,
history,
}))
}
/// Create from raw CLI args
#[allow(clippy::fn_params_excessive_bools)]
pub fn from_args(
dev: bool,
no_dev: bool,
only_dev: bool,
group: Vec<GroupName>,
no_group: Vec<GroupName>,
no_default_groups: bool,
only_group: Vec<GroupName>,
all_groups: bool,
) -> Self {
// Lower the --dev flags into a single dev mode.
//
// In theory only one of these 3 flags should be set (enforced by CLI),
// but we explicitly allow `--dev` and `--only-dev` to both be set,
// and "saturate" that to `--only-dev`.
let dev_mode = if only_dev {
Some(DevMode::Only)
} else if no_dev {
Some(DevMode::Exclude)
} else if dev {
Some(DevMode::Include)
} else {
None
};
Self::from_history(DevGroupsSpecificationHistory {
dev_mode,
group,
only_group,
no_group,
all_groups,
no_default_groups,
// This is unknown at CLI-time, use `.with_defaults(...)` to apply this later!
defaults: Vec::new(),
})
}
/// Helper to make a spec from just a --dev flag
pub fn from_dev_mode(dev_mode: DevMode) -> Self {
Self::from_history(DevGroupsSpecificationHistory {
dev_mode: Some(dev_mode),
..Default::default()
})
}
/// Helper to make a spec from just a --group
pub fn from_group(group: GroupName) -> Self {
Self::from_history(DevGroupsSpecificationHistory {
group: vec![group],
..Default::default()
})
}
/// Apply defaults to a base [`DevGroupsSpecification`].
///
/// This is appropriate in projects, where the `dev` group is synced by default.
pub fn with_defaults(&self, defaults: Vec<GroupName>) -> DevGroupsManifest {
// Explicitly clone the inner history and set the defaults, then remake the result.
let mut history = self.0.history.clone();
history.defaults = defaults;
DevGroupsManifest {
cur: Self::from_history(history),
prev: self.clone(),
}
}
}
impl std::ops::Deref for DevGroupsSpecification {
type Target = DevGroupsSpecificationInner;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl DevGroupsSpecificationInner {
/// Returns `true` if packages other than the ones referenced by these
/// dependency-groups should be considered.
///
/// That is, if I tell you to install a project and this is false,
/// you should ignore the project itself and all its dependencies,
/// and instead just install the dependency-groups.
///
/// (This is really just asking if an --only flag was passed.)
pub fn prod(&self) -> bool {
!self.only_groups
}
/// Returns `true` if the specification includes the given group.
pub fn contains(&self, group: &GroupName) -> bool {
// exclude always trumps include
!self.exclude.contains(group) && self.include.contains(group)
}
/// Iterate over all groups that we think should exist.
pub fn desugarred_names(&self) -> impl Iterator<Item = &GroupName> {
self.include.names().chain(&self.exclude)
}
/// Iterate over all groups the user explicitly asked for on the CLI
pub fn explicit_names(&self) -> impl Iterator<Item = &GroupName> {
let DevGroupsSpecificationHistory {
// Strictly speaking this is an explicit reference to "dev"
// but we're currently tolerant of dev not existing when referenced with
// these flags, since it kinda implicitly always exists even if
// it's not properly defined in a config file.
dev_mode: _,
group,
only_group,
no_group,
// These reference no groups explicitly
all_groups: _,
no_default_groups: _,
// This doesn't include defaults because the `dev` group may not be defined
// but gets implicitly added as a default sometimes!
defaults: _,
} = self.history();
group.iter().chain(no_group).chain(only_group)
}
/// Returns `true` if the specification will have no effect.
pub fn is_empty(&self) -> bool {
self.prod() && self.exclude.is_empty() && self.include.is_empty()
}
/// Get the raw history for diagnostics
pub fn history(&self) -> &DevGroupsSpecificationHistory {
&self.history
}
}
/// Context about a [`DevGroupsSpecification`][] that we've preserved for diagnostics
#[derive(Debug, Default, Clone)]
pub struct DevGroupsSpecificationHistory {
pub dev_mode: Option<DevMode>,
pub group: Vec<GroupName>,
pub only_group: Vec<GroupName>,
pub no_group: Vec<GroupName>,
pub all_groups: bool,
pub no_default_groups: bool,
pub defaults: Vec<GroupName>,
}
impl DevGroupsSpecificationHistory {
/// Returns all the CLI flags that this represents.
///
/// If a flag was provided multiple times (e.g. `--group A --group B`) this will
/// elide the arguments and just show the flag once (e.g. just yield "--group").
///
/// Conceptually this being an empty list should be equivalent to
/// [`DevGroupsSpecification::is_empty`][] when there aren't any defaults set.
/// When there are defaults the two will disagree, and rightfully so!
pub fn as_flags_pretty(&self) -> Vec<Cow<str>> {
let DevGroupsSpecificationHistory {
dev_mode,
group,
only_group,
no_group,
all_groups,
no_default_groups,
// defaults aren't CLI flags!
defaults: _,
} = self;
let mut flags = vec![];
if *all_groups {
flags.push(Cow::Borrowed("--all-groups"));
}
if *no_default_groups {
flags.push(Cow::Borrowed("--no-default-groups"));
}
if let Some(dev_mode) = dev_mode {
flags.push(Cow::Borrowed(dev_mode.as_flag()));
}
match &**group {
[] => {}
[group] => flags.push(Cow::Owned(format!("--group {group}"))),
[..] => flags.push(Cow::Borrowed("--group")),
}
match &**only_group {
[] => {}
[group] => flags.push(Cow::Owned(format!("--only-group {group}"))),
[..] => flags.push(Cow::Borrowed("--only-group")),
}
match &**no_group {
[] => {}
[group] => flags.push(Cow::Owned(format!("--no-group {group}"))),
[..] => flags.push(Cow::Borrowed("--no-group")),
}
flags
}
}
/// A trivial newtype wrapped around [`DevGroupsSpecification`][] that signifies "defaults applied"
///
/// It includes a copy of the previous semantics to provide info on if
/// the group being a default actually affected it being enabled, because it's obviously "correct".
/// (These are Arcs so it's ~free to hold onto the previous semantics)
#[derive(Debug, Clone)]
pub struct DevGroupsManifest {
/// The active semantics
cur: DevGroupsSpecification,
/// The semantics before defaults were applied
prev: DevGroupsSpecification,
}
impl DevGroupsManifest {
/// Returns `true` if the specification was enabled, and *only* because it was a default
pub fn contains_because_default(&self, group: &GroupName) -> bool {
self.cur.contains(group) && !self.prev.contains(group)
}
}
impl std::ops::Deref for DevGroupsManifest {
type Target = DevGroupsSpecification;
fn deref(&self) -> &Self::Target {
&self.cur
}
}
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
pub enum DevMode {
/// Include development dependencies.
@ -16,16 +311,6 @@ pub enum DevMode {
}
impl DevMode {
/// Returns `true` if the specification allows for production dependencies.
pub fn prod(&self) -> bool {
matches!(self, Self::Exclude | Self::Include)
}
/// Returns `true` if the specification only includes development dependencies.
pub fn only(&self) -> bool {
matches!(self, Self::Only)
}
/// Returns the flag that was used to request development dependencies.
pub fn as_flag(&self) -> &'static str {
match self {
@ -34,141 +319,6 @@ impl DevMode {
Self::Only => "--only-dev",
}
}
/// Returns `true` if the group is `dev`, and development dependencies should be included.
pub fn contains(&self, group: &GroupName) -> bool {
match self {
DevMode::Exclude => false,
DevMode::Include | DevMode::Only => group == &*DEV_DEPENDENCIES,
}
}
}
#[derive(Default, Debug, Clone)]
pub struct DevGroupsSpecification {
/// Legacy option for `dependency-groups.dev` and `tool.uv.dev-dependencies`.
///
/// Requested via the `--dev`, `--no-dev`, and `--only-dev` flags.
dev: Option<DevMode>,
/// The groups to include.
///
/// Requested via the `--group` and `--only-group` options.
groups: Option<GroupsSpecification>,
}
#[derive(Debug, Clone)]
pub enum GroupsSpecification {
/// Include dependencies from the specified groups alongside the default groups (omitting
/// those default groups that are explicitly excluded).
///
/// If the `include` is `IncludeGroups::Some`, it is guaranteed to omit groups in the `exclude`
/// list (i.e., they have an empty intersection).
Include {
include: IncludeGroups,
exclude: Vec<GroupName>,
},
/// Include dependencies from the specified groups, omitting any default groups.
///
/// If the list is empty, no group will be included.
Explicit { include: Vec<GroupName> },
/// Only include dependencies from the specified groups, exclude all other dependencies.
///
/// The `include` list is guaranteed to omit groups in the `exclude` list (i.e., they have an
/// empty intersection).
Only {
include: Vec<GroupName>,
exclude: Vec<GroupName>,
},
}
impl GroupsSpecification {
/// Create a [`GroupsSpecification`] that includes the given group.
pub fn from_group(group: GroupName) -> Self {
Self::Include {
include: IncludeGroups::Some(vec![group]),
exclude: Vec::new(),
}
}
/// Returns `true` if the specification allows for production dependencies.
pub fn prod(&self) -> bool {
matches!(self, Self::Include { .. } | Self::Explicit { .. })
}
/// Returns `true` if the specification is limited to a select set of groups.
pub fn only(&self) -> bool {
matches!(self, Self::Only { .. })
}
/// Returns the option that was used to request the groups, if any.
pub fn as_flag(&self) -> Option<Cow<'_, str>> {
match self {
Self::Include { include, exclude } => match include {
IncludeGroups::All => Some(Cow::Borrowed("--all-groups")),
IncludeGroups::Some(groups) => match groups.as_slice() {
[] => match exclude.as_slice() {
[] => None,
[group] => Some(Cow::Owned(format!("--no-group {group}"))),
[..] => Some(Cow::Borrowed("--no-group")),
},
[group] => Some(Cow::Owned(format!("--group {group}"))),
[..] => Some(Cow::Borrowed("--group")),
},
},
Self::Only { include, exclude } => match include.as_slice() {
[] => match exclude.as_slice() {
[] => None,
[group] => Some(Cow::Owned(format!("--no-group {group}"))),
[..] => Some(Cow::Borrowed("--no-group")),
},
[group] => Some(Cow::Owned(format!("--only-group {group}"))),
[..] => Some(Cow::Borrowed("--only-group")),
},
Self::Explicit { include } => match include.as_slice() {
[] => Some(Cow::Borrowed("--no-default-groups")),
[group] => Some(Cow::Owned(format!("--group {group}"))),
[..] => Some(Cow::Borrowed("--group")),
},
}
}
/// Iterate over all groups referenced in the [`GroupsSpecification`].
pub fn names(&self) -> impl Iterator<Item = &GroupName> {
match self {
GroupsSpecification::Include { include, exclude } => {
Either::Left(include.names().chain(exclude.iter()))
}
GroupsSpecification::Only { include, exclude } => {
Either::Left(include.iter().chain(exclude.iter()))
}
GroupsSpecification::Explicit { include } => Either::Right(include.iter()),
}
}
/// Returns `true` if the specification includes the given group.
pub fn contains(&self, group: &GroupName) -> bool {
match self {
GroupsSpecification::Include { include, exclude } => {
// For `--all-groups`, the group is included unless it is explicitly excluded.
include.contains(group) && !exclude.contains(group)
}
GroupsSpecification::Only { include, .. } => include.contains(group),
GroupsSpecification::Explicit { include } => include.contains(group),
}
}
/// Returns `true` if the specification will have no effect.
pub fn is_empty(&self) -> bool {
let GroupsSpecification::Include {
include: IncludeGroups::Some(includes),
exclude,
} = self
else {
return false;
};
includes.is_empty() && exclude.is_empty()
}
}
#[derive(Debug, Clone)]
@ -188,6 +338,16 @@ impl IncludeGroups {
}
}
/// Returns `true` if the specification will have no effect.
pub fn is_empty(&self) -> bool {
match self {
IncludeGroups::Some(groups) => groups.is_empty(),
// Although technically this is a noop if they have no groups,
// conceptually they're *trying* to have an effect, so treat it as one.
IncludeGroups::All => false,
}
}
/// Iterate over all groups referenced in the [`IncludeGroups`].
pub fn names(&self) -> std::slice::Iter<GroupName> {
match self {
@ -197,249 +357,8 @@ impl IncludeGroups {
}
}
impl DevGroupsSpecification {
/// Determine the [`DevGroupsSpecification`] policy from the command-line arguments.
#[allow(clippy::fn_params_excessive_bools)]
pub fn from_args(
dev: bool,
no_dev: bool,
only_dev: bool,
mut group: Vec<GroupName>,
no_group: Vec<GroupName>,
no_default_groups: bool,
mut only_group: Vec<GroupName>,
all_groups: bool,
) -> Self {
let dev = if only_dev {
Some(DevMode::Only)
} else if no_dev {
Some(DevMode::Exclude)
} else if dev {
Some(DevMode::Include)
} else {
None
};
let groups = if no_default_groups {
// Remove groups specified with `--no-group`.
group.retain(|group| !no_group.contains(group));
Some(GroupsSpecification::Explicit { include: group })
} else if all_groups {
Some(GroupsSpecification::Include {
include: IncludeGroups::All,
exclude: no_group,
})
} else if !group.is_empty() {
if matches!(dev, Some(DevMode::Only)) {
unreachable!("cannot specify both `--only-dev` and `--group`")
};
// Ensure that `--no-group` and `--group` are mutually exclusive.
group.retain(|group| !no_group.contains(group));
Some(GroupsSpecification::Include {
include: IncludeGroups::Some(group),
exclude: no_group,
})
} else if !only_group.is_empty() {
if matches!(dev, Some(DevMode::Include)) {
unreachable!("cannot specify both `--dev` and `--only-group`")
};
// Ensure that `--no-group` and `--only-group` are mutually exclusive.
only_group.retain(|group| !no_group.contains(group));
Some(GroupsSpecification::Only {
include: only_group,
exclude: no_group,
})
} else if !no_group.is_empty() {
Some(GroupsSpecification::Include {
include: IncludeGroups::Some(Vec::new()),
exclude: no_group,
})
} else {
None
};
Self { dev, groups }
}
/// Return a new [`DevGroupsSpecification`] with development dependencies included by default.
///
/// This is appropriate in projects, where the `dev` group is synced by default.
#[must_use]
pub fn with_defaults(self, defaults: Vec<GroupName>) -> DevGroupsManifest {
DevGroupsManifest {
spec: self,
defaults,
}
}
/// Returns `true` if the specification allows for production dependencies.
pub fn prod(&self) -> bool {
self.dev.as_ref().map_or(true, DevMode::prod)
&& self.groups.as_ref().map_or(true, GroupsSpecification::prod)
}
/// Returns `true` if the specification is limited to a select set of groups.
pub fn only(&self) -> bool {
self.dev.as_ref().is_some_and(DevMode::only)
|| self.groups.as_ref().is_some_and(GroupsSpecification::only)
}
/// Returns the flag that was used to request development dependencies, if specified.
pub fn dev_mode(&self) -> Option<&DevMode> {
self.dev.as_ref()
}
/// Returns the list of groups to include, if specified.
pub fn groups(&self) -> Option<&GroupsSpecification> {
self.groups.as_ref()
}
/// Returns `true` if the group is included in the specification.
pub fn contains(&self, group: &GroupName) -> bool {
if group == &*DEV_DEPENDENCIES {
match self.dev.as_ref() {
None => {}
Some(DevMode::Exclude) => {
// If `--no-dev` was provided, always exclude dev.
return false;
}
Some(DevMode::Only) => {
// If `--only-dev` was provided, always include dev.
return true;
}
Some(DevMode::Include) => {
// If `--no-group dev` was provided, exclude dev.
return match self.groups.as_ref() {
Some(GroupsSpecification::Include { exclude, .. }) => {
!exclude.contains(group)
}
_ => true,
};
}
}
}
self.groups
.as_ref()
.is_some_and(|groups| groups.contains(group))
}
/// Returns `true` if the specification will have no effect.
pub fn is_empty(&self) -> bool {
let groups_empty = self
.groups
.as_ref()
.map(GroupsSpecification::is_empty)
.unwrap_or(true);
let dev_empty = self.dev_mode().is_none();
groups_empty && dev_empty
}
}
impl From<DevMode> for DevGroupsSpecification {
fn from(dev: DevMode) -> Self {
Self {
dev: Some(dev),
groups: None,
}
}
}
impl From<GroupsSpecification> for DevGroupsSpecification {
fn from(groups: GroupsSpecification) -> Self {
Self {
dev: None,
groups: Some(groups),
}
}
}
/// The manifest of `dependency-groups` to include, taking into account the user-provided
/// [`DevGroupsSpecification`] and the project-specific default groups.
#[derive(Debug, Default, Clone)]
pub struct DevGroupsManifest {
/// The specification for the development dependencies.
pub(crate) spec: DevGroupsSpecification,
/// The default groups to include.
pub(crate) defaults: Vec<GroupName>,
}
impl DevGroupsManifest {
/// Returns a new [`DevGroupsManifest`] with the given default groups.
pub fn from_defaults(defaults: Vec<GroupName>) -> Self {
Self {
spec: DevGroupsSpecification::default(),
defaults,
}
}
/// Returns a new [`DevGroupsManifest`] with the given specification.
pub fn from_spec(spec: DevGroupsSpecification) -> Self {
Self {
spec,
defaults: Vec::new(),
}
}
/// Returns `true` if the specification allows for production dependencies.
pub fn prod(&self) -> bool {
self.spec.prod()
}
/// Returns `true` if the group was enabled by default.
pub fn is_default(&self, group: &GroupName) -> bool {
if self.spec.contains(group) {
// If the group was explicitly requested, then it wasn't enabled by default.
false
} else {
// If the group was enabled, but wasn't explicitly requested, then it was enabled by
// default.
self.contains(group)
}
}
/// Returns `true` if the group is included in the manifest.
pub fn contains(&self, group: &GroupName) -> bool {
if self.spec.contains(group) {
return true;
}
if self.spec.only() {
return false;
}
self.defaults
.iter()
.filter(|default| {
// If `--no-dev` was provided, exclude the `dev` group from the list of defaults.
if matches!(self.spec.dev_mode(), Some(DevMode::Exclude)) {
if *default == &*DEV_DEPENDENCIES {
return false;
};
}
// If `--no-default-groups` was provided, only include group if it's explicitly
// included with `--group <group>`.
if let Some(GroupsSpecification::Explicit { include }) = self.spec.groups() {
return include.contains(group);
}
// If `--no-group` was provided, exclude the group from the list of defaults.
if let Some(GroupsSpecification::Include {
include: _,
exclude,
}) = self.spec.groups()
{
if exclude.contains(default) {
return false;
}
}
true
})
.any(|default| default == group)
impl Default for IncludeGroups {
fn default() -> Self {
Self::Some(Vec::new())
}
}