Skip to content

Commit

Permalink
Respect index strategy in source distribution builds (#4468)
Browse files Browse the repository at this point in the history
## Summary

The `--index-strategy` is linked to the index locations, which we
propagate to source distribution builds; so it makes sense to pass the
`--index-strategy` too.

While I was here, I made `exclude_newer` a required argument so that we
don't forget to set it via the `with_options` builder.

Closes #4465.
  • Loading branch information
charliermarsh committed Jun 24, 2024
1 parent 1eee427 commit cba270f
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 123 deletions.
27 changes: 14 additions & 13 deletions crates/bench/benches/uv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ mod resolver {
use uv_cache::Cache;
use uv_client::RegistryClient;
use uv_configuration::{
BuildOptions, Concurrency, ConfigSettings, PreviewMode, SetupPyStrategy,
BuildOptions, Concurrency, ConfigSettings, IndexStrategy, PreviewMode, SetupPyStrategy,
};
use uv_dispatch::BuildDispatch;
use uv_distribution::DistributionDatabase;
Expand Down Expand Up @@ -128,8 +128,17 @@ mod resolver {
venv: &PythonEnvironment,
) -> Result<ResolutionGraph> {
let build_isolation = BuildIsolation::Isolated;
let build_options = BuildOptions::default();
let concurrency = Concurrency::default();
let config_settings = ConfigSettings::default();
let exclude_newer = Some(
NaiveDate::from_ymd_opt(2024, 6, 20)
.unwrap()
.and_hms_opt(0, 0, 0)
.unwrap()
.and_utc()
.into(),
);
let flat_index = FlatIndex::default();
let git = GitResolver::default();
let hashes = HashStrategy::None;
Expand All @@ -139,7 +148,8 @@ mod resolver {
let installed_packages = EmptyInstalledPackages;
let interpreter = venv.interpreter();
let python_requirement = PythonRequirement::from_interpreter(interpreter);
let build_options = BuildOptions::default();

let options = OptionsBuilder::new().exclude_newer(exclude_newer).build();

let build_context = BuildDispatch::new(
client,
Expand All @@ -150,26 +160,17 @@ mod resolver {
&index,
&git,
&in_flight,
IndexStrategy::default(),
SetupPyStrategy::default(),
&config_settings,
build_isolation,
LinkMode::default(),
&build_options,
exclude_newer,
concurrency,
PreviewMode::Disabled,
);

let options = OptionsBuilder::new()
.exclude_newer(Some(
NaiveDate::from_ymd_opt(2024, 6, 20)
.unwrap()
.and_hms_opt(0, 0, 0)
.unwrap()
.and_utc()
.into(),
))
.build();

let resolver = Resolver::new(
manifest,
options,
Expand Down
7 changes: 6 additions & 1 deletion crates/uv-dev/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use uv_build::{SourceBuild, SourceBuildContext};
use uv_cache::{Cache, CacheArgs};
use uv_client::RegistryClientBuilder;
use uv_configuration::{
BuildKind, BuildOptions, Concurrency, ConfigSettings, PreviewMode, SetupPyStrategy,
BuildKind, BuildOptions, Concurrency, ConfigSettings, IndexStrategy, PreviewMode,
SetupPyStrategy,
};
use uv_dispatch::BuildDispatch;
use uv_git::GitResolver;
Expand Down Expand Up @@ -59,11 +60,13 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
let client = RegistryClientBuilder::new(cache.clone()).build();
let concurrency = Concurrency::default();
let config_settings = ConfigSettings::default();
let exclude_newer = None;
let flat_index = FlatIndex::default();
let git = GitResolver::default();
let in_flight = InFlight::default();
let index = InMemoryIndex::default();
let index_urls = IndexLocations::default();
let index_strategy = IndexStrategy::default();
let setup_py = SetupPyStrategy::default();
let toolchain = PythonEnvironment::find(
&ToolchainRequest::default(),
Expand All @@ -81,11 +84,13 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
&index,
&git,
&in_flight,
index_strategy,
setup_py,
&config_settings,
BuildIsolation::Isolated,
install_wheel_rs::linker::LinkMode::default(),
&build_options,
exclude_newer,
concurrency,
PreviewMode::Enabled,
);
Expand Down
35 changes: 16 additions & 19 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,16 @@ use pypi_types::Requirement;
use uv_build::{SourceBuild, SourceBuildContext};
use uv_cache::Cache;
use uv_client::RegistryClient;
use uv_configuration::{BuildKind, BuildOptions, ConfigSettings, Reinstall, SetupPyStrategy};
use uv_configuration::{
BuildKind, BuildOptions, ConfigSettings, IndexStrategy, Reinstall, SetupPyStrategy,
};
use uv_configuration::{Concurrency, PreviewMode};
use uv_distribution::DistributionDatabase;
use uv_git::GitResolver;
use uv_installer::{Installer, Plan, Planner, Preparer, SitePackages};
use uv_resolver::{FlatIndex, InMemoryIndex, Manifest, Options, PythonRequirement, Resolver};
use uv_resolver::{
ExcludeNewer, FlatIndex, InMemoryIndex, Manifest, OptionsBuilder, PythonRequirement, Resolver,
};
use uv_toolchain::{Interpreter, PythonEnvironment};
use uv_types::{BuildContext, BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight};

Expand All @@ -32,6 +36,7 @@ pub struct BuildDispatch<'a> {
cache: &'a Cache,
interpreter: &'a Interpreter,
index_locations: &'a IndexLocations,
index_strategy: IndexStrategy,
flat_index: &'a FlatIndex,
index: &'a InMemoryIndex,
git: &'a GitResolver,
Expand All @@ -41,8 +46,8 @@ pub struct BuildDispatch<'a> {
link_mode: install_wheel_rs::linker::LinkMode,
build_options: &'a BuildOptions,
config_settings: &'a ConfigSettings,
exclude_newer: Option<ExcludeNewer>,
source_build_context: SourceBuildContext,
options: Options,
build_extra_env_vars: FxHashMap<OsString, OsString>,
concurrency: Concurrency,
preview_mode: PreviewMode,
Expand All @@ -59,11 +64,13 @@ impl<'a> BuildDispatch<'a> {
index: &'a InMemoryIndex,
git: &'a GitResolver,
in_flight: &'a InFlight,
index_strategy: IndexStrategy,
setup_py: SetupPyStrategy,
config_settings: &'a ConfigSettings,
build_isolation: BuildIsolation<'a>,
link_mode: install_wheel_rs::linker::LinkMode,
build_options: &'a BuildOptions,
exclude_newer: Option<ExcludeNewer>,
concurrency: Concurrency,
preview_mode: PreviewMode,
) -> Self {
Expand All @@ -76,25 +83,20 @@ impl<'a> BuildDispatch<'a> {
index,
git,
in_flight,
index_strategy,
setup_py,
config_settings,
build_isolation,
link_mode,
build_options,
exclude_newer,
concurrency,
source_build_context: SourceBuildContext::default(),
options: Options::default(),
build_extra_env_vars: FxHashMap::default(),
preview_mode,
}
}

#[must_use]
pub fn with_options(mut self, options: Options) -> Self {
self.options = options;
self
}

/// Set the environment variables to be used when building a source distribution.
#[must_use]
pub fn with_build_extra_env_vars<I, K, V>(mut self, sdist_build_env_variables: I) -> Self
Expand Down Expand Up @@ -126,10 +128,6 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.interpreter
}

fn build_isolation(&self) -> BuildIsolation {
self.build_isolation
}

fn build_options(&self) -> &BuildOptions {
self.build_options
}
Expand All @@ -138,17 +136,16 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.index_locations
}

fn setup_py_strategy(&self) -> SetupPyStrategy {
self.setup_py
}

async fn resolve<'data>(&'data self, requirements: &'data [Requirement]) -> Result<Resolution> {
let python_requirement = PythonRequirement::from_interpreter(self.interpreter);
let markers = self.interpreter.markers();
let tags = self.interpreter.tags()?;
let resolver = Resolver::new(
Manifest::simple(requirements.to_vec()),
self.options,
OptionsBuilder::new()
.exclude_newer(self.exclude_newer)
.index_strategy(self.index_strategy)
.build(),
&python_requirement,
Some(markers),
Some(tags),
Expand Down
14 changes: 3 additions & 11 deletions crates/uv-types/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ use distribution_types::{CachedDist, IndexLocations, InstalledDist, Resolution,
use pep508_rs::PackageName;
use pypi_types::Requirement;
use uv_cache::Cache;
use uv_configuration::{BuildKind, BuildOptions, SetupPyStrategy};
use uv_configuration::{BuildKind, BuildOptions};
use uv_git::GitResolver;
use uv_toolchain::{Interpreter, PythonEnvironment};

use crate::BuildIsolation;

/// Avoids cyclic crate dependencies between resolver, installer and builder.
///
/// To resolve the dependencies of a packages, we may need to build one or more source
Expand Down Expand Up @@ -63,9 +61,6 @@ pub trait BuildContext {
/// it's metadata (e.g. wheel compatibility tags).
fn interpreter(&self) -> &Interpreter;

/// Whether to enforce build isolation when building source distributions.
fn build_isolation(&self) -> BuildIsolation;

/// Whether source distribution building or pre-built wheels is disabled.
///
/// This [`BuildContext::setup_build`] calls will fail if builds are disabled.
Expand All @@ -75,9 +70,6 @@ pub trait BuildContext {
/// The index locations being searched.
fn index_locations(&self) -> &IndexLocations;

/// The strategy to use when building source distributions that lack a `pyproject.toml`.
fn setup_py_strategy(&self) -> SetupPyStrategy;

/// Resolve the given requirements into a ready-to-install set of package versions.
fn resolve<'a>(
&'a self,
Expand All @@ -92,7 +84,7 @@ pub trait BuildContext {
venv: &'a PythonEnvironment,
) -> impl Future<Output = Result<Vec<CachedDist>>> + 'a;

/// Setup a source distribution build by installing the required dependencies. A wrapper for
/// Set up a source distribution build by installing the required dependencies. A wrapper for
/// `uv_build::SourceBuild::setup`.
///
/// For PEP 517 builds, this calls `get_requires_for_build_wheel`.
Expand Down Expand Up @@ -141,7 +133,7 @@ pub trait InstalledPackagesProvider: Clone + Send + Sync + 'static {
pub struct EmptyInstalledPackages;

impl InstalledPackagesProvider for EmptyInstalledPackages {
fn get_packages(&self, _name: &pep508_rs::PackageName) -> Vec<&InstalledDist> {
fn get_packages(&self, _name: &PackageName) -> Vec<&InstalledDist> {
Vec::new()
}

Expand Down
5 changes: 3 additions & 2 deletions crates/uv/src/commands/pip/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,16 @@ pub(crate) async fn pip_compile(
&source_index,
&git,
&in_flight,
index_strategy,
setup_py,
&config_settings,
build_isolation,
link_mode,
&build_options,
exclude_newer,
concurrency,
preview,
)
.with_options(OptionsBuilder::new().exclude_newer(exclude_newer).build());
);

// Resolve the requirements from the provided sources.
let requirements = {
Expand Down
8 changes: 5 additions & 3 deletions crates/uv/src/commands/pip/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,15 +294,16 @@ pub(crate) async fn pip_install(
&index,
&git,
&in_flight,
index_strategy,
setup_py,
config_settings,
build_isolation,
link_mode,
&build_options,
exclude_newer,
concurrency,
preview,
)
.with_options(OptionsBuilder::new().exclude_newer(exclude_newer).build());
);

let options = OptionsBuilder::new()
.resolution_mode(resolution_mode)
Expand Down Expand Up @@ -368,15 +369,16 @@ pub(crate) async fn pip_install(
&index,
&git,
&in_flight,
index_strategy,
setup_py,
config_settings,
build_isolation,
link_mode,
&build_options,
exclude_newer,
concurrency,
preview,
)
.with_options(OptionsBuilder::new().exclude_newer(exclude_newer).build())
};

// Sync the environment.
Expand Down
8 changes: 5 additions & 3 deletions crates/uv/src/commands/pip/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,16 @@ pub(crate) async fn pip_sync(
&index,
&git,
&in_flight,
index_strategy,
setup_py,
config_settings,
build_isolation,
link_mode,
&build_options,
exclude_newer,
concurrency,
preview,
)
.with_options(OptionsBuilder::new().exclude_newer(exclude_newer).build());
);

// Determine the set of installed packages.
let site_packages = SitePackages::from_executable(&environment)?;
Expand Down Expand Up @@ -320,15 +321,16 @@ pub(crate) async fn pip_sync(
&index,
&git,
&in_flight,
index_strategy,
setup_py,
config_settings,
build_isolation,
link_mode,
&build_options,
exclude_newer,
concurrency,
preview,
)
.with_options(OptionsBuilder::new().exclude_newer(exclude_newer).build())
};

// Sync the environment.
Expand Down
Loading

0 comments on commit cba270f

Please sign in to comment.