Split resolver inputs into manifest and options (#446)

## Summary

This is a refactor to address a TODO in the build context whereby we
aren't respecting the resolution options in recursive resolutions. Now,
the options are split out from the resolution _manifest_, and shared
across the build context tree.
This commit is contained in:
Charlie Marsh 2023-11-17 10:53:53 -08:00 committed by GitHub
parent 9db6644be6
commit 03599d2bb4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 145 additions and 119 deletions

View file

@ -1,4 +1,3 @@
use anstream::AutoStream;
use std::borrow::Cow;
use std::fmt::Write;
use std::io::stdout;
@ -6,6 +5,7 @@ use std::path::Path;
use std::str::FromStr;
use std::{env, fs};
use anstream::AutoStream;
use anyhow::{anyhow, Result};
use chrono::{DateTime, Utc};
use colored::Colorize;
@ -19,7 +19,7 @@ use puffin_client::RegistryClientBuilder;
use puffin_dispatch::BuildDispatch;
use puffin_interpreter::Virtualenv;
use puffin_normalize::ExtraName;
use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode};
use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, ResolutionOptions};
use crate::commands::reporters::ResolverReporter;
use crate::commands::{elapsed, ExitStatus};
@ -107,15 +107,8 @@ pub(crate) async fn pip_compile(
.unwrap_or_default();
// Create a manifest of the requirements.
let manifest = Manifest::new(
requirements,
constraints,
preferences,
resolution_mode,
prerelease_mode,
project,
exclude_newer,
);
let manifest = Manifest::new(requirements, constraints, preferences, project);
let options = ResolutionOptions::new(resolution_mode, prerelease_mode, exclude_newer);
// Detect the current Python interpreter.
let platform = Platform::current()?;
@ -164,12 +157,19 @@ pub(crate) async fn pip_compile(
interpreter_info,
fs::canonicalize(venv.python_executable())?,
no_build,
);
)
.with_options(options);
// Resolve the dependencies.
let resolver =
puffin_resolver::Resolver::new(manifest, &markers, &tags, &client, &build_dispatch)
.with_reporter(ResolverReporter::from(printer));
let resolver = puffin_resolver::Resolver::new(
manifest,
options,
&markers,
&tags,
&client,
&build_dispatch,
)
.with_reporter(ResolverReporter::from(printer));
let resolution = match resolver.resolve().await {
Err(puffin_resolver::ResolveError::PubGrub(err)) => {
#[allow(clippy::print_stderr)]

View file

@ -16,7 +16,7 @@ use puffin_cache::{CacheArgs, CacheDir};
use puffin_client::RegistryClientBuilder;
use puffin_dispatch::BuildDispatch;
use puffin_interpreter::Virtualenv;
use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, Resolver};
use puffin_resolver::{Manifest, ResolutionOptions, Resolver};
#[derive(ValueEnum, Default, Clone)]
pub(crate) enum ResolveCliFormat {
@ -61,17 +61,13 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> anyhow::Result<()> {
venv.interpreter_info().simple_version(),
)?;
let resolver = Resolver::new(
// TODO(konstin): Split settings (for all resolutions) and inputs (only for this
// resolution) and attach the former to Self.
Manifest::new(
args.requirements.clone(),
Vec::default(),
Vec::default(),
ResolutionMode::default(),
PreReleaseMode::default(),
None, // TODO(zanieb): We may want to provide a project name here
None,
),
ResolutionOptions::default(),
venv.interpreter_info().markers(),
&tags,
&client,

View file

@ -18,7 +18,7 @@ use puffin_client::RegistryClient;
use puffin_distribution::Metadata;
use puffin_installer::{Builder, Downloader, InstallPlan, Installer, Unzipper};
use puffin_interpreter::{InterpreterInfo, Virtualenv};
use puffin_resolver::{DistFinder, Manifest, PreReleaseMode, ResolutionMode, Resolver};
use puffin_resolver::{DistFinder, Manifest, ResolutionOptions, Resolver};
use puffin_traits::BuildContext;
/// The main implementation of [`BuildContext`], used by the CLI, see [`BuildContext`]
@ -28,8 +28,9 @@ pub struct BuildDispatch {
cache: PathBuf,
interpreter_info: InterpreterInfo,
base_python: PathBuf,
source_build_context: SourceBuildContext,
no_build: bool,
source_build_context: SourceBuildContext,
options: ResolutionOptions,
}
impl BuildDispatch {
@ -45,10 +46,17 @@ impl BuildDispatch {
cache,
interpreter_info,
base_python,
source_build_context: SourceBuildContext::default(),
no_build,
source_build_context: SourceBuildContext::default(),
options: ResolutionOptions::default(),
}
}
#[must_use]
pub fn with_options(mut self, options: ResolutionOptions) -> Self {
self.options = options;
self
}
}
impl BuildContext for BuildDispatch {
@ -79,17 +87,8 @@ impl BuildContext for BuildDispatch {
self.interpreter_info.simple_version(),
)?;
let resolver = Resolver::new(
// TODO(konstin): Split settings (for all resolutions) and inputs (only for this
// resolution) and attach the former to Self.
Manifest::new(
requirements.to_vec(),
Vec::default(),
Vec::default(),
ResolutionMode::default(),
PreReleaseMode::default(),
None, // TODO(zanieb): We may want to provide a project name here
None,
),
Manifest::new(requirements.to_vec(), Vec::default(), Vec::default(), None),
self.options,
self.interpreter_info.markers(),
&tags,
&self.client,

View file

@ -9,7 +9,7 @@ use crate::prerelease_mode::PreReleaseStrategy;
use crate::pubgrub::PubGrubVersion;
use crate::resolution_mode::ResolutionStrategy;
use crate::version_map::VersionMap;
use crate::Manifest;
use crate::{Manifest, ResolutionOptions};
#[derive(Debug)]
pub(crate) struct CandidateSelector {
@ -18,16 +18,16 @@ pub(crate) struct CandidateSelector {
preferences: Preferences,
}
impl From<&Manifest> for CandidateSelector {
impl CandidateSelector {
/// Return a [`CandidateSelector`] for the given [`Manifest`].
fn from(manifest: &Manifest) -> Self {
pub(crate) fn for_resolution(manifest: &Manifest, options: ResolutionOptions) -> Self {
Self {
resolution_strategy: ResolutionStrategy::from_mode(
manifest.resolution_mode,
options.resolution_mode,
manifest.requirements.as_slice(),
),
prerelease_strategy: PreReleaseStrategy::from_mode(
manifest.prerelease_mode,
options.prerelease_mode,
manifest.requirements.as_slice(),
),
preferences: Preferences::from(manifest.preferences.as_slice()),

View file

@ -5,6 +5,7 @@ pub use prerelease_mode::PreReleaseMode;
pub use pubgrub::ResolutionFailureReporter;
pub use resolution::Graph;
pub use resolution_mode::ResolutionMode;
pub use resolution_options::ResolutionOptions;
pub use resolver::{BuildId, Reporter as ResolverReporter, Resolver};
mod candidate_selector;
@ -18,5 +19,6 @@ mod prerelease_mode;
mod pubgrub;
mod resolution;
mod resolution_mode;
mod resolution_options;
mod resolver;
mod version_map;

View file

@ -1,20 +1,13 @@
use chrono::{DateTime, Utc};
use pep508_rs::Requirement;
use puffin_normalize::PackageName;
use crate::prerelease_mode::PreReleaseMode;
use crate::resolution_mode::ResolutionMode;
/// A manifest of requirements, constraints, and preferences.
#[derive(Debug)]
pub struct Manifest {
pub(crate) requirements: Vec<Requirement>,
pub(crate) constraints: Vec<Requirement>,
pub(crate) preferences: Vec<Requirement>,
pub(crate) resolution_mode: ResolutionMode,
pub(crate) prerelease_mode: PreReleaseMode,
pub(crate) project: Option<PackageName>,
pub(crate) exclude_newer: Option<DateTime<Utc>>,
}
impl Manifest {
@ -22,19 +15,13 @@ impl Manifest {
requirements: Vec<Requirement>,
constraints: Vec<Requirement>,
preferences: Vec<Requirement>,
resolution_mode: ResolutionMode,
prerelease_mode: PreReleaseMode,
project: Option<PackageName>,
exclude_newer: Option<DateTime<Utc>>,
) -> Self {
Self {
requirements,
constraints,
preferences,
resolution_mode,
prerelease_mode,
project,
exclude_newer,
}
}
}

View file

@ -0,0 +1,24 @@
use crate::{PreReleaseMode, ResolutionMode};
use chrono::{DateTime, Utc};
/// Options for resolving a manifest.
#[derive(Debug, Default, Copy, Clone)]
pub struct ResolutionOptions {
pub(crate) resolution_mode: ResolutionMode,
pub(crate) prerelease_mode: PreReleaseMode,
pub(crate) exclude_newer: Option<DateTime<Utc>>,
}
impl ResolutionOptions {
pub fn new(
resolution_mode: ResolutionMode,
prerelease_mode: PreReleaseMode,
exclude_newer: Option<DateTime<Utc>>,
) -> Self {
Self {
resolution_mode,
prerelease_mode,
exclude_newer,
}
}
}

View file

@ -40,6 +40,7 @@ use crate::pubgrub::{
};
use crate::resolution::Graph;
use crate::version_map::VersionMap;
use crate::ResolutionOptions;
pub struct Resolver<'a, Context: BuildContext + Sync> {
project: Option<PackageName>,
@ -61,6 +62,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
/// Initialize a new resolver.
pub fn new(
manifest: Manifest,
options: ResolutionOptions,
markers: &'a MarkerEnvironment,
tags: &'a Tags,
client: &'a RegistryClient,
@ -69,7 +71,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
Self {
index: Arc::new(Index::default()),
locks: Arc::new(Locks::default()),
selector: CandidateSelector::from(&manifest),
selector: CandidateSelector::for_resolution(&manifest, options),
allowed_urls: manifest
.requirements
.iter()
@ -85,9 +87,9 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
project: manifest.project,
requirements: manifest.requirements,
constraints: manifest.constraints,
exclude_newer: options.exclude_newer,
markers,
tags,
exclude_newer: manifest.exclude_newer,
client,
build_context,
reporter: None,

View file

@ -17,7 +17,9 @@ use platform_host::{Arch, Os, Platform};
use platform_tags::Tags;
use puffin_client::RegistryClientBuilder;
use puffin_interpreter::{InterpreterInfo, Virtualenv};
use puffin_resolver::{Graph, Manifest, PreReleaseMode, ResolutionMode, Resolver};
use puffin_resolver::{
Graph, Manifest, PreReleaseMode, ResolutionMode, ResolutionOptions, Resolver,
};
use puffin_traits::BuildContext;
struct DummyContext {
@ -65,6 +67,7 @@ impl BuildContext for DummyContext {
async fn resolve(
manifest: Manifest,
options: ResolutionOptions,
markers: &'static MarkerEnvironment,
tags: &Tags,
) -> Result<Graph> {
@ -79,7 +82,7 @@ async fn resolve(
PathBuf::from("/dev/null"),
),
};
let resolver = Resolver::new(manifest, markers, tags, &client, &build_context);
let resolver = Resolver::new(manifest, options, markers, tags, &client, &build_context);
Ok(resolver.resolve().await?)
}
@ -91,13 +94,16 @@ async fn black() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -112,13 +118,16 @@ async fn black_colorama() -> Result<()> {
vec![Requirement::from_str("black[colorama]<=23.9.1").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -133,13 +142,16 @@ async fn black_python_310() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_310, &TAGS_310).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_310,
&TAGS_310,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -156,13 +168,16 @@ async fn black_mypy_extensions() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![Requirement::from_str("mypy-extensions<0.4.4").unwrap()],
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -179,13 +194,16 @@ async fn black_mypy_extensions_extra() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![Requirement::from_str("mypy-extensions[extra]<0.4.4").unwrap()],
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -202,13 +220,16 @@ async fn black_flake8() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![Requirement::from_str("flake8<1").unwrap()],
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -223,13 +244,11 @@ async fn black_lowest() -> Result<()> {
vec![Requirement::from_str("black>21").unwrap()],
vec![],
vec![],
ResolutionMode::Lowest,
PreReleaseMode::default(),
None,
None,
);
let options = ResolutionOptions::new(ResolutionMode::Lowest, PreReleaseMode::default(), None);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?;
insta::assert_display_snapshot!(resolution);
@ -244,13 +263,15 @@ async fn black_lowest_direct() -> Result<()> {
vec![Requirement::from_str("black>21").unwrap()],
vec![],
vec![],
None,
);
let options = ResolutionOptions::new(
ResolutionMode::LowestDirect,
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?;
insta::assert_display_snapshot!(resolution);
@ -265,13 +286,16 @@ async fn black_respect_preference() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![],
vec![Requirement::from_str("black==23.9.0").unwrap()],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -286,13 +310,16 @@ async fn black_ignore_preference() -> Result<()> {
vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![],
vec![Requirement::from_str("black==23.9.2").unwrap()],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
None,
);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(
manifest,
ResolutionOptions::default(),
&MARKERS_311,
&TAGS_311,
)
.await?;
insta::assert_display_snapshot!(resolution);
@ -307,13 +334,11 @@ async fn black_disallow_prerelease() -> Result<()> {
vec![Requirement::from_str("black<=20.0").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::Disallow,
None,
None,
);
let options = ResolutionOptions::new(ResolutionMode::default(), PreReleaseMode::Disallow, None);
let err = resolve(manifest, &MARKERS_311, &TAGS_311)
let err = resolve(manifest, options, &MARKERS_311, &TAGS_311)
.await
.unwrap_err();
@ -330,13 +355,12 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> {
vec![Requirement::from_str("black<=20.0").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::IfNecessary,
None,
None,
);
let options =
ResolutionOptions::new(ResolutionMode::default(), PreReleaseMode::IfNecessary, None);
let err = resolve(manifest, &MARKERS_311, &TAGS_311)
let err = resolve(manifest, options, &MARKERS_311, &TAGS_311)
.await
.unwrap_err();
@ -353,13 +377,11 @@ async fn pylint_disallow_prerelease() -> Result<()> {
vec![Requirement::from_str("pylint==2.3.0").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::Disallow,
None,
None,
);
let options = ResolutionOptions::new(ResolutionMode::default(), PreReleaseMode::Disallow, None);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?;
insta::assert_display_snapshot!(resolution);
@ -374,13 +396,11 @@ async fn pylint_allow_prerelease() -> Result<()> {
vec![Requirement::from_str("pylint==2.3.0").unwrap()],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::Allow,
None,
None,
);
let options = ResolutionOptions::new(ResolutionMode::default(), PreReleaseMode::Allow, None);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?;
insta::assert_display_snapshot!(resolution);
@ -398,13 +418,11 @@ async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> {
],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::Explicit,
None,
None,
);
let options = ResolutionOptions::new(ResolutionMode::default(), PreReleaseMode::Explicit, None);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?;
insta::assert_display_snapshot!(resolution);
@ -422,13 +440,11 @@ async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> {
],
vec![],
vec![],
ResolutionMode::default(),
PreReleaseMode::Explicit,
None,
None,
);
let options = ResolutionOptions::new(ResolutionMode::default(), PreReleaseMode::Explicit, None);
let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolve(manifest, options, &MARKERS_311, &TAGS_311).await?;
insta::assert_display_snapshot!(resolution);