Skip to content

Commit

Permalink
Warn when 'requires-python' does not include a lower bound
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jun 11, 2024
1 parent 3ea5a2a commit 0c562b8
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 36 deletions.
2 changes: 1 addition & 1 deletion crates/distribution-types/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use pypi_types::{CoreMetadata, HashDigest, Yanked};
/// Error converting [`pypi_types::File`] to [`distribution_type::File`].
#[derive(Debug, thiserror::Error)]
pub enum FileConversionError {
#[error("Failed to parse 'requires-python': `{0}`")]
#[error("Failed to parse `requires-python`: `{0}`")]
RequiresPython(String, #[source] VersionSpecifiersParseError),
#[error("Failed to parse URL: {0}")]
Url(String, #[source] url::ParseError),
Expand Down
78 changes: 45 additions & 33 deletions crates/uv-resolver/src/requires_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,20 @@ pub enum RequiresPythonError {
///
/// See: <https://packaging.python.org/en/latest/guides/dropping-older-python-versions/>
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct RequiresPython(VersionSpecifiers);
pub struct RequiresPython {
specifiers: VersionSpecifiers,
bound: Bound<Version>,
}

impl RequiresPython {
/// Returns a [`RequiresPython`] to express `>=` equality with the given version.
pub fn greater_than_equal_version(version: Version) -> Self {
Self(VersionSpecifiers::from(
VersionSpecifier::greater_than_equal_version(version),
))
Self {
specifiers: VersionSpecifiers::from(VersionSpecifier::greater_than_equal_version(
version.clone(),
)),
bound: Bound::Included(version),
}
}

/// Returns a [`RequiresPython`] to express the union of the given version specifiers.
Expand All @@ -53,20 +59,25 @@ impl RequiresPython {
return Ok(None);
};

// Extract the lower bound.
let bound = range
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded);

// Convert back to PEP 440 specifiers.
let requires_python = Self(
range
.iter()
.flat_map(VersionSpecifier::from_bounds)
.collect(),
);

Ok(Some(requires_python))
let specifiers = range
.iter()
.flat_map(VersionSpecifier::from_bounds)
.collect();

Ok(Some(Self { specifiers, bound }))
}

/// Returns `true` if the `Requires-Python` is compatible with the given version.
pub fn contains(&self, version: &Version) -> bool {
self.0.contains(version)
self.specifiers.contains(version)
}

/// Returns `true` if the `Requires-Python` is compatible with the given version specifiers.
Expand All @@ -76,24 +87,14 @@ impl RequiresPython {
/// provided range. However, `>=3.9` would not be considered compatible, as the
/// `Requires-Python` includes Python 3.8, but `>=3.9` does not.
pub fn is_contained_by(&self, target: &VersionSpecifiers) -> bool {
let Ok(requires_python) = crate::pubgrub::PubGrubSpecifier::try_from(&self.0) else {
return false;
};

let Ok(target) = crate::pubgrub::PubGrubSpecifier::try_from(target) else {
return false;
};

// If the dependency has no lower bound, then it supports all versions.
let Some((target_lower, _)) = target.iter().next() else {
return true;
};

// If we have no lower bound, then there must be versions we support that the
// dependency does not.
let Some((requires_python_lower, _)) = requires_python.iter().next() else {
return false;
};
let target = target
.iter()
.next()
.map(|(lower, _)| lower)
.unwrap_or(&Bound::Unbounded);

// We want, e.g., `requires_python_lower` to be `>=3.8` and `version_lower` to be
// `>=3.7`.
Expand Down Expand Up @@ -138,7 +139,7 @@ impl RequiresPython {
// Alternatively, we could vary the semantics depending on whether or not the user included
// a pre-release in their specifier, enforcing pre-release compatibility only if the user
// explicitly requested it.
match (target_lower, requires_python_lower) {
match (target, &self.bound) {
(Bound::Included(target_lower), Bound::Included(requires_python_lower)) => {
target_lower.release() <= requires_python_lower.release()
}
Expand All @@ -161,25 +162,36 @@ impl RequiresPython {

/// Returns the [`VersionSpecifiers`] for the `Requires-Python` specifier.
pub fn specifiers(&self) -> &VersionSpecifiers {
&self.0
&self.specifiers
}

/// Returns the lower [`Bound`] for the `Requires-Python` specifier.
pub fn bound(&self) -> &Bound<Version> {
&self.bound
}
}

impl std::fmt::Display for RequiresPython {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(&self.0, f)
std::fmt::Display::fmt(&self.specifiers, f)
}
}

impl serde::Serialize for RequiresPython {
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
self.0.serialize(serializer)
self.specifiers.serialize(serializer)
}
}

impl<'de> serde::Deserialize<'de> for RequiresPython {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let specifiers = VersionSpecifiers::deserialize(deserializer)?;
Ok(Self(specifiers))
let bound = crate::pubgrub::PubGrubSpecifier::try_from(&specifiers)
.map_err(serde::de::Error::custom)?
.iter()
.next()
.map(|(lower, _)| lower.clone())
.unwrap_or(Bound::Unbounded);
Ok(Self { specifiers, bound })
}
}
17 changes: 15 additions & 2 deletions crates/uv/src/commands/project/lock.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::Bound;

use anstream::eprint;

use distribution_types::{IndexLocations, UnresolvedRequirementSpecification};
Expand Down Expand Up @@ -128,7 +130,7 @@ pub(super) async fn do_lock(
//
// For a workspace, we compute the union of all workspace requires-python values, ensuring we
// keep track of `None` vs. a full range.
let requires_python_workspace =
let requires_python =
RequiresPython::union(workspace.packages().values().filter_map(|member| {
member
.pyproject_toml()
Expand All @@ -137,7 +139,18 @@ pub(super) async fn do_lock(
.and_then(|project| project.requires_python.as_ref())
}))?;

let requires_python = if let Some(requires_python) = requires_python_workspace {
let requires_python = if let Some(requires_python) = requires_python {
if matches!(requires_python.bound(), Bound::Unbounded) {
if let Some(root_project_name) = root_project_name.as_ref() {
warn_user!(
"The `requires-python` field found in `{root_project_name}` does not contain a lower bound: `{requires_python}`. Set a lower bound to indicate the minimum compatible Python version (e.g., '>=3.8').",
);
} else {
warn_user!(
"The `requires-python` field found in workspace does not contain a lower bound: `{requires_python}`. Set a lower bound to indicate the minimum compatible Python version (e.g., '>=3.8').",
);
}
}
requires_python
} else {
let requires_python =
Expand Down
63 changes: 63 additions & 0 deletions crates/uv/tests/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2428,6 +2428,69 @@ fn lock_requires_python_pre() -> Result<()> {
Ok(())
}

/// Warn if `Requires-Python` does not include a lower bound.
#[test]
fn lock_requires_python_unbounded() -> Result<()> {
let context = TestContext::new("3.11");

let lockfile = context.temp_dir.join("uv.lock");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = "<=3.12"
dependencies = ["iniconfig"]
"#,
)?;

uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning.
warning: The `requires-python` field found in `project` does not contain a lower bound: `<=3.12`. Set a lower bound to indicate the minimum compatible Python version (e.g., '>=3.8').
Resolved 2 packages in [TIME]
"###);

let lock = fs_err::read_to_string(lockfile)?;

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = "<=3.12"
[[distribution]]
name = "iniconfig"
version = "1.1.1"
source = "registry+https://pypi.org/simple"
sdist = { url = "https://files.pythonhosted.org/packages/23/a2/97899f6bd0e873fed3a7e67ae8d3a08b21799430fb4da15cfedf10d6e2c2/iniconfig-1.1.1.tar.gz", hash = "sha256:bc3af051d7d14b2ee5ef9969666def0cd1a000e121eaea580d4a313df4b37f32", size = 8104 }
wheels = [{ url = "https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl", hash = "sha256:011e24c64b7f47f6ebd835bb12a743f2fbe9a26d4cecaa7f53bc4f35ee9da8b3", size = 4990 }]
[[distribution]]
name = "project"
version = "0.1.0"
source = "editable+."
sdist = { path = "." }
[[distribution.dependencies]]
name = "iniconfig"
version = "1.1.1"
source = "registry+https://pypi.org/simple"
"###
);
});

Ok(())
}

/// Lock the development dependencies for a project.
#[test]
fn lock_dev() -> Result<()> {
Expand Down

0 comments on commit 0c562b8

Please sign in to comment.