Skip to content

Commit

Permalink
Avoid overwriting different markers
Browse files Browse the repository at this point in the history
  • Loading branch information
blueraft committed Aug 11, 2024
1 parent 5c44937 commit 0fbde82
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 33 deletions.
44 changes: 25 additions & 19 deletions crates/uv-workspace/src/pyproject_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,10 @@ impl PyProjectTomlMut {
}

/// Removes all occurrences of dependencies with the given name.
pub fn remove_dependency(&mut self, req: &PackageName) -> Result<Vec<Requirement>, Error> {
pub fn remove_dependency(
&mut self,
requirement: &Requirement,
) -> Result<Vec<Requirement>, Error> {
// Try to get `project.dependencies`.
let Some(dependencies) = self
.doc_mut()?
Expand All @@ -354,14 +357,17 @@ impl PyProjectTomlMut {
return Ok(Vec::new());
};

let requirements = remove_dependency(req, dependencies);
self.remove_source(req)?;
let requirements = remove_dependency(requirement, dependencies);
self.remove_source(requirement)?;

Ok(requirements)
}

/// Removes all occurrences of development dependencies with the given name.
pub fn remove_dev_dependency(&mut self, req: &PackageName) -> Result<Vec<Requirement>, Error> {
pub fn remove_dev_dependency(
&mut self,
requirement: &Requirement,
) -> Result<Vec<Requirement>, Error> {
// Try to get `tool.uv.dev-dependencies`.
let Some(dev_dependencies) = self
.doc
Expand All @@ -378,16 +384,16 @@ impl PyProjectTomlMut {
return Ok(Vec::new());
};

let requirements = remove_dependency(req, dev_dependencies);
self.remove_source(req)?;
let requirements = remove_dependency(requirement, dev_dependencies);
self.remove_source(requirement)?;

Ok(requirements)
}

/// Removes all occurrences of optional dependencies in the group with the given name.
pub fn remove_optional_dependency(
&mut self,
req: &PackageName,
requirement: &Requirement,
group: &ExtraName,
) -> Result<Vec<Requirement>, Error> {
// Try to get `project.optional-dependencies.<group>`.
Expand All @@ -403,14 +409,14 @@ impl PyProjectTomlMut {
return Ok(Vec::new());
};

let requirements = remove_dependency(req, optional_dependencies);
self.remove_source(req)?;
let requirements = remove_dependency(requirement, optional_dependencies);
self.remove_source(requirement)?;

Ok(requirements)
}

/// Remove a matching source from `tool.uv.sources`, if it exists.
fn remove_source(&mut self, name: &PackageName) -> Result<(), Error> {
fn remove_source(&mut self, requirement: &Requirement) -> Result<(), Error> {
if let Some(sources) = self
.doc
.get_mut("tool")
Expand All @@ -423,7 +429,7 @@ impl PyProjectTomlMut {
.map(|sources| sources.as_table_mut().ok_or(Error::MalformedSources))
.transpose()?
{
sources.remove(name.as_ref());
sources.remove(requirement.name.as_ref());
}

Ok(())
Expand All @@ -434,13 +440,13 @@ impl PyProjectTomlMut {
///
/// This method searches `project.dependencies`, `tool.uv.dev-dependencies`, and
/// `tool.uv.optional-dependencies`.
pub fn find_dependency(&self, name: &PackageName) -> Vec<DependencyType> {
pub fn find_dependency(&self, requirement: &Requirement) -> Vec<DependencyType> {
let mut types = Vec::new();

if let Some(project) = self.doc.get("project").and_then(Item::as_table) {
// Check `project.dependencies`.
if let Some(dependencies) = project.get("dependencies").and_then(Item::as_array) {
if !find_dependencies(name, dependencies).is_empty() {
if !find_dependencies(requirement, dependencies).is_empty() {
types.push(DependencyType::Production);
}
}
Expand All @@ -458,7 +464,7 @@ impl PyProjectTomlMut {
continue;
};

if !find_dependencies(name, dependencies).is_empty() {
if !find_dependencies(requirement, dependencies).is_empty() {
types.push(DependencyType::Optional(extra));
}
}
Expand All @@ -475,7 +481,7 @@ impl PyProjectTomlMut {
.and_then(|tool| tool.get("dev-dependencies"))
.and_then(Item::as_array)
{
if !find_dependencies(name, dev_dependencies).is_empty() {
if !find_dependencies(requirement, dev_dependencies).is_empty() {
types.push(DependencyType::Dev);
}
}
Expand All @@ -500,7 +506,7 @@ pub fn add_dependency(
has_source: bool,
) -> Result<ArrayEdit, Error> {
// Find matching dependencies.
let mut to_replace = find_dependencies(&req.name, deps);
let mut to_replace = find_dependencies(req, deps);
match to_replace.as_slice() {
[] => {
deps.push(req.to_string());
Expand Down Expand Up @@ -545,7 +551,7 @@ fn update_requirement(old: &mut Requirement, new: &Requirement, has_source: bool
}

/// Removes all occurrences of dependencies with the given name from the given `deps` array.
fn remove_dependency(req: &PackageName, deps: &mut Array) -> Vec<Requirement> {
fn remove_dependency(req: &Requirement, deps: &mut Array) -> Vec<Requirement> {
// Remove matching dependencies.
let removed = find_dependencies(req, deps)
.into_iter()
Expand All @@ -566,11 +572,11 @@ fn remove_dependency(req: &PackageName, deps: &mut Array) -> Vec<Requirement> {

/// Returns a `Vec` containing the all dependencies with the given name, along with their positions
/// in the array.
fn find_dependencies(name: &PackageName, deps: &Array) -> Vec<(usize, Requirement)> {
fn find_dependencies(requirement: &Requirement, deps: &Array) -> Vec<(usize, Requirement)> {
let mut to_replace = Vec::new();
for (i, dep) in deps.iter().enumerate() {
if let Some(req) = dep.as_str().and_then(try_parse_requirement) {
if req.name == *name {
if req.name == requirement.name && req.marker == requirement.marker {
to_replace.push((i, req));
}
}
Expand Down
31 changes: 19 additions & 12 deletions crates/uv/src/commands/project/remove.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::str::FromStr;

use anyhow::{Context, Result};

use pep508_rs::PackageName;
use pep508_rs::{PackageName, Requirement};
use uv_cache::Cache;
use uv_client::Connectivity;
use uv_configuration::{Concurrency, ExtrasSpecification, PreviewMode};
Expand Down Expand Up @@ -93,29 +95,30 @@ pub(crate) async fn remove(
}?;

for package in packages {
let requirement = Requirement::from_str(package.as_str())?;
match dependency_type {
DependencyType::Production => {
let deps = toml.remove_dependency(&package)?;
let deps = toml.remove_dependency(&requirement)?;
if deps.is_empty() {
warn_if_present(&package, &toml);
warn_if_present(&requirement, &toml);
anyhow::bail!(
"The dependency `{package}` could not be found in `dependencies`"
);
}
}
DependencyType::Dev => {
let deps = toml.remove_dev_dependency(&package)?;
let deps = toml.remove_dev_dependency(&requirement)?;
if deps.is_empty() {
warn_if_present(&package, &toml);
warn_if_present(&requirement, &toml);
anyhow::bail!(
"The dependency `{package}` could not be found in `dev-dependencies`"
);
}
}
DependencyType::Optional(ref group) => {
let deps = toml.remove_optional_dependency(&package, group)?;
let deps = toml.remove_optional_dependency(&requirement, group)?;
if deps.is_empty() {
warn_if_present(&package, &toml);
warn_if_present(&requirement, &toml);
anyhow::bail!(
"The dependency `{package}` could not be found in `optional-dependencies`"
);
Expand Down Expand Up @@ -223,18 +226,22 @@ enum Target {
///
/// This is useful when a dependency of the user-specified type was not found, but it may be present
/// elsewhere.
fn warn_if_present(name: &PackageName, pyproject: &PyProjectTomlMut) {
for dep_ty in pyproject.find_dependency(name) {
fn warn_if_present(requirement: &Requirement, pyproject: &PyProjectTomlMut) {
for dep_ty in pyproject.find_dependency(requirement) {
match dep_ty {
DependencyType::Production => {
warn_user!("`{name}` is a production dependency");
warn_user!("`{}` is a production dependency", requirement.name);
}
DependencyType::Dev => {
warn_user!("`{name}` is a development dependency; try calling `uv remove --dev`");
warn_user!(
"`{}` is a development dependency; try calling `uv remove --dev`",
requirement.name
);
}
DependencyType::Optional(group) => {
warn_user!(
"`{name}` is an optional dependency; try calling `uv remove --optional {group}`"
"`{}` is an optional dependency; try calling `uv remove --optional {group}`",
requirement.name
);
}
}
Expand Down
122 changes: 120 additions & 2 deletions crates/uv/tests/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,8 @@ fn update() -> Result<()> {
version = "0.1.0"
requires-python = ">=3.12"
dependencies = [
"requests[security,socks,use-chardet-on-py3]==2.31.0 ; python_version > '3.7'",
"requests[security]==2.31.0",
"requests[socks,use-chardet-on-py3]>=2.31.0 ; python_version > '3.7'",
]
"###
);
Expand Down Expand Up @@ -1562,7 +1563,8 @@ fn update() -> Result<()> {
version = "0.1.0"
requires-python = ">=3.12"
dependencies = [
"requests[security,socks,use-chardet-on-py3]==2.31.0 ; python_version > '3.7'",
"requests[security]==2.31.0",
"requests[socks,use-chardet-on-py3]>=2.31.0 ; python_version > '3.7'",
]
[tool.uv.sources]
Expand Down Expand Up @@ -1697,6 +1699,122 @@ fn update() -> Result<()> {
Ok(())
}

/// Update a requirement, with different markers
#[test]
fn update_marker() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! {r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'"
]
"#})?;
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
Resolved 6 packages in [TIME]
"###);

uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv sync` is experimental and may change without warning
Prepared 6 packages in [TIME]
Installed 6 packages in [TIME]
+ certifi==2024.2.2
+ charset-normalizer==3.3.2
+ idna==3.6
+ project==0.1.0 (from file://[TEMP_DIR]/)
+ requests==2.31.0
+ urllib3==2.2.1
"###);

// Restrict requests version for py < 3.11
uv_snapshot!(context.filters(), context.add(&["requests>=2.0,< 2.29; python_version < '3.11'" ]), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv add` is experimental and may change without warning
Resolved 8 packages in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- project==0.1.0 (from file://[TEMP_DIR]/)
+ project==0.1.0 (from file://[TEMP_DIR]/)
"###);

let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
pyproject_toml, @r###"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'",
"requests>=2.0,<2.29 ; python_version < '3.11'",
]
"###
);
});

// Restrict requests version for windows
uv_snapshot!(context.filters(), context.add(&["requests>=2.31 ; sys_platform == 'win32' and python_version > '3.11'" ]), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv add` is experimental and may change without warning
Resolved 8 packages in [TIME]
Prepared 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- project==0.1.0 (from file://[TEMP_DIR]/)
+ project==0.1.0 (from file://[TEMP_DIR]/)
"###);

let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
pyproject_toml, @r###"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'",
"requests>=2.0,<2.29 ; python_version < '3.11'",
"requests>=2.31 ; python_version > '3.11' and sys_platform == 'win32'",
]
"###
);
});

Ok(())
}

#[test]
#[cfg(feature = "git")]
fn update_source_replace_url() -> Result<()> {
Expand Down

0 comments on commit 0fbde82

Please sign in to comment.