Add a dedicated error for packages that fail due to `distutils` deprecation (#7239)

## Summary

Closes https://github.com/astral-sh/uv/issues/7183.

## Test Plan

![Screenshot 2024-09-09 at 9 33
45 PM](https://github.com/user-attachments/assets/ffe896cc-d5dd-4ec8-96c9-c37a964489e3)
This commit is contained in:
Charlie Marsh 2024-09-09 22:51:17 -04:00 committed by GitHub
parent a8bd0211e0
commit 53a722cc09
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 183 additions and 33 deletions

View File

@ -1,4 +1,7 @@
use crate::PythonRunnerOutput;
use itertools::Itertools;
use pep440_rs::Version;
use pep508_rs::PackageName;
use regex::Regex;
use std::env;
use std::fmt::{Display, Formatter};
@ -8,8 +11,6 @@ use std::process::ExitStatus;
use std::sync::LazyLock;
use thiserror::Error;
use tracing::error;
use crate::PythonRunnerOutput;
use uv_configuration::BuildOutput;
use uv_fs::Simplified;
@ -46,6 +47,10 @@ static WHEEL_NOT_FOUND_RE: LazyLock<Regex> =
static TORCH_NOT_FOUND_RE: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"ModuleNotFoundError: No module named 'torch'").unwrap());
/// e.g. `ModuleNotFoundError: No module named 'distutils'`
static DISTUTILS_NOT_FOUND_RE: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"ModuleNotFoundError: No module named 'distutils'").unwrap());
#[derive(Error, Debug)]
pub enum Error {
#[error(transparent)]
@ -99,12 +104,15 @@ pub enum Error {
enum MissingLibrary {
Header(String),
Linker(String),
PythonPackage(String),
BuildDependency(String),
DeprecatedModule(String, Version),
}
#[derive(Debug, Error)]
pub struct MissingHeaderCause {
missing_library: MissingLibrary,
package_name: Option<PackageName>,
package_version: Option<Version>,
version_id: String,
}
@ -112,38 +120,80 @@ impl Display for MissingHeaderCause {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match &self.missing_library {
MissingLibrary::Header(header) => {
write!(
f,
"This error likely indicates that you need to install a library that provides \"{}\" for {}",
header, self.version_id
)
if let (Some(package_name), Some(package_version)) =
(&self.package_name, &self.package_version)
{
write!(
f,
"This error likely indicates that you need to install a library that provides \"{header}\" for {package_name}@{package_version}",
)
} else {
write!(
f,
"This error likely indicates that you need to install a library that provides \"{header}\" for {}",
self.version_id
)
}
}
MissingLibrary::Linker(library) => {
write!(
f,
"This error likely indicates that you need to install the library that provides a shared library \
for {library} for {version_id} (e.g. lib{library}-dev)",
library = library, version_id = self.version_id
)
if let (Some(package_name), Some(package_version)) =
(&self.package_name, &self.package_version)
{
write!(
f,
"This error likely indicates that you need to install the library that provides a shared library for {library} for {package_name}@{package_version} (e.g., lib{library}-dev)",
)
} else {
write!(
f,
"This error likely indicates that you need to install the library that provides a shared library for {library} for {version_id} (e.g., lib{library}-dev)",
version_id = self.version_id
)
}
}
MissingLibrary::PythonPackage(package) => {
write!(
f,
"This error likely indicates that {version_id} depends on {package}, but doesn't declare it as a build dependency. \
If {version_id} is a first-party package, consider adding {package} to its `build-system.requires`. \
Otherwise, `uv pip install {package}` into the environment and re-run with `--no-build-isolation`.",
package = package, version_id = self.version_id
)
MissingLibrary::BuildDependency(package) => {
if let (Some(package_name), Some(package_version)) =
(&self.package_name, &self.package_version)
{
write!(
f,
"This error likely indicates that {package_name}@{package_version} depends on {package}, but doesn't declare it as a build dependency. If {package_name} is a first-party package, consider adding {package} to its `build-system.requires`. Otherwise, `uv pip install {package}` into the environment and re-run with `--no-build-isolation`."
)
} else {
write!(
f,
"This error likely indicates that {version_id} depends on {package}, but doesn't declare it as a build dependency. If {version_id} is a first-party package, consider adding {package} to its `build-system.requires`. Otherwise, `uv pip install {package}` into the environment and re-run with `--no-build-isolation`.",
version_id = self.version_id
)
}
}
MissingLibrary::DeprecatedModule(package, version) => {
if let (Some(package_name), Some(package_version)) =
(&self.package_name, &self.package_version)
{
write!(
f,
"{package} was removed from the standard library in Python {version}. Consider adding a constraint (like `{package_name} >{package_version}`) to avoid building a version of {package_name} that depends on {package}.",
)
} else {
write!(
f,
"{package} was removed from the standard library in Python {version}. Consider adding a constraint to avoid building a package-version that depends on {package}.",
)
}
}
}
}
}
impl Error {
/// Construct an [`Error`] from the output of a failed command.
pub(crate) fn from_command_output(
message: String,
output: &PythonRunnerOutput,
level: BuildOutput,
name: Option<&PackageName>,
version: Option<&Version>,
version_id: impl Into<String>,
) -> Self {
// In the cases I've seen it was the 5th and 3rd last line (see test case), 10 seems like a reasonable cutoff.
@ -160,9 +210,14 @@ impl Error {
{
Some(MissingLibrary::Linker(library.to_string()))
} else if WHEEL_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::PythonPackage("wheel".to_string()))
Some(MissingLibrary::BuildDependency("wheel".to_string()))
} else if TORCH_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::PythonPackage("torch".to_string()))
Some(MissingLibrary::BuildDependency("torch".to_string()))
} else if DISTUTILS_NOT_FOUND_RE.is_match(line.trim()) {
Some(MissingLibrary::DeprecatedModule(
"distutils".to_string(),
Version::new([3, 12]),
))
} else {
None
}
@ -175,6 +230,8 @@ impl Error {
exit_code: output.status,
missing_header_cause: MissingHeaderCause {
missing_library,
package_name: name.cloned(),
package_version: version.cloned(),
version_id: version_id.into(),
},
},
@ -185,6 +242,8 @@ impl Error {
stderr: output.stderr.iter().join("\n"),
missing_header_cause: MissingHeaderCause {
missing_library,
package_name: name.cloned(),
package_version: version.cloned(),
version_id: version_id.into(),
},
},
@ -208,10 +267,12 @@ impl Error {
#[cfg(test)]
mod test {
use std::process::ExitStatus;
use crate::{Error, PythonRunnerOutput};
use indoc::indoc;
use pep440_rs::Version;
use pep508_rs::PackageName;
use std::process::ExitStatus;
use std::str::FromStr;
use uv_configuration::BuildOutput;
#[test]
@ -244,6 +305,8 @@ mod test {
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
None,
None,
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
@ -297,6 +360,8 @@ mod test {
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
None,
None,
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
@ -316,7 +381,7 @@ mod test {
"###);
insta::assert_snapshot!(
std::error::Error::source(&err).unwrap(),
@"This error likely indicates that you need to install the library that provides a shared library for ncurses for pygraphviz-1.11 (e.g. libncurses-dev)"
@"This error likely indicates that you need to install the library that provides a shared library for ncurses for pygraphviz-1.11 (e.g., libncurses-dev)"
);
}
@ -343,6 +408,8 @@ mod test {
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
None,
None,
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
@ -366,4 +433,46 @@ mod test {
@"This error likely indicates that pygraphviz-1.11 depends on wheel, but doesn't declare it as a build dependency. If pygraphviz-1.11 is a first-party package, consider adding wheel to its `build-system.requires`. Otherwise, `uv pip install wheel` into the environment and re-run with `--no-build-isolation`."
);
}
#[test]
fn missing_distutils() {
let output = PythonRunnerOutput {
status: ExitStatus::default(), // This is wrong but `from_raw` is platform-gated.
stdout: Vec::new(),
stderr: indoc!(
r"
import distutils.core
ModuleNotFoundError: No module named 'distutils'
"
)
.lines()
.map(ToString::to_string)
.collect(),
};
let err = Error::from_command_output(
"Failed building wheel through setup.py".to_string(),
&output,
BuildOutput::Debug,
Some(&PackageName::from_str("pygraphviz").unwrap()),
Some(&Version::new([1, 11])),
"pygraphviz-1.11",
);
assert!(matches!(err, Error::MissingHeaderOutput { .. }));
// Unix uses exit status, Windows uses exit code.
let formatted = err.to_string().replace("exit status: ", "exit code: ");
insta::assert_snapshot!(formatted, @r###"
Failed building wheel through setup.py with exit code: 0
--- stdout:
--- stderr:
import distutils.core
ModuleNotFoundError: No module named 'distutils'
---
"###);
insta::assert_snapshot!(
std::error::Error::source(&err).unwrap(),
@"distutils was removed from the standard library in Python 3.12. Consider adding a constraint (like `pygraphviz >1.11`) to avoid building a version of pygraphviz that depends on distutils."
);
}
}

View File

@ -214,7 +214,12 @@ pub struct SourceBuild {
/// > directory created by `prepare_metadata_for_build_wheel`, including any unrecognized files
/// > it created.
metadata_directory: Option<PathBuf>,
/// Package id such as `foo-1.2.3`, for error reporting
/// The name of the package, if known.
package_name: Option<PackageName>,
/// The version of the package, if known.
package_version: Option<Version>,
/// Distribution identifier, e.g., `foo-1.2.3`. Used for error reporting if the name and
/// version are unknown.
version_id: String,
/// Whether we do a regular PEP 517 build or an PEP 660 editable build
build_kind: BuildKind,
@ -237,6 +242,7 @@ impl SourceBuild {
source: &Path,
subdirectory: Option<&Path>,
fallback_package_name: Option<&PackageName>,
fallback_package_version: Option<&Version>,
interpreter: &Interpreter,
build_context: &impl BuildContext,
source_build_context: SourceBuildContext,
@ -262,10 +268,19 @@ impl SourceBuild {
let (pep517_backend, project) =
Self::extract_pep517_backend(&source_tree, &default_backend).map_err(|err| *err)?;
let package_name = project.as_ref().map(|p| &p.name).or(fallback_package_name);
let package_name = project
.as_ref()
.map(|project| &project.name)
.or(fallback_package_name)
.cloned();
let package_version = project
.as_ref()
.and_then(|project| project.version.as_ref())
.or(fallback_package_version)
.cloned();
// Create a virtual environment, or install into the shared environment if requested.
let venv = if let Some(venv) = build_isolation.shared_environment(package_name) {
let venv = if let Some(venv) = build_isolation.shared_environment(package_name.as_ref()) {
venv.clone()
} else {
uv_virtualenv::create_venv(
@ -280,7 +295,7 @@ impl SourceBuild {
// Setup the build environment. If build isolation is disabled, we assume the build
// environment is already setup.
if build_isolation.is_isolated(package_name) {
if build_isolation.is_isolated(package_name.as_ref()) {
debug!("Resolving build requirements");
let resolved_requirements = Self::get_resolved_requirements(
@ -334,7 +349,7 @@ impl SourceBuild {
// Create the PEP 517 build environment. If build isolation is disabled, we assume the build
// environment is already setup.
let runner = PythonRunner::new(concurrent_builds, level);
if build_isolation.is_isolated(package_name) {
if build_isolation.is_isolated(package_name.as_ref()) {
debug!("Creating PEP 517 build environment");
create_pep517_build_environment(
@ -343,6 +358,8 @@ impl SourceBuild {
&venv,
&pep517_backend,
build_context,
package_name.as_ref(),
package_version.as_ref(),
&version_id,
build_kind,
level,
@ -364,6 +381,8 @@ impl SourceBuild {
level,
config_settings,
metadata_directory: None,
package_name,
package_version,
version_id,
environment_variables,
modified_path,
@ -548,6 +567,8 @@ impl SourceBuild {
format!("Build backend failed to determine metadata through `prepare_metadata_for_build_{}`", self.build_kind),
&output,
self.level,
self.package_name.as_ref(),
self.package_version.as_ref(),
&self.version_id,
));
}
@ -678,6 +699,8 @@ impl SourceBuild {
),
&output,
self.level,
self.package_name.as_ref(),
self.package_version.as_ref(),
&self.version_id,
));
}
@ -691,6 +714,8 @@ impl SourceBuild {
),
&output,
self.level,
self.package_name.as_ref(),
self.package_version.as_ref(),
&self.version_id,
));
}
@ -721,6 +746,8 @@ async fn create_pep517_build_environment(
venv: &PythonEnvironment,
pep517_backend: &Pep517Backend,
build_context: &impl BuildContext,
package_name: Option<&PackageName>,
package_version: Option<&Version>,
version_id: &str,
build_kind: BuildKind,
level: BuildOutput,
@ -778,6 +805,8 @@ async fn create_pep517_build_environment(
format!("Build backend failed to determine extra requires with `build_{build_kind}()`"),
&output,
level,
package_name,
package_version,
version_id,
));
}
@ -790,6 +819,8 @@ async fn create_pep517_build_environment(
),
&output,
level,
package_name,
package_version,
version_id,
)
})?;
@ -802,6 +833,8 @@ async fn create_pep517_build_environment(
),
&output,
level,
package_name,
package_version,
version_id,
)
})?;

View File

@ -12,7 +12,7 @@ use rustc_hash::FxHashMap;
use tracing::{debug, instrument};
use distribution_types::{
CachedDist, IndexCapabilities, IndexLocations, Name, Resolution, SourceDist,
CachedDist, IndexCapabilities, IndexLocations, Name, Resolution, SourceDist, VersionOrUrlRef,
};
use pypi_types::Requirement;
use uv_build::{SourceBuild, SourceBuildContext};
@ -319,6 +319,13 @@ impl<'a> BuildContext for BuildDispatch<'a> {
build_output: BuildOutput,
) -> Result<SourceBuild> {
let dist_name = dist.map(distribution_types::Name::name);
let dist_version = dist
.map(distribution_types::DistributionMetadata::version_or_url)
.and_then(|version| match version {
VersionOrUrlRef::Version(version) => Some(version),
VersionOrUrlRef::Url(_) => None,
});
// Note we can only prevent builds by name for packages with names
// unless all builds are disabled.
if self
@ -340,6 +347,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
source,
subdirectory,
dist_name,
dist_version,
self.interpreter,
self,
self.source_build_context.clone(),