Warn when 'requires-python' does not include a lower bound (#4234)

## Summary

Closes https://github.com/astral-sh/uv/issues/4089.
This commit is contained in:
Charlie Marsh 2024-06-11 11:50:05 -07:00 committed by GitHub
parent b3a99d9ff9
commit dce913c542
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 130 additions and 42 deletions

View file

@ -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),

View file

@ -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.
@ -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
let specifiers = range
.iter()
.flat_map(VersionSpecifier::from_bounds)
.collect(),
);
.collect();
Ok(Some(requires_python))
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.
@ -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`.
@ -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()
}
@ -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 })
}
}

View file

@ -1,3 +1,5 @@
use std::collections::Bound;
use anstream::eprint;
use distribution_types::{IndexLocations, UnresolvedRequirementSpecification};
@ -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()
@ -137,21 +139,32 @@ 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 {
requires_python
} else {
let requires_python =
let requires_python = if let Some(requires_python) = requires_python {
if matches!(requires_python.bound(), Bound::Unbounded) {
let default =
RequiresPython::greater_than_equal_version(interpreter.python_minor_version());
if let Some(root_project_name) = root_project_name.as_ref() {
warn_user!(
"No `requires-python` field found in `{root_project_name}`. Defaulting to `{requires_python}`.",
"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., `{default}`).",
);
} else {
warn_user!(
"No `requires-python` field found in workspace. Defaulting to `{requires_python}`.",
"The `requires-python` field does not contain a lower bound: `{requires_python}`. Set a lower bound to indicate the minimum compatible Python version (e.g., `{default}`).",
);
}
}
requires_python
} else {
let default =
RequiresPython::greater_than_equal_version(interpreter.python_minor_version());
if let Some(root_project_name) = root_project_name.as_ref() {
warn_user!(
"No `requires-python` field found in `{root_project_name}`. Defaulting to `{default}`.",
);
} else {
warn_user!("No `requires-python` field found in workspace. Defaulting to `{default}`.",);
}
default
};
// Initialize the registry client.

View file

@ -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.11`).
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<()> {