From aca1bd44faa4696c3867c13181b70623d3f2cd95 Mon Sep 17 00:00:00 2001 From: nichmor Date: Tue, 3 Sep 2024 13:13:54 +0300 Subject: [PATCH] fix: offline mapping should not request (#1968) --- Cargo.lock | 50 +++++++------- Cargo.toml | 6 ++ crates/pypi_mapping/src/lib.rs | 23 ++++++- .../src/prefix_pypi_name_mapping.rs | 22 +++++-- src/lock_file/update.rs | 4 +- src/project/mod.rs | 5 ++ tests/common/client.rs | 49 ++++++++++++++ tests/common/mod.rs | 1 + tests/solve_group_tests.rs | 65 ++++++++++++++++++- 9 files changed, 185 insertions(+), 40 deletions(-) create mode 100644 tests/common/client.rs diff --git a/Cargo.lock b/Cargo.lock index 8128d3f9f..8587e3ae6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -331,9 +331,9 @@ checksum = "8b75356056920673b02621b35afd0f7dda9306d03c79a30f5c56c44cf256e3de" [[package]] name = "async-trait" -version = "0.1.81" +version = "0.1.82" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e0c28dcc82d7c8ead5cb13beb15405b57b8546e93215673ff8ca0349a028107" +checksum = "a27b8a3a6e1a44fa4c8baf1f653e4172e81486d4941f2237e20dc2d0cf4ddff1" dependencies = [ "proc-macro2", "quote", @@ -2279,7 +2279,7 @@ dependencies = [ "pypi-types", "reflink-copy", "regex", - "rustc-hash 2.0.0", + "rustc-hash", "serde", "serde_json", "sha2", @@ -2364,7 +2364,7 @@ checksum = "db2b7379a75544c94b3da32821b0bf41f9062e9970e23b78cc577d0d89676d16" dependencies = [ "jiff-tzdb-platform", "serde", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3229,7 +3229,7 @@ dependencies = [ "pep440_rs", "pubgrub", "regex", - "rustc-hash 2.0.0", + "rustc-hash", "schemars", "serde", "smallvec", @@ -3351,6 +3351,7 @@ dependencies = [ "ahash 0.8.11", "assert_matches", "async-once-cell", + "async-trait", "barrier_cell", "cfg-if", "chrono", @@ -3372,6 +3373,7 @@ dependencies = [ "flate2", "fs_extra", "futures", + "http 1.1.0", "http-cache-reqwest", "human_bytes", "humantime", @@ -3646,7 +3648,7 @@ name = "platform-tags" version = "0.0.1" source = "git+https://github.com/astral-sh/uv?tag=0.4.0#d9bd3bc7a536037ea8645fb70f1d35c0bb62b68e" dependencies = [ - "rustc-hash 2.0.0", + "rustc-hash", "serde", "thiserror", ] @@ -3770,7 +3772,7 @@ dependencies = [ "indexmap 2.3.0", "log", "priority-queue", - "rustc-hash 1.1.0", + "rustc-hash", "thiserror", ] @@ -3881,7 +3883,7 @@ dependencies = [ "pin-project-lite", "quinn-proto", "quinn-udp", - "rustc-hash 2.0.0", + "rustc-hash", "rustls 0.23.12", "socket2", "thiserror", @@ -3898,7 +3900,7 @@ dependencies = [ "bytes", "rand", "ring", - "rustc-hash 2.0.0", + "rustc-hash", "rustls 0.23.12", "slab", "thiserror", @@ -4782,12 +4784,6 @@ version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" -[[package]] -name = "rustc-hash" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" - [[package]] name = "rustc-hash" version = "2.0.0" @@ -6150,7 +6146,7 @@ dependencies = [ "reqwest 0.12.5", "reqwest-middleware", "rust-netrc", - "rustc-hash 2.0.0", + "rustc-hash", "tokio", "tracing", "url", @@ -6171,7 +6167,7 @@ dependencies = [ "pep508_rs", "pypi-types", "regex", - "rustc-hash 2.0.0", + "rustc-hash", "serde", "serde_json", "tempfile", @@ -6199,7 +6195,7 @@ dependencies = [ "nanoid", "pypi-types", "rmp-serde", - "rustc-hash 2.0.0", + "rustc-hash", "serde", "tempfile", "tracing", @@ -6266,7 +6262,7 @@ dependencies = [ "pep508_rs", "platform-tags", "pypi-types", - "rustc-hash 2.0.0", + "rustc-hash", "serde", "serde_json", "thiserror", @@ -6288,7 +6284,7 @@ dependencies = [ "install-wheel-rs", "itertools 0.13.0", "pypi-types", - "rustc-hash 2.0.0", + "rustc-hash", "tracing", "uv-build", "uv-cache", @@ -6321,7 +6317,7 @@ dependencies = [ "reqwest 0.12.5", "reqwest-middleware", "rmp-serde", - "rustc-hash 2.0.0", + "rustc-hash", "serde", "tempfile", "thiserror", @@ -6356,7 +6352,7 @@ dependencies = [ "pypi-types", "rayon", "reqwest 0.12.5", - "rustc-hash 2.0.0", + "rustc-hash", "sha2", "thiserror", "tokio", @@ -6427,7 +6423,7 @@ dependencies = [ "platform-tags", "pypi-types", "rayon", - "rustc-hash 2.0.0", + "rustc-hash", "same-file", "tempfile", "thiserror", @@ -6558,7 +6554,7 @@ dependencies = [ "pypi-types", "requirements-txt", "rkyv", - "rustc-hash 2.0.0", + "rustc-hash", "same-file", "serde", "textwrap", @@ -6604,7 +6600,7 @@ dependencies = [ "pep440_rs", "pep508_rs", "pypi-types", - "rustc-hash 2.0.0", + "rustc-hash", "thiserror", "url", "uv-cache", @@ -6643,7 +6639,7 @@ source = "git+https://github.com/astral-sh/uv?tag=0.4.0#d9bd3bc7a536037ea8645fb7 dependencies = [ "anstream", "owo-colors", - "rustc-hash 2.0.0", + "rustc-hash", ] [[package]] @@ -6657,7 +6653,7 @@ dependencies = [ "pep440_rs", "pep508_rs", "pypi-types", - "rustc-hash 2.0.0", + "rustc-hash", "serde", "thiserror", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 7273e7501..2d325686d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ repository = "https://github.com/prefix-dev/pixi" ahash = "0.8.11" assert_matches = "1.5.0" async-once-cell = "0.5.3" +async-trait = "0.1.82" cfg-if = "1.0" chrono = "0.4.38" clap = { version = "4.5.9", default-features = false } @@ -33,6 +34,7 @@ fd-lock = "4.0.2" flate2 = "1.0.28" fs_extra = "1.3.0" futures = "0.3.30" +http = "1.1.0" http-cache-reqwest = "0.14.0" human_bytes = "0.4.3" humantime = "2.1.0" @@ -284,6 +286,7 @@ uv-types = { workspace = true } xxhash-rust = { workspace = true } zip = { workspace = true, features = ["deflate", "time"] } + [target.'cfg(unix)'.dependencies] libc = { workspace = true, default-features = false } nix = { workspace = true, features = ["fs", "signal", "term", "poll"] } @@ -306,12 +309,15 @@ strip = false [dev-dependencies] +async-trait = { workspace = true } +http = { workspace = true } insta = { workspace = true, features = ["yaml", "glob"] } rstest = { workspace = true } serde_json = { workspace = true } serial_test = { workspace = true } tokio = { workspace = true, features = ["rt"] } + [patch.crates-io] # For pyproject-toml # pyproject-toml = { git = "https://github.com/tdejager/pyproject-toml-rs", branch = "feat/bump-508-440" } diff --git a/crates/pypi_mapping/src/lib.rs b/crates/pypi_mapping/src/lib.rs index b044dc28d..7d27e6f64 100644 --- a/crates/pypi_mapping/src/lib.rs +++ b/crates/pypi_mapping/src/lib.rs @@ -1,4 +1,9 @@ -use std::{collections::HashMap, path::PathBuf, str::FromStr, sync::Arc}; +use std::{ + collections::{BTreeSet, HashMap}, + path::PathBuf, + str::FromStr, + sync::Arc, +}; use async_once_cell::OnceCell as AsyncCell; use custom_pypi_mapping::fetch_mapping_from_path; @@ -96,6 +101,7 @@ impl CustomMapping { pub enum MappingSource { Custom(Arc), Prefix, + Disabled, } impl MappingSource { @@ -130,7 +136,7 @@ impl PurlSource { } pub async fn amend_pypi_purls( - client: reqwest::Client, + client: ClientWithMiddleware, mapping_source: &MappingSource, conda_packages: &mut [RepoDataRecord], reporter: Option>, @@ -148,7 +154,7 @@ pub async fn amend_pypi_purls( options: HttpCacheOptions::default(), }); - let client = ClientBuilder::new(client) + let client = ClientBuilder::from_client(client) .with(cache_strategy) .with(retry_strategy) .build(); @@ -161,6 +167,17 @@ pub async fn amend_pypi_purls( MappingSource::Prefix => { prefix_pypi_name_mapping::amend_pypi_purls(&client, conda_packages, reporter).await?; } + MappingSource::Disabled => { + for record in conda_packages.iter_mut() { + if let Some(purl) = prefix_pypi_name_mapping::assume_conda_is_pypi(None, record) { + record + .package_record + .purls + .get_or_insert_with(BTreeSet::new) + .insert(purl); + } + } + } } Ok(()) diff --git a/crates/pypi_mapping/src/prefix_pypi_name_mapping.rs b/crates/pypi_mapping/src/prefix_pypi_name_mapping.rs index 91fd8ca79..b36ecc350 100644 --- a/crates/pypi_mapping/src/prefix_pypi_name_mapping.rs +++ b/crates/pypi_mapping/src/prefix_pypi_name_mapping.rs @@ -236,12 +236,8 @@ pub fn amend_pypi_purls_for_record( // package is not in our mapping yet // so we assume that it is the same as the one from conda-forge - if purls.is_none() && is_conda_forge_record(record) { - // Convert the conda package names to pypi package names. If the conversion - // fails we just assume that its not a valid python package. - if let Some(purl) = build_pypi_purl_from_package_record(&record.package_record) { - purls.get_or_insert_with(Vec::new).push(purl); - } + if let Some(purl) = assume_conda_is_pypi(purls.as_ref(), record) { + purls.get_or_insert_with(Vec::new).push(purl); } // If we have found some purls we overwrite whatever was there before. @@ -251,3 +247,17 @@ pub fn amend_pypi_purls_for_record( Ok(()) } + +/// Try to assume that the conda-forge package is a PyPI package and return a purl. +pub fn assume_conda_is_pypi( + purls: Option<&Vec>, + record: &RepoDataRecord, +) -> Option { + if purls.is_none() && is_conda_forge_record(record) { + // Convert the conda package names to pypi package names. If the conversion + // fails we just assume that its not a valid python package. + build_pypi_purl_from_package_record(&record.package_record) + } else { + None + } +} diff --git a/src/lock_file/update.rs b/src/lock_file/update.rs index 84c68a6ae..4fede01ae 100644 --- a/src/lock_file/update.rs +++ b/src/lock_file/update.rs @@ -1569,7 +1569,7 @@ async fn spawn_solve_conda_environment_task( if has_pypi_dependencies { pb.set_message("extracting pypi packages"); pypi_mapping::amend_pypi_purls( - client, + client.into(), &pypi_name_mapping_location, &mut records, Some(pb.purl_amend_reporter()), @@ -1803,7 +1803,7 @@ async fn spawn_solve_pypi_task( let locked_pypi_records = locked_pypi_packages.records.clone(); pypi_mapping::amend_pypi_purls( - environment.project().client().clone(), + environment.project().client().clone().into(), pypi_name_mapping_location, &mut conda_records, None, diff --git a/src/project/mod.rs b/src/project/mod.rs index 7b36bce6c..336f7097c 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -521,6 +521,11 @@ impl Project { }) .collect::>>()?; + // User can disable the mapping by providing an empty map + if channel_to_location_map.is_empty() { + return Ok(MappingSource::Disabled); + } + let project_channels: HashSet<_> = manifest .parsed .project diff --git a/tests/common/client.rs b/tests/common/client.rs new file mode 100644 index 000000000..f49ed3328 --- /dev/null +++ b/tests/common/client.rs @@ -0,0 +1,49 @@ +/// Inspired from uv: https://github.com/astral-sh/uv/blob/ccdf2d793bbc2401c891b799772f615a28607e79/crates/uv-client/src/middleware.rs#L33 +/// and used to verify that we don't do any requests during the tests. +use http::Extensions; +use std::fmt::Debug; + +use reqwest::{Request, Response}; +use reqwest_middleware::{Middleware, Next}; +use url::Url; + +/// A custom error type for the offline middleware. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct OfflineError { + url: Url, +} + +impl OfflineError { + /// Returns the URL that caused the error. + pub(crate) fn url(&self) -> &Url { + &self.url + } +} + +impl std::fmt::Display for OfflineError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Network connectivity is disabled for `{}`", self.url) + } +} + +impl std::error::Error for OfflineError {} + +/// A middleware that always returns an error indicating that the client is offline. +pub(crate) struct OfflineMiddleware; + +#[async_trait::async_trait] +impl Middleware for OfflineMiddleware { + async fn handle( + &self, + req: Request, + _extensions: &mut Extensions, + _next: Next<'_>, + ) -> reqwest_middleware::Result { + Err(reqwest_middleware::Error::Middleware( + OfflineError { + url: req.url().clone(), + } + .into(), + )) + } +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 211b86413..a506aeade 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1,6 +1,7 @@ #![allow(dead_code)] pub mod builders; +pub mod client; pub mod package_database; use std::{ diff --git a/tests/solve_group_tests.rs b/tests/solve_group_tests.rs index 9e1e93fca..621cbc345 100644 --- a/tests/solve_group_tests.rs +++ b/tests/solve_group_tests.rs @@ -6,12 +6,14 @@ use std::{ use pypi_mapping::{self, PurlSource}; use rattler_conda_types::{PackageName, Platform, RepoDataRecord}; use rattler_lock::DEFAULT_ENVIRONMENT_NAME; +use reqwest_middleware::ClientBuilder; use serial_test::serial; use tempfile::TempDir; use url::Url; use crate::common::{ builders::{HasDependencyConfig, HasPrefixUpdateConfig}, + client::OfflineMiddleware, package_database::{Package, PackageDatabase}, LockFileExt, PixiControl, }; @@ -361,7 +363,7 @@ async fn test_dont_record_not_present_package_as_purl() { .unwrap(); // we verify that even if this name is not present in our mapping - // we anyway record a purl because we make an assumption + // we record a purl anyways. Because we make the assumption // that it's a pypi package assert_eq!(first_purl.name(), "pixi-something-new-for-test"); @@ -461,7 +463,7 @@ async fn test_we_record_not_present_package_as_purl_for_custom_mapping() { .unwrap(); // we verify that even if this name is not present in our mapping - // we anyway record a purl because we make an assumption + // we record a purl anyways. Because we make the assumption // that it's a pypi package assert_eq!(first_purl.name(), "pixi-something-new"); assert!(first_purl.qualifiers().is_empty()); @@ -692,3 +694,62 @@ async fn test_file_url_as_mapping_location() { PurlSource::ProjectDefinedMapping.as_str() ); } + +#[tokio::test] +async fn test_disabled_mapping() { + let pixi = PixiControl::from_manifest( + r#" + [project] + name = "test-channel-change" + channels = ["conda-forge"] + platforms = ["linux-64"] + conda-pypi-map = { } + "#, + ) + .unwrap(); + + let project = pixi.project().unwrap(); + + let client = project.authenticated_client(); + + let blocking_middleware = OfflineMiddleware; + + let blocked_client = ClientBuilder::from_client(client.clone()) + .with(blocking_middleware) + .build(); + + let boltons_package = Package::build("boltons", "2").finish(); + + let boltons_repo_data_record = RepoDataRecord { + package_record: boltons_package.package_record, + file_name: "boltons".to_owned(), + url: Url::parse("https://pypi.org/simple/boltons/").unwrap(), + channel: "https://conda.anaconda.org/conda-forge/".to_owned(), + }; + + let mut packages = vec![boltons_repo_data_record]; + + pypi_mapping::amend_pypi_purls( + blocked_client, + project.pypi_name_mapping_source().unwrap(), + &mut packages, + None, + ) + .await + .unwrap(); + + let boltons_package = packages.pop().unwrap(); + + let boltons_first_purl = boltons_package + .package_record + .purls + .as_ref() + .and_then(BTreeSet::first) + .unwrap(); + + // we verify that even if this name is not present in our mapping + // we record a purl anyways. Because we make the assumption + // that it's a pypi package + assert_eq!(boltons_first_purl.name(), "boltons"); + assert!(boltons_first_purl.qualifiers().is_empty()); +}