Skip to content

Commit

Permalink
Omit transitive development dependencies from workspace lockfile (#5646)
Browse files Browse the repository at this point in the history
## Summary

Omit development dependencies from (e.g.) path dependencies.

Closes #5593.
  • Loading branch information
charliermarsh authored Jul 31, 2024
1 parent cbb13e6 commit 2574f5b
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 16 deletions.
28 changes: 17 additions & 11 deletions crates/uv-resolver/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Manifest {
&'a self,
markers: Option<&'a MarkerEnvironment>,
mode: DependencyMode,
) -> impl Iterator<Item = Cow<'a, PackageName>> + 'a {
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
match mode {
// Include direct requirements, dependencies of editables, and transitive dependencies
// of local packages.
Expand All @@ -200,26 +200,32 @@ impl Manifest {
self.overrides
.apply(&self.requirements)
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
)
.map(|requirement| match requirement {
Cow::Borrowed(requirement) => Cow::Borrowed(&requirement.name),
Cow::Owned(requirement) => Cow::Owned(requirement.name),
}),
),
),

// Restrict to the direct requirements.
DependencyMode::Direct => Either::Right(
self.overrides
.apply(self.requirements.iter())
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))
.map(|requirement| match requirement {
Cow::Borrowed(requirement) => Cow::Borrowed(&requirement.name),
Cow::Owned(requirement) => Cow::Owned(requirement.name),
}),
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
),
}
}

/// Returns an iterator over the direct requirements, with overrides applied.
///
/// At time of writing, this is used for:
/// - Determining which packages should have development dependencies included in the
/// resolution (assuming the user enabled development dependencies).
pub fn direct_requirements<'a>(
&'a self,
markers: Option<&'a MarkerEnvironment>,
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
self.overrides
.apply(self.requirements.iter())
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))
}

/// Apply the overrides and constraints to a set of requirements.
///
/// Constraints are always applied _on top_ of overrides, such that constraints are applied
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/resolution_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl ResolutionStrategy {
ResolutionMode::LowestDirect => Self::LowestDirect(
manifest
.user_requirements(markers, dependencies)
.map(|requirement| (*requirement).clone())
.map(|requirement| requirement.name.clone())
.collect(),
),
}
Expand Down
35 changes: 35 additions & 0 deletions crates/uv-resolver/src/resolver/groups.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use rustc_hash::FxHashMap;

use pep508_rs::MarkerEnvironment;
use uv_normalize::{GroupName, PackageName};

use crate::Manifest;

/// A map of package names to their activated dependency groups.
#[derive(Debug, Default, Clone)]
pub(crate) struct Groups(FxHashMap<PackageName, Vec<GroupName>>);

