Skip to content

Commit

Permalink
feat: switch the interpreter to the build options (#136)
Browse files Browse the repository at this point in the history
Moves the `PythonLocation` to the resolve options, I think it makes more
sense there 👍
  • Loading branch information
tdejager authored Jan 8, 2024
1 parent b9110f4 commit 123ea11
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 49 deletions.
22 changes: 0 additions & 22 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions crates/rattler_installs_packages/src/artifacts/sdist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ fn generic_archive_reader(
#[cfg(test)]
mod tests {
use crate::artifacts::SDist;
use crate::python_env::{Pep508EnvMakers, PythonLocation};
use crate::python_env::Pep508EnvMakers;
use crate::wheel_builder::WheelBuilder;
use crate::{index::PackageDb, resolve::ResolveOptions};
use insta::{assert_debug_snapshot, assert_ron_snapshot};
Expand Down Expand Up @@ -261,7 +261,6 @@ mod tests {
None,
&resolve_options,
package_db.1.path(),
PythonLocation::System,
);

let result = wheel_builder.get_sdist_metadata(&sdist).await.unwrap();
Expand All @@ -285,7 +284,6 @@ mod tests {
None,
&resolve_options,
package_db.1.path(),
PythonLocation::System,
);

// Build the wheel
Expand All @@ -311,7 +309,6 @@ mod tests {
None,
&resolve_options,
package_db.1.path(),
PythonLocation::System,
);

// Build the wheel
Expand Down
2 changes: 1 addition & 1 deletion crates/rattler_installs_packages/src/python_env/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ mod tests {
#[test]
pub fn venv_creation() {
let venv_dir = tempfile::tempdir().unwrap();
let venv = VEnv::create(&venv_dir.path(), PythonLocation::System).unwrap();
let venv = VEnv::create(venv_dir.path(), PythonLocation::System).unwrap();

// Does python exist
assert!(venv.python_executable().is_file());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::SDistResolution;
use crate::artifacts::SDist;
use crate::artifacts::Wheel;
use crate::index::PackageDb;
use crate::python_env::{PythonLocation, WheelTags};
use crate::python_env::WheelTags;
use crate::resolve::{PinnedPackage, ResolveOptions};
use crate::types::{
Artifact, ArtifactInfo, ArtifactName, Extra, NormalizedPackageName, PackageName,
Expand Down Expand Up @@ -140,15 +140,13 @@ impl<'db, 'i> PypiDependencyProvider<'db, 'i> {
locked_packages: HashMap<NormalizedPackageName, PinnedPackage<'db>>,
favored_packages: HashMap<NormalizedPackageName, PinnedPackage<'db>>,
options: &'i ResolveOptions,
python_location: PythonLocation,
) -> miette::Result<Self> {
let wheel_builder = WheelBuilder::new(
package_db,
markers,
compatible_tags,
options,
package_db.cache_dir(),
python_location,
);

Ok(Self {
Expand Down
6 changes: 4 additions & 2 deletions crates/rattler_installs_packages/src/resolve/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ pub struct ResolveOptions {
/// Defines how to handle sdists during resolution. By default sdists will be treated the same
/// as wheels.
pub sdist_resolution: SDistResolution,

/// Defines what python interpreter to use for resolution. By default the python interpreter
/// from the system is used. This is only used during resolution and building of wheel files
pub python_location: PythonLocation,
}

/// Resolves an environment that contains the given requirements and all dependencies of those
Expand All @@ -168,7 +172,6 @@ pub async fn resolve<'db>(
locked_packages: HashMap<NormalizedPackageName, PinnedPackage<'db>>,
favored_packages: HashMap<NormalizedPackageName, PinnedPackage<'db>>,
options: &ResolveOptions,
python_location: PythonLocation,
) -> miette::Result<Vec<PinnedPackage<'db>>> {
// Construct a provider
let provider = PypiDependencyProvider::new(
Expand All @@ -178,7 +181,6 @@ pub async fn resolve<'db>(
locked_packages,
favored_packages,
options,
python_location,
)?;
let pool = &provider.pool;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub(crate) struct BuildEnvironment<'db> {
build_requirements: Vec<Requirement>,
resolved_wheels: Vec<PinnedPackage<'db>>,
venv: VEnv,
#[allow(dead_code)]
python_location: PythonLocation,
}

Expand Down Expand Up @@ -90,7 +91,7 @@ impl<'db> BuildEnvironment<'db> {
.cloned()
.collect::<Vec<_>>();

// Install extra requirements if any new ones were foujnd
// Install extra requirements if any new ones were found
if !extra_requirements.is_empty()
&& self.build_requirements.len() != combined_requirements.len()
{
Expand All @@ -106,7 +107,6 @@ impl<'db> BuildEnvironment<'db> {
locked_packages,
favored_packages,
resolve_options,
self.python_location.clone(),
)
.await
.map_err(|_| WheelBuildError::CouldNotResolveEnvironment(all_requirements))?;
Expand Down Expand Up @@ -154,11 +154,13 @@ impl<'db> BuildEnvironment<'db> {
env_markers: &MarkerEnvironment,
wheel_tags: Option<&WheelTags>,
resolve_options: &ResolveOptions,
python_location: PythonLocation,
) -> Result<BuildEnvironment<'db>, WheelBuildError> {
// Setup a work directory and a new env dir
let work_dir = tempfile::tempdir().unwrap();
let venv = VEnv::create(&work_dir.path().join("venv"), python_location.clone())?;
let venv = VEnv::create(
&work_dir.path().join("venv"),
resolve_options.python_location.clone(),
)?;

// Find the build system
let build_system =
Expand Down Expand Up @@ -187,7 +189,6 @@ impl<'db> BuildEnvironment<'db> {
HashMap::default(),
HashMap::default(),
resolve_options,
python_location.clone(),
)
.await
.map_err(|_| WheelBuildError::CouldNotResolveEnvironment(build_requirements.to_vec()))?;
Expand Down Expand Up @@ -232,7 +233,7 @@ impl<'db> BuildEnvironment<'db> {
entry_point,
resolved_wheels,
venv,
python_location,
python_location: resolve_options.python_location.clone(),
})
}
}
12 changes: 3 additions & 9 deletions crates/rattler_installs_packages/src/wheel_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{collections::HashMap, path::PathBuf};
use parking_lot::Mutex;
use pep508_rs::{MarkerEnvironment, Requirement};

use crate::python_env::{PythonLocation, VEnvError};
use crate::python_env::VEnvError;
use crate::resolve::{ResolveOptions, SDistResolution};
use crate::types::{NormalizedPackageName, ParseArtifactNameError, WheelFilename};
use crate::wheel_builder::build_environment::BuildEnvironment;
Expand Down Expand Up @@ -50,9 +50,6 @@ pub struct WheelBuilder<'db, 'i> {

/// Cache of locally built wheels on the system
locally_built_wheels: WheelCache,

/// Location of the python interpreter to use
python_location: PythonLocation,
}

/// An error that can occur while building a wheel
Expand Down Expand Up @@ -132,7 +129,6 @@ impl<'db, 'i> WheelBuilder<'db, 'i> {
wheel_tags: Option<&'i WheelTags>,
resolve_options: &'i ResolveOptions,
wheel_cache_dir: &Path,
python_location: PythonLocation,
) -> Self {
// We are running into a chicken & egg problem if we want to build wheels for packages that
// require their build system as sdist as well. For example, `hatchling` requires `hatchling` as
Expand All @@ -142,6 +138,7 @@ impl<'db, 'i> WheelBuilder<'db, 'i> {
let resolve_options = if resolve_options.sdist_resolution == SDistResolution::OnlySDists {
ResolveOptions {
sdist_resolution: SDistResolution::PreferWheels,
..resolve_options.clone()
}
} else {
resolve_options.clone()
Expand All @@ -154,7 +151,6 @@ impl<'db, 'i> WheelBuilder<'db, 'i> {
wheel_tags,
resolve_options,
locally_built_wheels: WheelCache::new(wheel_cache_dir.to_path_buf()),
python_location,
}
}

Expand Down Expand Up @@ -183,7 +179,6 @@ impl<'db, 'i> WheelBuilder<'db, 'i> {
self.env_markers,
self.wheel_tags,
&self.resolve_options,
self.python_location.clone(),
)
.await?;

Expand Down Expand Up @@ -321,7 +316,7 @@ impl<'db, 'i> WheelBuilder<'db, 'i> {
mod tests {
use crate::artifacts::SDist;
use crate::index::PackageDb;
use crate::python_env::{Pep508EnvMakers, PythonLocation};
use crate::python_env::Pep508EnvMakers;
use crate::resolve::ResolveOptions;
use crate::wheel_builder::wheel_cache::WheelKey;
use crate::wheel_builder::WheelBuilder;
Expand Down Expand Up @@ -357,7 +352,6 @@ mod tests {
None,
&resolve_options,
package_db.1.path(),
PythonLocation::System,
);

// Build the wheel
Expand Down
3 changes: 1 addition & 2 deletions crates/rip_bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ async fn actual_main() -> miette::Result<()> {

let resolve_opts = ResolveOptions {
sdist_resolution: args.sdist_resolution.into(),
..Default::default()
};
// Solve the environment
let blueprint = match resolve(
Expand All @@ -140,7 +141,6 @@ async fn actual_main() -> miette::Result<()> {
HashMap::default(),
HashMap::default(),
&resolve_opts,
PythonLocation::System,
)
.await
{
Expand Down Expand Up @@ -201,7 +201,6 @@ async fn actual_main() -> miette::Result<()> {
Some(&compatible_tags),
&resolve_opts,
package_db.cache_dir(),
PythonLocation::System,
);

for pinned_package in blueprint.into_iter().sorted_by(|a, b| a.name.cmp(&b.name)) {
Expand Down

0 comments on commit 123ea11

Please sign in to comment.