Skip to content

Commit

Permalink
refactor: remove all lifetimes from PackageDb and WheelBuilder to sim…
Browse files Browse the repository at this point in the history 
…plify code (prefix-dev#186)

Removes the lifetimes from `WheelBuilder` and `resolve` in favor of some
`Arc`s. I assume this will have only a very minimal impact on
performance/memory usage but it makes the code a hell of a lot easier to
understand.

The motivation to remove them is for concurrency where I often want to
`tokio::spawn` stuff. This requires that the future is `'static` which
is often not possible with all the lifetimes we used.

Most likely some lifetimes could be returned in the `resolve` function
(which don't bubble through the rest of the code) but we can do that
later.

Some of the things I turned into Arcs:

* `PackageDb`: we often want to share this between different tasks and
datastructures.
* `WheelTags`: this can amount to a lot of data so I thought it would be
worth to wrap in an Arc.
* `MarkerEnvironment`: takes up some memory, probably not worth cloning
all over the place.
* `ArtifactInfo`: This one bubbled the `'db` lifetime through to
everything. I simply used `Arc<ArtifactInfo>` now.
  • Loading branch information
@baszalmstra
baszalmstra authored Feb 1, 2024
1 parent 15a9215 commit fd843c6
Show file tree
Hide file tree
Showing 8 changed files with 310 additions and 315 deletions.
5 changes: 5 additions & 0 deletions Cargo.lock
View file

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

143 changes: 68 additions & 75 deletions crates/rattler_installs_packages/src/artifacts/sdist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,20 +390,23 @@ mod tests {
use std::env;
use std::path::Path;
use std::str::FromStr;
use std::sync::Arc;
use tempfile::TempDir;
use url::Url;

fn get_package_db() -> (PackageDb, TempDir) {
fn get_package_db() -> (Arc<PackageDb>, TempDir) {
let tempdir = tempfile::tempdir().unwrap();
let client = ClientWithMiddleware::from(Client::new());

(
PackageDb::new(
client,
&[url::Url::parse("https://pypi.org/simple/").unwrap()],
tempdir.path(),
)
.unwrap(),
Arc::new(
PackageDb::new(
client,
&[url::Url::parse("https://pypi.org/simple/").unwrap()],
tempdir.path(),
)
.unwrap(),
),
tempdir,
)
}
Expand Down Expand Up @@ -450,13 +453,12 @@ mod tests {
let sdist = SDist::from_path(&path, &"rich".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -477,13 +479,12 @@ mod tests {
let sdist = SDist::from_path(&path, &"rich".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -503,13 +504,12 @@ mod tests {
let sdist = SDist::from_path(&path, &"rich".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -529,7 +529,7 @@ mod tests {
let sdist = SDist::from_path(&path, &"env_package".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let resolve_options = ResolveOptions {
..Default::default()
};
Expand All @@ -540,10 +540,10 @@ mod tests {
mandatory_env.insert("MY_ENV_VAR".to_string(), "SOME_VALUE".to_string());

let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
resolve_options,
mandatory_env,
)
.unwrap();
Expand All @@ -567,7 +567,7 @@ mod tests {
let sdist = SDist::from_path(&path, &"env_package".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let resolve_options = ResolveOptions {
clean_env: true,
..Default::default()
Expand All @@ -579,10 +579,10 @@ mod tests {
mandatory_env.insert(String::from("MY_ENV_VAR"), String::from("SOME_VALUE"));

let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
resolve_options,
mandatory_env,
)
.unwrap();
Expand All @@ -603,7 +603,7 @@ mod tests {
let sdist = SDist::from_path(&path, &"env_package".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let resolve_options = ResolveOptions {
clean_env: true,
..Default::default()
Expand All @@ -614,10 +614,10 @@ mod tests {
let mandatory_env = HashMap::new();

let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
resolve_options,
mandatory_env,
)
.unwrap();
Expand All @@ -638,13 +638,12 @@ mod tests {
let sdist = SDist::from_path(&path, &"filterpy".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
);

Expand Down Expand Up @@ -703,17 +702,17 @@ mod tests {
let sdist = SDist::from_path(&path, &"setuptools".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let resolve_options = ResolveOptions {
sdist_resolution: SDistResolution::OnlySDists,
..Default::default()
};

let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0,
env_markers,
None,
&resolve_options,
resolve_options,
HashMap::default(),
)
.unwrap();
Expand Down Expand Up @@ -745,13 +744,12 @@ mod tests {
let url = Url::from_file_path(path.canonicalize().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0.clone(),
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -775,13 +773,12 @@ mod tests {
let url = Url::from_file_path(path.canonicalize().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0.clone(),
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -807,13 +804,12 @@ mod tests {
// let sdist = SDist::from_path(&path, &"rich".parse().unwrap()).unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0.clone(),
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -835,13 +831,12 @@ mod tests {
Url::parse("https://github.com/Textualize/rich/archive/refs/tags/v13.7.0.zip").unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0.clone(),
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -855,7 +850,7 @@ mod tests {

let artifact_info = content
.iter()
.flat_map(|(_, artifacts)| artifacts.iter())
.flat_map(|(_, artifacts)| artifacts.iter().cloned())
.collect::<Vec<_>>();

let wheel_metadata = package_db
Expand All @@ -876,13 +871,12 @@ mod tests {
let url = Url::parse("git+https://github.com/Textualize/rich.git").unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0.clone(),
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -904,13 +898,12 @@ mod tests {
let url = Url::parse("git+https://github.com/Textualize/rich.git@v1.0.0").unwrap();

let package_db = get_package_db();
let env_markers = Pep508EnvMakers::from_env().await.unwrap();
let resolve_options = ResolveOptions::default();
let env_markers = Arc::new(Pep508EnvMakers::from_env().await.unwrap().0);
let wheel_builder = WheelBuilder::new(
&package_db.0,
&env_markers,
package_db.0.clone(),
env_markers,
None,
&resolve_options,
ResolveOptions::default(),
HashMap::default(),
)
.unwrap();
Expand All @@ -924,7 +917,7 @@ mod tests {

let artifact_info = content
.iter()
.flat_map(|(_, artifacts)| artifacts.iter())
.flat_map(|(_, artifacts)| artifacts.iter().cloned())
.collect::<Vec<_>>();

let wheel_metadata = package_db
Expand Down
Loading

0 comments on commit fd843c6

Please sign in to comment.