impl Groups {
/// Determine the set of enabled dependency groups in the [`Manifest`].
pub(crate) fn from_manifest(manifest: &Manifest, markers: Option<&MarkerEnvironment>) -> Self {
let mut groups = FxHashMap::default();

// Enable the groups for all direct dependencies. In practice, this tends to mean: when
// development dependencies are enabled, enable them for all direct dependencies.
for group in &manifest.dev {
for requirement in manifest.direct_requirements(markers) {
groups
.entry(requirement.name.clone())
.or_insert_with(Vec::new)
.push(group.clone());
}
}

Self(groups)
}

/// Retrieve the enabled dependency groups for a given package.
pub(crate) fn get(&self, package: &PackageName) -> Option<&[GroupName]> {
self.0.get(package).map(Vec::as_slice)
}
}
8 changes: 5 additions & 3 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ pub(crate) use crate::resolver::availability::{
IncompletePackage, ResolverVersion, UnavailablePackage, UnavailableReason, UnavailableVersion,
};
use crate::resolver::batch_prefetch::BatchPrefetcher;
use crate::resolver::groups::Groups;
pub(crate) use crate::resolver::index::FxOnceMap;
pub use crate::resolver::index::InMemoryIndex;
pub use crate::resolver::provider::{
Expand All @@ -74,6 +75,7 @@ use crate::{DependencyMode, Exclusions, FlatIndex, Options};
mod availability;
mod batch_prefetch;
mod fork_map;
mod groups;
mod index;
mod locals;
mod provider;
Expand All @@ -93,7 +95,7 @@ struct ResolverState<InstalledPackages: InstalledPackagesProvider> {
requirements: Vec<Requirement>,
constraints: Constraints,
overrides: Overrides,
dev: Vec<GroupName>,
groups: Groups,
preferences: Preferences,
git: GitResolver,
exclusions: Exclusions,
Expand Down Expand Up @@ -224,11 +226,11 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
markers.marker_environment(),
options.dependency_mode,
),
groups: Groups::from_manifest(&manifest, markers.marker_environment()),
project: manifest.project,
requirements: manifest.requirements,
constraints: manifest.constraints,
overrides: manifest.overrides,
dev: manifest.dev,
preferences: manifest.preferences,
exclusions: manifest.exclusions,
hasher: hasher.clone(),
Expand Down Expand Up @@ -1308,7 +1310,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// add a dependency from it to the same package with the group
// enabled.
if extra.is_none() && dev.is_none() {
for group in &self.dev {
for group in self.groups.get(name).into_iter().flatten() {
if !metadata.dev_dependencies.contains_key(group) {
continue;
}
Expand Down
135 changes: 134 additions & 1 deletion crates/uv/tests/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use common::{uv_snapshot, TestContext};

mod common;

// Wraps a group of snapshots and runs them multiple times in sequence.
/// Wraps a group of snapshots and runs them multiple times in sequence.
///
/// This is useful to ensure that resolution runs independent of an existing lockfile
/// and does not change across repeated calls to `uv lock`.
Expand Down Expand Up @@ -4254,3 +4254,136 @@ fn lock_exclusion() -> Result<()> {

Ok(())
}

/// Ensure that development dependencies are omitted for non-workspace members. Below, `bar` depends
/// on `foo`, but `bar/uv.lock` should omit `anyio`, but should include `typing-extensions`.
#[test]
fn lock_dev_transitive() -> Result<()> {
let context = TestContext::new("3.12");

let foo = context.temp_dir.child("foo");
fs_err::create_dir_all(&foo)?;

let pyproject_toml = foo.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "foo"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = []
[tool.uv]
dev-dependencies = ["anyio"]
"#,
)?;

let bar = context.temp_dir.child("bar");
fs_err::create_dir_all(&bar)?;

let pyproject_toml = bar.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "bar"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["foo", "baz", "iniconfig>1"]
[tool.uv.sources]
foo = { path = "../foo" }
baz = { workspace = true }
[tool.uv.workspace]
members = ["baz"]
"#,
)?;

let baz = bar.child("baz");
fs_err::create_dir_all(&baz)?;

let pyproject_toml = baz.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "baz"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = []
[tool.uv]
dev-dependencies = ["typing-extensions>4"]
"#,
)?;

uv_snapshot!(context.filters(), context.lock().current_dir(&bar), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
warning: `uv.sources` is experimental and may change without warning
Resolved 5 packages in [TIME]
"###);

let lock = fs_err::read_to_string(bar.join("uv.lock")).unwrap();

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "bar"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "baz" },
{ name = "foo" },
{ name = "iniconfig" },
]
[[distribution]]
name = "baz"
version = "0.1.0"
source = { editable = "baz" }
[distribution.dev-dependencies]
dev = [
{ name = "typing-extensions" },
]
[[distribution]]
name = "foo"
version = "0.1.0"
source = { directory = "../foo" }
[[distribution]]
name = "iniconfig"
version = "2.0.0"
source = { registry = "https://pypi.org/simple" }
sdist = { url = "https://files.pythonhosted.org/packages/d7/4b/cbd8e699e64a6f16ca3a8220661b5f83792b3017d0f79807cb8708d33913/iniconfig-2.0.0.tar.gz", hash = "sha256:2d91e135bf72d31a410b17c16da610a82cb55f6b0477d1a902134b24a455b8b3", size = 4646 }
wheels = [
{ url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", hash = "sha256:b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374", size = 5892 },
]
[[distribution]]
name = "typing-extensions"
version = "4.10.0"
source = { registry = "https://pypi.org/simple" }
sdist = { url = "https://files.pythonhosted.org/packages/16/3a/0d26ce356c7465a19c9ea8814b960f8a36c3b0d07c323176620b7b483e44/typing_extensions-4.10.0.tar.gz", hash = "sha256:b0abd7c89e8fb96f98db18d86106ff1d90ab692004eb746cf6eda2682f91b3cb", size = 77558 }
wheels = [
{ url = "https://files.pythonhosted.org/packages/f9/de/dc04a3ea60b22624b51c703a84bbe0184abcd1d0b9bc8074b5d6b7ab90bb/typing_extensions-4.10.0-py3-none-any.whl", hash = "sha256:69b1a937c3a517342112fb4c6df7e72fc39a38e7891a5730ed4985b5214b5475", size = 33926 },
]
"###
);
});

Ok(())
}

0 comments on commit 2574f5b

Please sign in to comment.