Skip to content

Commit

Permalink
Refactor development dependency configuration (#8309)
Browse files Browse the repository at this point in the history
Part of #8090
Unblocks #8274

Refactors `DevMode` and `DevSpecification` into a shared type
`DevGroupsSpecification` that allows us to track if `--dev` was
implicitly or explicitly provided.
  • Loading branch information
zanieb authored and charliermarsh committed Oct 25, 2024
1 parent afffab6 commit 7aa0201
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 109 deletions.
147 changes: 102 additions & 45 deletions crates/uv-configuration/src/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,44 @@ pub enum DevMode {
}

impl DevMode {
/// Determine the [`DevMode`] policy from the command-line arguments.
pub fn from_args(dev: bool, no_dev: bool, only_dev: bool) -> Self {
if only_dev {
Self::Only
} else if no_dev {
Self::Exclude
} else if dev {
Self::Include
} else {
Self::default()
/// Iterate over the group names to include.
pub fn iter(&self) -> impl Iterator<Item = &GroupName> {
match self {
Self::Exclude => Either::Left(std::iter::empty()),
Self::Include | Self::Only => Either::Right(std::iter::once(&*DEV_DEPENDENCIES)),
}
}

/// Returns `true` if the specification allows for production dependencies.
pub fn prod(&self) -> bool {
matches!(self, Self::Exclude | Self::Include)
}
}

#[derive(Debug, Clone)]
pub enum DevSpecification {
/// Include dev dependencies from the specified group.
pub struct DevGroupsSpecification {
/// Legacy option for `dependency-group.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: GroupsSpecification,
}

#[derive(Debug, Clone)]
pub enum GroupsSpecification {
/// Include dependencies from the specified groups.
Include(Vec<GroupName>),
/// Do not include dev dependencies.
/// Do not include dependencies from groups.
Exclude,
/// Include dev dependencies from the specified groups, and exclude all non-dev dependencies.
/// Only include dependencies from the specified groups, exclude all other dependencies.
Only(Vec<GroupName>),
}

impl DevSpecification {
impl GroupsSpecification {
/// Returns an [`Iterator`] over the group names to include.
pub fn iter(&self) -> impl Iterator<Item = &GroupName> {
match self {
Expand All @@ -52,48 +65,92 @@ impl DevSpecification {
}
}

impl From<DevMode> for DevSpecification {
fn from(mode: DevMode) -> Self {
match mode {
DevMode::Include => Self::Include(vec![DEV_DEPENDENCIES.clone()]),
DevMode::Exclude => Self::Exclude,
DevMode::Only => Self::Only(vec![DEV_DEPENDENCIES.clone()]),
impl DevGroupsSpecification {
/// Returns an [`Iterator`] over the group names to include.
pub fn iter(&self) -> impl Iterator<Item = &GroupName> {
match self.dev {
None => Either::Left(self.groups.iter()),
Some(ref dev_mode) => Either::Right(self.groups.iter().chain(dev_mode.iter())),
}
}
}

impl DevSpecification {
/// Determine the [`DevSpecification`] policy from the command-line arguments.
/// Determine the [`DevGroupsSpecification`] policy from the command-line arguments.
pub fn from_args(
dev: bool,
no_dev: bool,
only_dev: bool,
group: Vec<GroupName>,
only_group: Vec<GroupName>,
) -> Self {
let from_mode = DevSpecification::from(DevMode::from_args(dev, no_dev, only_dev));
if !group.is_empty() {
match from_mode {
DevSpecification::Exclude => Self::Include(group),
DevSpecification::Include(dev) => {
Self::Include(group.into_iter().chain(dev).collect())
}
DevSpecification::Only(_) => {
unreachable!("cannot specify both `--only-dev` and `--group`")
}
}
let dev_mode = if only_dev {
Some(DevMode::Only)
} else if no_dev {
Some(DevMode::Exclude)
} else if dev {
Some(DevMode::Include)
} else {
None
};

let groups = if !group.is_empty() {
if matches!(dev_mode, Some(DevMode::Only)) {
unreachable!("cannot specify both `--only-dev` and `--group`")
};
GroupsSpecification::Include(group)
} else if !only_group.is_empty() {
match from_mode {
DevSpecification::Exclude => Self::Only(only_group),
DevSpecification::Only(dev) => {
Self::Only(only_group.into_iter().chain(dev).collect())
}
// TODO(zanieb): `dev` defaults to true we can't tell if `--dev` was provided in
// conflict with `--only-group` here
DevSpecification::Include(_) => Self::Only(only_group),
}
if matches!(dev_mode, Some(DevMode::Include)) {
unreachable!("cannot specify both `--dev` and `--only-group`")
};
GroupsSpecification::Only(only_group)
} else {
from_mode
GroupsSpecification::Exclude
};

Self {
dev: dev_mode,
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_default_dev(self) -> Self {
match self.dev {
Some(_) => self,
None => match self.groups {
// Only include the default `dev` group if `--only-group` wasn't used
GroupsSpecification::Only(_) => self,
GroupsSpecification::Exclude | GroupsSpecification::Include(_) => Self {
dev: Some(DevMode::Include),
..self
},
},
}
}

/// Returns `true` if the specification allows for production dependencies.
pub fn prod(&self) -> bool {
(self.dev.is_none() || self.dev.as_ref().is_some_and(DevMode::prod)) && self.groups.prod()
}

pub fn dev_mode(&self) -> Option<&DevMode> {
self.dev.as_ref()
}
}

impl From<DevMode> for DevGroupsSpecification {
fn from(dev: DevMode) -> Self {
Self {
dev: Some(dev),
groups: GroupsSpecification::Exclude,
}
}
}

impl From<GroupsSpecification> for DevGroupsSpecification {
fn from(groups: GroupsSpecification) -> Self {
Self { dev: None, groups }
}
}
4 changes: 2 additions & 2 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use toml_edit::{value, Array, ArrayOfTables, InlineTable, Item, Table, Value};
use url::Url;

use uv_cache_key::RepositoryUrl;
use uv_configuration::{BuildOptions, DevSpecification, ExtrasSpecification, InstallOptions};
use uv_configuration::{BuildOptions, DevGroupsSpecification, ExtrasSpecification, InstallOptions};
use uv_distribution::DistributionDatabase;
use uv_distribution_filename::{DistExtension, ExtensionError, SourceDistExtension, WheelFilename};
use uv_distribution_types::{
Expand Down Expand Up @@ -580,7 +580,7 @@ impl Lock {
marker_env: &ResolverMarkerEnvironment,
tags: &Tags,
extras: &ExtrasSpecification,
dev: &DevSpecification,
dev: &DevGroupsSpecification,
build_options: &BuildOptions,
install_options: &InstallOptions,
) -> Result<Resolution, LockError> {
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-resolver/src/lock/requirements_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use petgraph::{Directed, Graph};
use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
use url::Url;

use uv_configuration::{DevSpecification, EditableMode, ExtrasSpecification, InstallOptions};
use uv_configuration::{DevGroupsSpecification, EditableMode, ExtrasSpecification, InstallOptions};
use uv_distribution_filename::{DistExtension, SourceDistExtension};
use uv_fs::Simplified;
use uv_git::GitReference;
Expand Down Expand Up @@ -43,7 +43,7 @@ impl<'lock> RequirementsTxtExport<'lock> {
lock: &'lock Lock,
root_name: &PackageName,
extras: &ExtrasSpecification,
dev: &DevSpecification,
dev: &DevGroupsSpecification,
editable: EditableMode,
hashes: bool,
install_options: &'lock InstallOptions,
Expand Down
31 changes: 12 additions & 19 deletions crates/uv-resolver/src/lock/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use petgraph::visit::Dfs;
use petgraph::Direction;
use rustc_hash::{FxHashMap, FxHashSet};

use uv_configuration::DevMode;
use uv_configuration::DevGroupsSpecification;
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pypi_types::ResolverMarkerEnvironment;

Expand All @@ -34,7 +34,7 @@ impl<'env> TreeDisplay<'env> {
depth: usize,
prune: &[PackageName],
packages: &[PackageName],
dev: DevMode,
dev: &DevGroupsSpecification,
no_dedupe: bool,
invert: bool,
) -> Self {
Expand Down Expand Up @@ -134,8 +134,6 @@ impl<'env> TreeDisplay<'env> {
}
}

let mut modified = false;

// Step 1: Filter out packages that aren't reachable on this platform.
if let Some(environment_markers) = markers {
let mut reachable = FxHashSet::default();
Expand Down Expand Up @@ -167,12 +165,11 @@ impl<'env> TreeDisplay<'env> {

// Remove the unreachable nodes from the graph.
graph.retain_nodes(|_, index| reachable.contains(&index));
modified = true;
}

// Step 2: Filter the graph to those that are reachable in production or development, if
// `--no-dev` or `--only-dev` were specified, respectively.
if dev != DevMode::Include {
{
let mut reachable = FxHashSet::default();

// Perform a DFS from the root nodes to find the reachable nodes, following only the
Expand All @@ -189,27 +186,24 @@ impl<'env> TreeDisplay<'env> {
while let Some(node) = stack.pop_front() {
reachable.insert(node);
for edge in graph.edges_directed(node, Direction::Outgoing) {
if matches!(edge.weight(), Edge::Prod(_) | Edge::Optional(_, _)) {
let include = match edge.weight() {
Edge::Prod(_) => dev.prod(),
Edge::Optional(_, _) => dev.prod(),
Edge::Dev(group, _) => dev.iter().contains(*group),
};
if include {
stack.push_back(edge.target());
}
}
}

// Remove the unreachable nodes from the graph.
graph.retain_nodes(|_, index| {
if reachable.contains(&index) {
dev != DevMode::Only
} else {
dev != DevMode::Exclude
}
});
modified = true;
graph.retain_nodes(|_, index| reachable.contains(&index));
}

// Step 3: Reverse the graph.
if invert {
graph.reverse();
modified = true;
}

// Step 4: Filter the graph to those nodes reachable from the target packages.
Expand All @@ -230,11 +224,10 @@ impl<'env> TreeDisplay<'env> {

// Remove the unreachable nodes from the graph.
graph.retain_nodes(|_, index| reachable.contains(&index));
modified = true;
}

// If the graph was modified, re-create the inverse map.
if modified {
// Re-create the inverse map.
{
inverse.clear();
for node in graph.node_indices() {
inverse.insert(graph[node], node);
Expand Down
15 changes: 9 additions & 6 deletions crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use uv_cache::Cache;
use uv_cache_key::RepositoryUrl;
use uv_client::{BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClientBuilder};
use uv_configuration::{
Concurrency, Constraints, DevMode, DevSpecification, EditableMode, ExtrasSpecification,
InstallOptions, LowerBound, SourceStrategy,
Concurrency, Constraints, DevGroupsSpecification, DevMode, EditableMode, ExtrasSpecification,
GroupsSpecification, InstallOptions, LowerBound, SourceStrategy,
};
use uv_dispatch::BuildDispatch;
use uv_distribution::DistributionDatabase;
Expand Down Expand Up @@ -811,22 +811,25 @@ async fn lock_and_sync(
let (extras, dev) = match dependency_type {
DependencyType::Production => {
let extras = ExtrasSpecification::None;
let dev = DevSpecification::from(DevMode::Exclude);
let dev = DevGroupsSpecification::from(DevMode::Exclude);
(extras, dev)
}
DependencyType::Dev => {
let extras = ExtrasSpecification::None;
let dev = DevSpecification::from(DevMode::Include);
let dev = DevGroupsSpecification::from(DevMode::Include);
(extras, dev)
}
DependencyType::Optional(ref extra_name) => {
let extras = ExtrasSpecification::Some(vec![extra_name.clone()]);
let dev = DevSpecification::from(DevMode::Exclude);
let dev = DevGroupsSpecification::from(DevMode::Exclude);
(extras, dev)
}
DependencyType::Group(ref group_name) => {
let extras = ExtrasSpecification::None;
let dev = DevSpecification::Include(vec![group_name.clone()]);
let dev =
DevGroupsSpecification::from(GroupsSpecification::Include(
vec![group_name.clone()],
));
(extras, dev)
}
};
Expand Down
6 changes: 3 additions & 3 deletions crates/uv/src/commands/project/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::path::{Path, PathBuf};
use uv_cache::Cache;
use uv_client::Connectivity;
use uv_configuration::{
Concurrency, DevMode, DevSpecification, EditableMode, ExportFormat, ExtrasSpecification,
Concurrency, DevGroupsSpecification, EditableMode, ExportFormat, ExtrasSpecification,
InstallOptions, LowerBound,
};
use uv_normalize::PackageName;
Expand All @@ -33,7 +33,7 @@ pub(crate) async fn export(
install_options: InstallOptions,
output_file: Option<PathBuf>,
extras: ExtrasSpecification,
dev: DevMode,
dev: DevGroupsSpecification,
editable: EditableMode,
locked: bool,
frozen: bool,
Expand Down Expand Up @@ -142,7 +142,7 @@ pub(crate) async fn export(
&lock,
project.project_name(),
&extras,
&DevSpecification::from(dev),
&dev.with_default_dev(),
editable,
hashes,
&install_options,
Expand Down
6 changes: 3 additions & 3 deletions crates/uv/src/commands/project/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use owo_colors::OwoColorize;
use uv_cache::Cache;
use uv_client::Connectivity;
use uv_configuration::{
Concurrency, DevMode, DevSpecification, EditableMode, ExtrasSpecification, InstallOptions,
LowerBound,
Concurrency, DevGroupsSpecification, DevMode, EditableMode, ExtrasSpecification,
InstallOptions, LowerBound,
};
use uv_fs::Simplified;
use uv_pep508::PackageName;
Expand Down Expand Up @@ -216,7 +216,7 @@ pub(crate) async fn remove(
&venv,
&lock,
&extras,
&DevSpecification::from(dev),
&DevGroupsSpecification::from(dev),
EditableMode::Editable,
install_options,
Modifications::Exact,
Expand Down
Loading

0 comments on commit 7aa0201

Please sign in to comment.