Skip to content

Commit

Permalink
Use separate versions
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 3, 2024
1 parent 0cad13a commit 0fe1635
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 52 deletions.
2 changes: 1 addition & 1 deletion crates/puffin-cli/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ fn compile_python_37() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because there is no version of Python available matching >=3.8 and
black==23.10.1 depends on Python*, black==23.10.1 is forbidden.
black==23.10.1 depends on Python>=3.8, black==23.10.1 is forbidden.
And because root depends on black==23.10.1, version solving failed.
"###);
});
Expand Down
8 changes: 4 additions & 4 deletions crates/puffin-cli/tests/pip_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ fn no_solution() -> Result<()> {
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because flask==3.0.0 depends on werkzeug>=3.0.0 and there is no
version of flask available matching >3.0.0, flask>=3.0.0 depends on
╰─▶ Because there is no version of flask available matching >3.0.0 and
flask==3.0.0 depends on werkzeug>=3.0.0, flask>=3.0.0 depends on
werkzeug>=3.0.0.
And because root depends on werkzeug<1.0.0 and root depends on
flask>=3.0.0, version solving failed.
And because root depends on flask>=3.0.0 and root depends on
werkzeug<1.0.0, version solving failed.
"###);

Ok(())
Expand Down
35 changes: 27 additions & 8 deletions crates/puffin-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ use puffin_traits::OnceMap;
use pypi_types::BaseUrl;

use crate::candidate_selector::CandidateSelector;
use crate::pubgrub::{PubGrubHints, PubGrubPackage, PubGrubReportFormatter, PubGrubVersion};
use crate::pubgrub::{
PubGrubHints, PubGrubPackage, PubGrubPython, PubGrubReportFormatter, PubGrubVersion,
};
use crate::python_requirement::PythonRequirement;
use crate::version_map::VersionMap;

#[derive(Error, Debug)]
Expand Down Expand Up @@ -159,21 +162,37 @@ impl NoSolutionError {
#[must_use]
pub(crate) fn with_available_versions(
mut self,
python_requirement: &PythonRequirement,
package_versions: &OnceMap<PackageName, (IndexUrl, BaseUrl, VersionMap)>,
) -> Self {
let mut available_versions = FxHashMap::default();
for package in self.derivation_tree.packages() {
if let PubGrubPackage::Package(name, ..) = package {
if let Some(entry) = package_versions.get(name) {
let (_, _, version_map) = entry.value();
match package {
PubGrubPackage::Root(_) => {}
PubGrubPackage::Python(PubGrubPython::Installed) => {
available_versions.insert(
package.clone(),
version_map
.iter()
.map(|(version, _)| version.clone())
.collect(),
vec![PubGrubVersion::from(python_requirement.installed().clone())],
);
}
PubGrubPackage::Python(PubGrubPython::Target) => {
available_versions.insert(
package.clone(),
vec![PubGrubVersion::from(python_requirement.target().clone())],
);
}
PubGrubPackage::Package(name, ..) => {
if let Some(entry) = package_versions.get(name) {
let (_, _, version_map) = entry.value();
available_versions.insert(
package.clone(),
version_map
.iter()
.map(|(version, _)| version.clone())
.collect(),
);
}
}
}
}
self.available_versions = available_versions;
Expand Down
2 changes: 1 addition & 1 deletion crates/puffin-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ fn merge_package(
(PubGrubPackage::Root(_), _) | (_, PubGrubPackage::Root(_)) => Ok(None),

// Either package is the Python installation.
(PubGrubPackage::Python, _) | (_, PubGrubPackage::Python) => Ok(None),
(PubGrubPackage::Python(_), _) | (_, PubGrubPackage::Python(_)) => Ok(None),

// Left package has a URL. Propagate the URL.
(PubGrubPackage::Package(name, extra, Some(url)), PubGrubPackage::Package(.., None)) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/puffin-resolver/src/pubgrub/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub(crate) use crate::pubgrub::dependencies::PubGrubDependencies;
pub(crate) use crate::pubgrub::distribution::PubGrubDistribution;
pub(crate) use crate::pubgrub::package::PubGrubPackage;
pub(crate) use crate::pubgrub::package::{PubGrubPackage, PubGrubPython};
pub(crate) use crate::pubgrub::priority::{PubGrubPriorities, PubGrubPriority};
pub(crate) use crate::pubgrub::report::{PubGrubHints, PubGrubReportFormatter};
pub(crate) use crate::pubgrub::specifier::PubGrubSpecifier;
Expand Down
14 changes: 11 additions & 3 deletions crates/puffin-resolver/src/pubgrub/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use puffin_normalize::{ExtraName, PackageName};
pub enum PubGrubPackage {
/// The root package, which is used to start the resolution process.
Root(Option<PackageName>),
/// The current Python version.
Python,
/// A Python version.
Python(PubGrubPython),
/// A Python package.
Package(
PackageName,
Expand Down Expand Up @@ -74,6 +74,14 @@ pub enum PubGrubPackage {
),
}

#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub enum PubGrubPython {
/// The Python version installed in the current environment.
Installed,
/// The Python version for which dependencies are being resolved.
Target,
}

impl std::fmt::Display for PubGrubPackage {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand All @@ -84,7 +92,7 @@ impl std::fmt::Display for PubGrubPackage {
write!(f, "root")
}
}
PubGrubPackage::Python => write!(f, "Python"),
PubGrubPackage::Python(_) => write!(f, "Python"),
PubGrubPackage::Package(name, None, ..) => write!(f, "{name}"),
PubGrubPackage::Package(name, Some(extra), ..) => {
write!(f, "{name}[{extra}]")
Expand Down
2 changes: 1 addition & 1 deletion crates/puffin-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl PubGrubPriorities {
pub(crate) fn get(&self, package: &PubGrubPackage) -> Option<PubGrubPriority> {
match package {
PubGrubPackage::Root(_) => Some(Reverse(0)),
PubGrubPackage::Python => Some(Reverse(0)),
PubGrubPackage::Python(_) => Some(Reverse(0)),
PubGrubPackage::Package(name, _, _) => self
.0
.get(name)
Expand Down
25 changes: 11 additions & 14 deletions crates/puffin-resolver/src/python_requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,29 @@ pub struct PythonRequirement<'a> {
/// The target version of Python; that is, the version of Python for which we are resolving
/// dependencies. This is typically the same as the installed version, but may be different
/// when specifying an alternate Python version for the resolution.
target: Option<&'a Version>,
target: &'a Version,
}

impl<'a> PythonRequirement<'a> {
pub fn new(interpreter: &'a Interpreter, markers: &'a MarkerEnvironment) -> Self {
let installed = interpreter.version();
let target = &markers.python_version.version;
Self {
installed,
target: if installed == target {
None
} else {
Some(target)
},
installed: interpreter.version(),
target: &markers.python_version.version,
}
}

/// Return a version in the given range.
pub(crate) fn version(&self) -> &'a Version {
/// Return the installed version of Python.
pub(crate) fn installed(&self) -> &'a Version {
self.installed
}

/// Return the target version of Python.
pub(crate) fn target(&self) -> &'a Version {
self.target
}

/// Returns an iterator over the versions of Python to consider when resolving dependencies.
pub(crate) fn versions(&self) -> impl Iterator<Item = &'a Version> {
self.target
.into_iter()
.chain(std::iter::once(self.installed))
std::iter::once(self.installed).chain(std::iter::once(self.target))
}
}
48 changes: 31 additions & 17 deletions crates/puffin-resolver/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ use crate::manifest::Manifest;
use crate::overrides::Overrides;
use crate::pins::FilePins;
use crate::pubgrub::{
PubGrubDependencies, PubGrubDistribution, PubGrubPackage, PubGrubPriorities, PubGrubSpecifier,
PubGrubVersion, MIN_VERSION,
PubGrubDependencies, PubGrubDistribution, PubGrubPackage, PubGrubPriorities, PubGrubPython,
PubGrubSpecifier, PubGrubVersion, MIN_VERSION,
};
use crate::python_requirement::PythonRequirement;
use crate::resolution::ResolutionGraph;
Expand Down Expand Up @@ -297,7 +297,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
resolution.map_err(|err| {
// Add version information to improve unsat error messages
if let ResolveError::NoSolution(err) = err {
ResolveError::NoSolution(err.with_available_versions(&self.index.packages).with_selector(self.selector.clone()))
ResolveError::NoSolution(err.with_available_versions(&self.python_requirement, &self.index.packages).with_selector(self.selector.clone()))
} else {
err
}
Expand Down Expand Up @@ -450,7 +450,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
) -> Result<(), ResolveError> {
match package {
PubGrubPackage::Root(_) => {}
PubGrubPackage::Python => {}
PubGrubPackage::Python(_) => {}
PubGrubPackage::Package(package_name, _extra, None) => {
// Emit a request to fetch the metadata for this package.
if index.packages.register(package_name) {
Expand Down Expand Up @@ -532,16 +532,22 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
return match package {
PubGrubPackage::Root(_) => Ok(Some(MIN_VERSION.clone())),

PubGrubPackage::Python => {
if self
.python_requirement
.versions()
.any(|version| !range.contains(&PubGrubVersion::from(version.clone())))
{
return Ok(None);
PubGrubPackage::Python(PubGrubPython::Installed) => {
let version = PubGrubVersion::from(self.python_requirement.installed().clone());
if range.contains(&version) {
Ok(Some(version))
} else {
Ok(None)
}
}

PubGrubPackage::Python(PubGrubPython::Target) => {
let version = PubGrubVersion::from(self.python_requirement.target().clone());
if range.contains(&version) {
Ok(Some(version))
} else {
Ok(None)
}
let version = PubGrubVersion::from(self.python_requirement.version().clone());
Ok(Some(version))
}

PubGrubPackage::Package(package_name, extra, Some(url)) => {
Expand Down Expand Up @@ -702,7 +708,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
Ok(Dependencies::Known(constraints.into()))
}

PubGrubPackage::Python => Ok(Dependencies::Known(DependencyConstraints::default())),
PubGrubPackage::Python(_) => Ok(Dependencies::Known(DependencyConstraints::default())),

PubGrubPackage::Package(package_name, extra, url) => {
// Wait for the metadata to be available.
Expand All @@ -724,7 +730,11 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
})?;

let mut constraints = DependencyConstraints::default();
constraints.insert(PubGrubPackage::Python, version.clone());
constraints.insert(
PubGrubPackage::Python(PubGrubPython::Installed),
version.clone(),
);
constraints.insert(PubGrubPackage::Python(PubGrubPython::Target), version);
return Ok(Dependencies::Known(constraints));
}

Expand Down Expand Up @@ -756,7 +766,11 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier.into())
})?;
constraints.insert(PubGrubPackage::Python, version.clone());
constraints.insert(
PubGrubPackage::Python(PubGrubPython::Installed),
version.clone(),
);
constraints.insert(PubGrubPackage::Python(PubGrubPython::Target), version);
}

// If a package has an extra, insert a constraint on the base package.
Expand Down Expand Up @@ -856,7 +870,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
if let Some(reporter) = self.reporter.as_ref() {
match package {
PubGrubPackage::Root(_) => {}
PubGrubPackage::Python => {}
PubGrubPackage::Python(_) => {}
PubGrubPackage::Package(package_name, _extra, Some(url)) => {
reporter.on_progress(package_name, VersionOrUrl::Url(url));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/puffin-resolver/src/version_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl VersionMap {
match filename {
DistFilename::WheelFilename(filename) => {
// To be compatible, the wheel must both have compatible tags _and_ have a
// compatible Python version marker.
// compatible Python requirement.
let priority = filename.compatibility(tags).filter(|_| {
file.requires_python
.as_ref()
Expand Down
1 change: 0 additions & 1 deletion requirements.in

This file was deleted.

0 comments on commit 0fe1635

Please sign in to comment.