diff --git a/.changeset/python-registry-package-routing.md b/.changeset/python-registry-package-routing.md new file mode 100644 index 0000000000..a30963cf3a --- /dev/null +++ b/.changeset/python-registry-package-routing.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +Python `registries` entries now route packages by exact names or trailing-prefix patterns in `packages`. Registry declaration order no longer affects resolution. A matched package resolves exclusively from its assigned registry, including transitive and build dependencies. Use `packages: ["*"]` to declare the default index. diff --git a/Cargo.lock b/Cargo.lock index 246126a122..f8b3232b17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4782,6 +4782,7 @@ dependencies = [ "pnpm-testing-utils", "pnpm-versioning", "pnpm-workspace-state", + "pnpr-registry", "pretty_assertions", "serde", "serde-saphyr", diff --git a/pnpm/crates/cli/src/cargo_deps/tests.rs b/pnpm/crates/cli/src/cargo_deps/tests.rs index cd29623064..e700c1167b 100644 --- a/pnpm/crates/cli/src/cargo_deps/tests.rs +++ b/pnpm/crates/cli/src/cargo_deps/tests.rs @@ -366,7 +366,10 @@ fn maps_crate_names_to_sparse_index_paths() { fn config_with_cargo_credentials(index_url: &str) -> Config { let mut config = Config::new(); - config.indexes_by_ecosystem.insert(pnpm_config::Ecosystem::Cargo, vec![index_url.to_string()]); + config.indexes_by_ecosystem.insert( + pnpm_config::Ecosystem::Cargo, + vec![index_url.to_string().into()], + ); config.auth_headers = Arc::new(AuthHeaders::from_creds_map([ ("//registry.example.test/".to_string(), "Bearer crate-token".to_string()), ("//cdn.example.test/".to_string(), "Bearer unrelated-token".to_string()), diff --git a/pnpm/crates/cli/src/cargo_deps/tests/pnpr.rs b/pnpm/crates/cli/src/cargo_deps/tests/pnpr.rs index 89a84d3aab..ad77922de0 100644 --- a/pnpm/crates/cli/src/cargo_deps/tests/pnpr.rs +++ b/pnpm/crates/cli/src/cargo_deps/tests/pnpr.rs @@ -77,7 +77,7 @@ async fn configured_cargo_registry_is_sent_to_the_pnpr_server() { let mut config = config_for_pnpr(&server.url()); config.indexes_by_ecosystem.insert( pnpm_config::Ecosystem::Cargo, - vec!["https://registry.example.test/index/".to_string()], + vec!["https://registry.example.test/index/".to_string().into()], ); let lockfile = resolve_via_pnpr(&config, r#"{"packages":[],"workspace_members":[]}"#) diff --git a/pnpm/crates/cli/tests/suite/cargo_git_install.rs b/pnpm/crates/cli/tests/suite/cargo_git_install.rs index dbbff79ef6..e7199e80fa 100644 --- a/pnpm/crates/cli/tests/suite/cargo_git_install.rs +++ b/pnpm/crates/cli/tests/suite/cargo_git_install.rs @@ -1,7 +1,7 @@ use super::cargo_install::{cargo_workspace, crate_archive, install_in}; use assert_cmd::prelude::*; use pnpm_cargo_resolver::CRATES_IO_SPARSE_INDEX; -use pnpm_testing_utils::git_repo::GitRepoFixture; +use pnpm_testing_utils::{diagnostics::assert_diagnostic_contains, git_repo::GitRepoFixture}; use sha2::{Digest, Sha256}; use std::{fs, process::Command}; use tempfile::TempDir; @@ -643,7 +643,10 @@ fn submodule_fetching_preserves_the_callers_transport_allowlist() { eprintln!("A file-only caller must reject an HTTP submodule: {output:?}"); assert!(!output.status.success()); - assert!(String::from_utf8_lossy(&output.stderr).contains("transport 'http' not allowed")); + assert_diagnostic_contains( + &String::from_utf8_lossy(&output.stderr), + "transport 'http' not allowed", + ); assert!( !root .path() diff --git a/pnpm/crates/cli/tests/suite/hooks.rs b/pnpm/crates/cli/tests/suite/hooks.rs index f75bee1f17..d676135a8f 100644 --- a/pnpm/crates/cli/tests/suite/hooks.rs +++ b/pnpm/crates/cli/tests/suite/hooks.rs @@ -129,6 +129,7 @@ fn update_config_catalog_applies_to_import() { .expect("write package-lock.json"); pacquet_in(&workspace) + .with_env("PNPM_CONFIG_NPMRC_AUTH_FILE", workspace.join(".npmrc")) .with_arg("import") .assert() .success(); @@ -159,6 +160,7 @@ fn update_config_catalog_applies_to_import() { "the imported lockfile should resolve the hook-provided catalog entry:\n{lockfile}", ); pacquet_in(&workspace) + .with_env("PNPM_CONFIG_NPMRC_AUTH_FILE", workspace.join(".npmrc")) .with_args(["install", "--frozen-lockfile"]) .assert() .success(); diff --git a/pnpm/crates/cli/tests/suite/pack.rs b/pnpm/crates/cli/tests/suite/pack.rs index 0f63bce0e0..3413b59d8d 100644 --- a/pnpm/crates/cli/tests/suite/pack.rs +++ b/pnpm/crates/cli/tests/suite/pack.rs @@ -149,6 +149,7 @@ fn pack_installs_config_dependencies_before_loading_hooks() { fs::write(workspace_yaml, settings).expect("write configDependencies"); pacquet + .with_env("PNPM_CONFIG_NPMRC_AUTH_FILE", workspace.join(".npmrc")) .with_arg("pack") .assert() .success(); diff --git a/pnpm/crates/cli/tests/suite/python.rs b/pnpm/crates/cli/tests/suite/python.rs index 27272df091..0e37367651 100644 --- a/pnpm/crates/cli/tests/suite/python.rs +++ b/pnpm/crates/cli/tests/suite/python.rs @@ -178,13 +178,16 @@ async fn serve_wheels( mocks } -fn add_python_index_searched_first(root: &Path, index: &str) { +fn add_python_registry(root: &Path, index: &str, packages: &[&str]) { let workspace = fs::read_to_string(root.join("pnpm-workspace.yaml")).unwrap(); fs::write( root.join("pnpm-workspace.yaml"), workspace.replace( "registries:\n", - &format!("registries:\n '{index}':\n ecosystem: pypi\n"), + &format!( + "registries:\n '{index}':\n ecosystem: pypi\n packages: {}\n", + serde_json::to_string(packages).unwrap(), + ), ), ) .unwrap(); diff --git a/pnpm/crates/cli/tests/suite/python/indexes.rs b/pnpm/crates/cli/tests/suite/python/indexes.rs index 4bb59748b4..c096910f0f 100644 --- a/pnpm/crates/cli/tests/suite/python/indexes.rs +++ b/pnpm/crates/cli/tests/suite/python/indexes.rs @@ -1,15 +1,15 @@ mod rules; use super::{ - add_python_index_searched_first, add_python_settings, assert_failure_contains, pacquet_in, - project, python, serve, serve_with_index_auth, wheel, + TINY_BACKEND, add_python_registry, add_python_settings, assert_failure_contains, pacquet_in, + project, python, python_project, serve, serve_with_index_auth, wheel, }; use assert_cmd::assert::OutputAssertExt; use base64::{Engine as _, engine::general_purpose::STANDARD}; use std::fs; #[tokio::test] -async fn extra_indexes_have_priority_and_cache_missing_packages_for_offline_resolution() { +async fn package_routes_cover_transitive_dependencies_and_offline_resolution() { let root = tempfile::tempdir().unwrap(); let mut primary = mockito::Server::new_async().await; let mut extra = mockito::Server::new_async().await; @@ -25,15 +25,20 @@ async fn extra_indexes_have_priority_and_cache_missing_packages_for_offline_reso .await; let _primary_beta = serve(&mut primary, "beta", &[("1.0", wheel("beta", "1.0", "", &[]))]).await; - let missing = extra + let unused = extra .mock("GET", "/simple/beta/") - .match_header("authorization", authorization.as_str()) - .with_status(404) - .expect(1) + .expect(0) .create_async() .await; project(root.path(), &primary.url(), &["alpha"]); - add_python_index_searched_first(root.path(), &format!("{}/simple/", extra.url())); + let workspace = root.path().join("pnpm-workspace.yaml"); + let yaml = fs::read_to_string(&workspace).unwrap(); + fs::write( + &workspace, + yaml.replace(" ecosystem: pypi\n", " ecosystem: pypi\n packages: ['*']\n"), + ) + .unwrap(); + add_python_registry(root.path(), &format!("{}/simple/", extra.url()), &["alpha"]); write_index_credentials(root.path(), &extra.url(), "extra-user:extra-secret"); pacquet_in(root.path()) .arg("install") @@ -45,14 +50,14 @@ async fn extra_indexes_have_priority_and_cache_missing_packages_for_offline_reso .success(); let lock = fs::read_to_string(root.path().join("pylock.toml")).unwrap(); eprintln!("{lock}"); - assert!(lock.contains("extra-indexes")); + assert!(lock.contains("registry-packages")); assert!(!lock.contains("extra-secret")); fs::remove_file(root.path().join("pylock.toml")).unwrap(); pacquet_in(root.path()) .args(["install", "--offline"]) .assert() .success(); - missing.assert_async().await; + unused.assert_async().await; add_python_settings(root.path(), " constraints: ['alpha>=2']\n"); assert_failure_contains( pacquet_in(root.path()).args(["install", "--frozen-lockfile"]), @@ -65,7 +70,7 @@ async fn extra_indexes_have_priority_and_cache_missing_packages_for_offline_reso } #[tokio::test] -async fn extra_index_errors_do_not_fall_back_to_another_index() { +async fn selected_registry_errors_do_not_fall_back_to_another_index() { let root = tempfile::tempdir().unwrap(); let mut primary = mockito::Server::new_async().await; let mut extra = mockito::Server::new_async().await; @@ -80,7 +85,7 @@ async fn extra_index_errors_do_not_fall_back_to_another_index() { .create_async() .await; project(root.path(), &primary.url(), &["alpha"]); - add_python_index_searched_first(root.path(), &format!("{}/simple/", extra.url())); + add_python_registry(root.path(), &format!("{}/simple/", extra.url()), &["alpha"]); assert_failure_contains(pacquet_in(root.path()).arg("install"), "403 Forbidden"); unused.assert_async().await; denied.assert_async().await; @@ -99,37 +104,18 @@ async fn an_index_credential_does_not_travel_to_an_index_on_another_origin() { Some(&authorization), ) .await; - // Every lookup reaches the extra index first, and none of them may carry - // the credential configured for the other origin. - let mut anonymous = Vec::new(); - for distribution in ["alpha", "beta"] { - anonymous.push( - extra - .mock("GET", format!("/simple/{distribution}/").as_str()) - .match_header("authorization", mockito::Matcher::Missing) - .with_status(404) - .expect(1) - .create_async() - .await, - ); - } - let _beta = serve_with_index_auth( - &mut primary, - "beta", - &[("1.0", wheel("beta", "1.0", "", &[]))], - Some(&authorization), - ) - .await; + let beta = serve(&mut extra, "beta", &[("1.0", wheel("beta", "1.0", "", &[]))]).await; project(root.path(), &primary.url(), &["alpha"]); - add_python_index_searched_first(root.path(), &format!("{}/simple/", extra.url())); + add_python_registry(root.path(), &format!("{}/simple/", extra.url()), &["beta"]); write_index_credentials(root.path(), &primary.url(), "parent:secret"); pacquet_in(root.path()) .arg("install") .assert() .success(); - for mock in anonymous { - mock.assert_async().await; - } + beta.last() + .unwrap() + .assert_async() + .await; } #[tokio::test] @@ -140,13 +126,13 @@ async fn authenticated_index_caches_do_not_cross_credential_identities() { let _alpha = serve(&mut primary, "alpha", &[("1.0", wheel("alpha", "1.0", "", &[]))]).await; let alice = format!("Basic {}", STANDARD.encode("user:alice-secret")); let bob = format!("Basic {}", STANDARD.encode("user:bob-secret")); - let missing = extra - .mock("GET", "/simple/alpha/") - .match_header("authorization", alice.as_str()) - .with_status(404) - .expect(1) - .create_async() - .await; + let _alice_alpha = serve_with_index_auth( + &mut extra, + "alpha", + &[("1.0", wheel("alpha", "1.0", "", &[]))], + Some(&alice), + ) + .await; let _bob_alpha = serve_with_index_auth( &mut extra, "alpha", @@ -155,7 +141,7 @@ async fn authenticated_index_caches_do_not_cross_credential_identities() { ) .await; project(root.path(), &primary.url(), &["alpha"]); - add_python_index_searched_first(root.path(), &format!("{}/simple/", extra.url())); + add_python_registry(root.path(), &format!("{}/simple/", extra.url()), &["alpha"]); write_index_credentials(root.path(), &extra.url(), "user:alice-secret"); pacquet_in(root.path()) .arg("install") @@ -191,14 +177,13 @@ async fn authenticated_index_caches_do_not_cross_credential_identities() { .args(["-c", "import alpha; assert alpha.VERSION == '1.0'"]) .assert() .success(); - missing.assert_async().await; } #[test] fn an_index_may_not_carry_its_own_credentials() { let root = tempfile::tempdir().unwrap(); project(root.path(), "http://localhost:1", &["alpha"]); - add_python_index_searched_first(root.path(), "http://alice:private-secret@localhost:1/simple/"); + add_python_registry(root.path(), "http://alice:private-secret@localhost:1/simple/", &["alpha"]); let output = pacquet_in(root.path()) .arg("install") .assert() @@ -226,3 +211,112 @@ fn write_index_credentials(root: &std::path::Path, index: &str, user_and_passwor ) .unwrap(); } + +#[tokio::test] +async fn missing_private_packages_never_fall_back_to_the_default_index() { + for transitive in [false, true] { + let root = tempfile::tempdir().unwrap(); + let mut public = mockito::Server::new_async().await; + let mut private = mockito::Server::new_async().await; + let _alpha = serve( + &mut public, + "alpha", + &[("1.0", wheel("alpha", "1.0", "Requires-Dist: beta\n", &[]))], + ) + .await; + let unused = public + .mock("GET", "/simple/beta/") + .expect(0) + .create_async() + .await; + let missing = private + .mock("GET", "/simple/beta/") + .with_status(404) + .expect(1) + .create_async() + .await; + project(root.path(), &public.url(), &[if transitive { "alpha" } else { "beta" }]); + add_python_registry(root.path(), &format!("{}/simple/", private.url()), &["beta"]); + assert_failure_contains( + pacquet_in(root.path()).arg("install"), + "Python dependency resolution failed", + ); + assert_failure_contains( + pacquet_in(root.path()).args(["install", "--offline"]), + "Python dependency resolution failed", + ); + missing.assert_async().await; + unused.assert_async().await; + } +} + +#[tokio::test] +async fn incompatible_private_versions_never_fall_back_to_the_default_index() { + let root = tempfile::tempdir().unwrap(); + let mut public = mockito::Server::new_async().await; + let mut private = mockito::Server::new_async().await; + let unused = public + .mock("GET", "/simple/alpha/") + .expect(0) + .create_async() + .await; + let _private = serve(&mut private, "alpha", &[("1.0", wheel("alpha", "1.0", "", &[]))]).await; + project(root.path(), &public.url(), &["alpha>=2"]); + add_python_registry(root.path(), &format!("{}/simple/", private.url()), &["alpha"]); + assert_failure_contains( + pacquet_in(root.path()).arg("install"), + "Python dependency resolution failed", + ); + unused.assert_async().await; +} + +#[tokio::test] +async fn package_route_changes_invalidate_frozen_lockfiles() { + let root = tempfile::tempdir().unwrap(); + let public = mockito::Server::new_async().await; + let mut private = mockito::Server::new_async().await; + let _private = serve(&mut private, "alpha", &[("1.0", wheel("alpha", "1.0", "", &[]))]).await; + project(root.path(), &public.url(), &["alpha"]); + add_python_registry(root.path(), &format!("{}/simple/", private.url()), &["alpha"]); + pacquet_in(root.path()) + .arg("install") + .assert() + .success(); + let path = root.path().join("pnpm-workspace.yaml"); + let yaml = fs::read_to_string(&path).unwrap(); + fs::write(path, yaml.replace(r#"packages: ["alpha"]"#, r#"packages: ["beta"]"#)).unwrap(); + assert_failure_contains( + pacquet_in(root.path()).args(["install", "--frozen-lockfile"]), + "Python index changed", + ); +} + +#[tokio::test] +async fn isolated_build_dependencies_use_registry_claims() { + let root = tempfile::tempdir().unwrap(); + let mut public = mockito::Server::new_async().await; + let mut private = mockito::Server::new_async().await; + let unused = public + .mock("GET", "/simple/tinybackend/") + .expect(0) + .create_async() + .await; + let _backend = serve( + &mut private, + "tinybackend", + &[("80.0", wheel("tinybackend", "80.0", "", &[("tinybuild.py", TINY_BACKEND)]))], + ) + .await; + project(root.path(), &public.url(), &[]); + python_project(root.path(), "app", "dependencies = []"); + add_python_registry(root.path(), &format!("{}/simple/", private.url()), &["tinybackend"]); + pacquet_in(root.path()) + .arg("install") + .assert() + .success(); + python(root.path()) + .args(["-c", "import app"]) + .assert() + .success(); + unused.assert_async().await; +} diff --git a/pnpm/crates/config/Cargo.toml b/pnpm/crates/config/Cargo.toml index 147d29e9a4..738c7f5ce1 100644 --- a/pnpm/crates/config/Cargo.toml +++ b/pnpm/crates/config/Cargo.toml @@ -11,6 +11,7 @@ license.workspace = true repository.workspace = true [dependencies] +pnpr-registry = { workspace = true } pnpm-matcher = { workspace = true } pnpm-catalogs-types = { workspace = true } pnpm-config-dir = { workspace = true } diff --git a/pnpm/crates/config/src/lib.rs b/pnpm/crates/config/src/lib.rs index f93a5f2374..9e72933ec5 100644 --- a/pnpm/crates/config/src/lib.rs +++ b/pnpm/crates/config/src/lib.rs @@ -42,7 +42,10 @@ pub use workspace_yaml::{ ToolSettings, UnrecognizedTaskSettings, UpdateConfig, UpdateSettings, WORKSPACE_MANIFEST_FILENAME, WorkspaceKeyIssues, WorkspaceSettings, decided_allow_builds, package_configs::{self, PackageConfigsSetting, ProjectConfig, ProjectConfigMultiMatch}, - registries::{self, Ecosystem, RegistryDeclaration, RegistryEntry, RegistryLookups}, + registries::{ + self, Ecosystem, EcosystemIndex, PythonRegistryRoute, RegistryDeclaration, RegistryEntry, + RegistryLookups, + }, workspace_root_or, }; diff --git a/pnpm/crates/config/src/registry_options.rs b/pnpm/crates/config/src/registry_options.rs index 1bcee93347..6f0d7169f6 100644 --- a/pnpm/crates/config/src/registry_options.rs +++ b/pnpm/crates/config/src/registry_options.rs @@ -51,24 +51,35 @@ impl Config { registries::to_resolved_declarations(&self.resolved_registry_lookups()) } - /// The `PyPI` indexes to resolve Python packages from, in the order they - /// are searched. - /// - /// The first index that has a distribution supplies it, so the one - /// declared last answers what none before it had. - /// - /// The `registries` entries that name `ecosystem: pypi`, in declaration - /// order, or [`DEFAULT_PYPI_INDEX_URL`] when none do. + /// Python registries and their exclusive package routes, sorted by URL. + /// No declarations selects `PyPI`. Declared restricted registries do not + /// implicitly enable a public default. + #[must_use] + pub fn python_registry_indexes(&self) -> Vec { + let mut indexes = self.indexes_by_ecosystem + .get(&Ecosystem::Pypi) + .filter(|indexes| !indexes.is_empty()) + .cloned() + .unwrap_or_else(|| vec![DEFAULT_PYPI_INDEX_URL.to_string().into()]); + indexes.sort_by(|a, b| a.url.cmp(&b.url)); + indexes + } + + /// All Python index URLs, in canonical order independent of declarations. #[must_use] pub fn python_indexes(&self) -> Vec<&str> { - let declared = self.indexes_by_ecosystem.get(&Ecosystem::Pypi); - match declared.filter(|indexes| !indexes.is_empty()) { - Some(indexes) => indexes - .iter() - .map(String::as_str) - .collect(), - None => vec![DEFAULT_PYPI_INDEX_URL], - } + let Some(indexes) = self.indexes_by_ecosystem + .get(&Ecosystem::Pypi) + .filter(|indexes| !indexes.is_empty()) + else { + return vec![DEFAULT_PYPI_INDEX_URL]; + }; + let mut urls: Vec<_> = indexes + .iter() + .map(|index| index.url.as_str()) + .collect(); + urls.sort_unstable(); + urls } /// The sparse index to resolve Cargo dependencies from. @@ -81,7 +92,7 @@ impl Config { self.indexes_by_ecosystem .get(&Ecosystem::Cargo) .and_then(|indexes| indexes.first()) - .map_or(DEFAULT_CARGO_INDEX_URL, String::as_str) + .map_or(DEFAULT_CARGO_INDEX_URL, |index| index.url.as_str()) } /// The scope and prefix routes the CLI resolves a package's registry diff --git a/pnpm/crates/config/src/settings.rs b/pnpm/crates/config/src/settings.rs index 52378474b8..8fef1a8a89 100644 --- a/pnpm/crates/config/src/settings.rs +++ b/pnpm/crates/config/src/settings.rs @@ -621,16 +621,9 @@ pub struct Config { /// The `registries` setting. pub registry_options_by_url: BTreeMap, - /// The indexes each non-npm ecosystem resolves from, in the order the - /// configuration declares them, which is the order they are searched. - /// From the `registries` entries that name an `ecosystem`. - /// npm is absent: its registries are the three lookups above, which - /// carry the scope and prefix routing npm packages are addressed by. - /// - /// Read through [`Config::python_indexes`] and - /// [`Config::cargo_index_url`], which answer with the ecosystem's - /// default index when the configuration names none. - pub indexes_by_ecosystem: BTreeMap>, + /// The indexes and exclusive package routes declared for non-npm ecosystems. + /// Read through `python_indexes` and `cargo_index_url`. + pub indexes_by_ecosystem: BTreeMap>, /// Resolved proxy configuration — `https-proxy`, `http-proxy`, and /// `no-proxy` (plus the legacy `proxy` key and env-var fallbacks), diff --git a/pnpm/crates/config/src/workspace_yaml/apply.rs b/pnpm/crates/config/src/workspace_yaml/apply.rs index 0b1f39a6d3..bb03bb1581 100644 --- a/pnpm/crates/config/src/workspace_yaml/apply.rs +++ b/pnpm/crates/config/src/workspace_yaml/apply.rs @@ -194,9 +194,6 @@ impl WorkspaceSettings { config.registries_by_scope.extend(lookups.registries_by_scope); config.registries_by_prefix.extend(lookups.registries_by_prefix); config.registry_options_by_url.extend(lookups.registry_options_by_url); - // Replaced per ecosystem rather than appended to: the order is the - // search order, and a layer that names an ecosystem's indexes is - // naming that whole order, not adding to someone else's. config.indexes_by_ecosystem.extend(lookups.indexes_by_ecosystem); declared_prefixes } diff --git a/pnpm/crates/config/src/workspace_yaml/error.rs b/pnpm/crates/config/src/workspace_yaml/error.rs index 5ea412db89..e485484089 100644 --- a/pnpm/crates/config/src/workspace_yaml/error.rs +++ b/pnpm/crates/config/src/workspace_yaml/error.rs @@ -97,6 +97,12 @@ pub enum LoadWorkspaceYamlError { help("pnpm resolves Cargo dependencies from one sparse index. Declare the one to use.") )] CargoIndexDeclaredTwice { registries: String }, + #[display("Invalid Python package routes for {registry:?}: {reason}")] + #[diagnostic(code(ERR_PNPM_INVALID_SETTING))] + InvalidPythonRegistryPackages { registry: String, reason: String }, + #[display("The Python package pattern {pattern:?} is routed to two registries: {registries}")] + #[diagnostic(code(ERR_PNPM_INVALID_SETTING))] + PythonPackageRoutedTwice { pattern: String, registries: String }, #[display("The \"pipelines['{pipeline}']\" setting contains an entry with no task name")] #[diagnostic(code(ERR_PNPM_INVALID_SETTING))] EmptyPipelineTaskName { pipeline: String }, diff --git a/pnpm/crates/config/src/workspace_yaml/registries.rs b/pnpm/crates/config/src/workspace_yaml/registries.rs index 31fa862551..4932e799fe 100644 --- a/pnpm/crates/config/src/workspace_yaml/registries.rs +++ b/pnpm/crates/config/src/workspace_yaml/registries.rs @@ -6,6 +6,8 @@ //! the routes, because a scope resolves to exactly one registry while a //! registry serves many. +pub use python::{EcosystemIndex, PythonRegistryRoute}; + pub use ecosystems::{Ecosystem, serves_another_ecosystem, take_roles_from_earlier_layers}; use super::LoadWorkspaceYamlError; @@ -82,6 +84,10 @@ pub struct RegistryDeclaration { /// else to serve means what it did. #[serde(default, skip_serializing_if = "Option::is_none")] pub ecosystem: Option, + /// Python package names or trailing-prefix patterns routed exclusively here. + /// Omitted, or `*`, declares the default Python index. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub packages: Option>, #[serde(flatten)] pub unknown: BTreeMap, } @@ -138,11 +144,8 @@ pub struct RegistryLookups { /// verifies against. pub registries_by_prefix: BTreeMap, pub registry_options_by_url: BTreeMap, - /// The indexes declared for each ecosystem other than npm, in the order - /// they are searched: the last one answers what none before it had. - /// npm is absent because its - /// registries are the three lookups above. - pub indexes_by_ecosystem: BTreeMap>, + /// Ecosystem indexes and their exclusive Python package routes. + pub indexes_by_ecosystem: BTreeMap>, } /// The scopes `entries` routes, `@`-prefixed, with the bare `@` among them @@ -462,3 +465,4 @@ pub(super) fn quote_and_join<'a>(values: impl IntoIterator) -> S } mod ecosystems; +mod python; diff --git a/pnpm/crates/config/src/workspace_yaml/registries/ecosystems.rs b/pnpm/crates/config/src/workspace_yaml/registries/ecosystems.rs index 9279e90afb..6ae80253b3 100644 --- a/pnpm/crates/config/src/workspace_yaml/registries/ecosystems.rs +++ b/pnpm/crates/config/src/workspace_yaml/registries/ecosystems.rs @@ -1,17 +1,8 @@ -//! Which ecosystem's packages a `registries` entry serves, and the index -//! lists that follow from it. -//! -//! npm is absent from those lists: its registries are addressed by the scope -//! and prefix routes in [`super::RegistryLookups`], while every other -//! ecosystem resolves from an ordered list of indexes. -//! -//! A list is held in the order the configuration declares it, which is the -//! order the ecosystem searches: the last index answers what none before it -//! had. +//! Ecosystem index declarations and exclusive Python package routing. use super::{ - LoadWorkspaceYamlError, RegistryDeclaration, RegistryLookups, normalize_registry_url, - quote_and_join, redact_registry_url, + EcosystemIndex, LoadWorkspaceYamlError, RegistryDeclaration, RegistryLookups, + normalize_registry_url, quote_and_join, redact_registry_url, }; use indexmap::IndexMap; use pnpm_lockfile::RegistryOptions; @@ -61,7 +52,7 @@ impl fmt::Display for Ecosystem { /// sparse index. #[derive(Default)] pub(super) struct DeclaredIndexes { - urls: BTreeMap>, + urls: BTreeMap>, } impl DeclaredIndexes { @@ -71,6 +62,12 @@ impl DeclaredIndexes { declaration: &RegistryDeclaration, ) -> Result<(), LoadWorkspaceYamlError> { let ecosystem = declaration.ecosystem(); + if ecosystem != Ecosystem::Pypi && declaration.packages.is_some() { + return Err(LoadWorkspaceYamlError::InvalidPythonRegistryPackages { + registry: redact_registry_url(registry), + reason: "packages is only supported for pypi registries".to_string(), + }); + } if ecosystem == Ecosystem::Npm { return Ok(()); } @@ -81,27 +78,31 @@ impl DeclaredIndexes { field: field.to_owned(), }); } - // Keyed by URL, the map cannot tell that `.../simple` and - // `.../simple/` are one index, so two spellings would take two places - // in the search order and the second would never be reached. let normalized = normalize_registry_url(registry); let declared = self.urls.entry(ecosystem).or_default(); - if declared.contains(&normalized) { + if declared + .iter() + .any(|index| index.url == normalized) + { return Err(LoadWorkspaceYamlError::EcosystemIndexDeclaredTwice { ecosystem: ecosystem.to_string(), registry: redact_registry_url(&normalized), }); } - declared.push(normalized); + declared.push(EcosystemIndex { url: normalized, packages: declaration.packages.clone() }); Ok(()) } pub(super) fn finish(&self) -> Result<(), LoadWorkspaceYamlError> { match self.urls.get(&Ecosystem::Cargo) { Some(urls) if urls.len() > 1 => Err(LoadWorkspaceYamlError::CargoIndexDeclaredTwice { - registries: quote_and_join(urls.iter().map(String::as_str)), + registries: quote_and_join(urls.iter().map(|index| index.url.as_str())), }), - _ => Ok(()), + _ => super::python::validate_routes( + self.urls + .get(&Ecosystem::Pypi) + .map_or(&[], Vec::as_slice), + ), } } } @@ -120,11 +121,8 @@ fn npm_only_field(declaration: &RegistryDeclaration) -> Option<&'static str> { } /// Record a non-npm entry's index, answering whether it was one. -/// -/// Appended, so the list keeps the order the configuration declares, which is -/// the order the ecosystem searches. pub(super) fn collect_index( - indexes_by_ecosystem: &mut BTreeMap>, + indexes_by_ecosystem: &mut BTreeMap>, normalized: &str, declaration: &RegistryDeclaration, ) -> bool { @@ -135,21 +133,23 @@ pub(super) fn collect_index( indexes_by_ecosystem .entry(ecosystem) .or_default() - .push(normalized.to_owned()); + .push(EcosystemIndex { + url: normalized.to_owned(), + packages: declaration.packages.clone(), + }); true } -/// Declare each ecosystem's indexes back into the `registries` shape. -/// -/// The map they are written into preserves insertion order, so reading the -/// result back declares the same search order. +/// Declare each ecosystem's indexes and package routes back into `registries`. pub(super) fn extend_with_indexes( declarations: &mut IndexMap, - indexes_by_ecosystem: &BTreeMap>, + indexes_by_ecosystem: &BTreeMap>, ) { for (&ecosystem, indexes) in indexes_by_ecosystem { for index in indexes { - declarations.entry(index.clone()).or_default().ecosystem = Some(ecosystem); + let declaration = declarations.entry(index.url.clone()).or_default(); + declaration.ecosystem = Some(ecosystem); + declaration.packages.clone_from(&index.packages); } } } @@ -170,13 +170,13 @@ pub fn take_roles_from_earlier_layers( registries_by_scope: &mut BTreeMap, registries_by_prefix: &mut BTreeMap, registry_options_by_url: &mut BTreeMap, - indexes_by_ecosystem: &mut BTreeMap>, + indexes_by_ecosystem: &mut BTreeMap>, layer: &RegistryLookups, ) { let declared_as_index: BTreeSet<&str> = layer.indexes_by_ecosystem .values() .flatten() - .map(String::as_str) + .map(|index| index.url.as_str()) .collect(); let declared_at_all: BTreeSet = declared_as_index .iter() @@ -189,10 +189,8 @@ pub fn take_roles_from_earlier_layers( if declared_at_all.is_empty() { return; } - // Whatever this layer says a URL is, it is no longer an index of some - // other ecosystem, nor of the same one in another position. for indexes in indexes_by_ecosystem.values_mut() { - indexes.retain(|registry| !declared_at_all.contains(registry.as_str())); + indexes.retain(|registry| !declared_at_all.contains(registry.url.as_str())); } indexes_by_ecosystem.retain(|_, indexes| !indexes.is_empty()); if declared_as_index.is_empty() { @@ -211,12 +209,12 @@ pub fn take_roles_from_earlier_layers( /// `namedRegistries` alias is kept as written. #[must_use] pub fn serves_another_ecosystem( - indexes_by_ecosystem: &BTreeMap>, + indexes_by_ecosystem: &BTreeMap>, registry: &str, ) -> bool { let normalized = normalize_registry_url(registry); indexes_by_ecosystem .values() .flatten() - .any(|index| index == &normalized) + .any(|index| index.url == normalized) } diff --git a/pnpm/crates/config/src/workspace_yaml/registries/python.rs b/pnpm/crates/config/src/workspace_yaml/registries/python.rs new file mode 100644 index 0000000000..4f349c8d9e --- /dev/null +++ b/pnpm/crates/config/src/workspace_yaml/registries/python.rs @@ -0,0 +1,170 @@ +use super::{LoadWorkspaceYamlError, redact_registry_url}; +use pnpr_registry::{Ecosystem, PackagePattern}; + +/// One ecosystem index and the Python namespace it owns. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EcosystemIndex { + pub url: String, + pub packages: Option>, +} + +impl From for EcosystemIndex { + fn from(url: String) -> Self { + Self { url, packages: None } + } +} + +/// A validated Python namespace and its authoritative index. +#[derive(Debug)] +pub struct PythonRegistryRoute { + pub url: String, + patterns: Vec, +} + +impl PythonRegistryRoute { + pub fn from_indexes(indexes: &[EcosystemIndex]) -> Result, LoadWorkspaceYamlError> { + validate_routes(indexes)?; + indexes + .iter() + .map(|index| Ok(Self { url: index.url.clone(), patterns: patterns(index)? })) + .collect() + } + + #[must_use] + pub fn matches(&self, name: &str) -> bool { + self.patterns + .iter() + .any(|pattern| pattern.matches(name)) + } + + #[must_use] + pub fn is_default(&self) -> bool { + self.patterns.iter().any(PythonPattern::is_default) + } + + #[must_use] + pub fn packages(&self) -> Vec { + let mut packages: Vec<_> = self.patterns + .iter() + .map(PythonPattern::normalized) + .collect(); + packages.sort(); + packages + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum PythonPattern { + Name(PackagePattern), + Prefix(String), +} + +impl PythonPattern { + pub(super) fn parse(pattern: &str) -> Result { + if pattern == "*" { + return Ok(Self::Name(PackagePattern::All)); + } + if pattern != "**" + && let Some(prefix) = pattern.strip_suffix('*') + { + // A prefix may end with a separator, but a complete Python name cannot. + let name = PackagePattern::parse(&format!("{prefix}x"), Ecosystem::Pypi) + .map_err(|error| error.to_string())?; + let normalized = name.to_string(); + return Ok(Self::Prefix( + normalized + .strip_suffix('x') + .expect("Python name normalization preserves the final ASCII letter") + .to_string(), + )); + } + PackagePattern::parse(pattern, Ecosystem::Pypi) + .map(Self::Name) + .map_err(|error| error.to_string()) + } + + pub(super) fn matches(&self, name: &str) -> bool { + match self { + Self::Name(pattern) => pattern.matches(name), + Self::Prefix(prefix) => name.starts_with(prefix), + } + } + + fn overlaps(&self, other: &Self) -> bool { + match (self, other) { + (Self::Prefix(a), Self::Prefix(b)) => a.starts_with(b) || b.starts_with(a), + (Self::Name(PackagePattern::Exact(name)), Self::Prefix(prefix)) + | (Self::Prefix(prefix), Self::Name(PackagePattern::Exact(name))) => { + name.starts_with(prefix) + } + (Self::Name(a), Self::Name(b)) => a.covers(b) || b.covers(a), + _ => true, + } + } + + pub(super) fn is_default(&self) -> bool { + matches!(self, Self::Name(PackagePattern::All)) + } + + pub(super) fn normalized(&self) -> String { + match self { + Self::Name(PackagePattern::All) => "*".to_string(), + Self::Name(pattern) => pattern.to_string(), + Self::Prefix(prefix) => format!("{prefix}*"), + } + } +} + +pub(super) fn patterns( + index: &EcosystemIndex, +) -> Result, LoadWorkspaceYamlError> { + let invalid = |reason: String| LoadWorkspaceYamlError::InvalidPythonRegistryPackages { + registry: redact_registry_url(&index.url), + reason, + }; + let Some(packages) = &index.packages else { + return Ok(vec![PythonPattern::Name(PackagePattern::All)]); + }; + if packages.is_empty() { + return Err(invalid("packages must not be empty".to_string())); + } + let patterns = packages + .iter() + .map(|pattern| PythonPattern::parse(pattern).map_err(&invalid)) + .collect::, _>>()?; + for (i, pattern) in patterns.iter().enumerate() { + if patterns[..i] + .iter() + .any(|other| pattern.overlaps(other)) + { + return Err(invalid(format!("overlapping package pattern {:?}", packages[i]))); + } + } + Ok(patterns) +} + +pub(super) fn validate_routes(indexes: &[EcosystemIndex]) -> Result<(), LoadWorkspaceYamlError> { + let mut claims: Vec<(PythonPattern, &str)> = Vec::new(); + for index in indexes { + for pattern in patterns(index)? { + if let Some((_, registry)) = claims + .iter() + .find(|(other, _)| { + pattern.is_default() == other.is_default() && pattern.overlaps(other) + }) + { + return Err(LoadWorkspaceYamlError::PythonPackageRoutedTwice { + pattern: pattern.normalized(), + registries: super::quote_and_join( + [*registry, index.url.as_str()] + .map(redact_registry_url) + .iter() + .map(String::as_str), + ), + }); + } + claims.push((pattern, &index.url)); + } + } + Ok(()) +} diff --git a/pnpm/crates/config/src/workspace_yaml/tests/registry_ecosystems.rs b/pnpm/crates/config/src/workspace_yaml/tests/registry_ecosystems.rs index b1fc91e1ad..a87e630422 100644 --- a/pnpm/crates/config/src/workspace_yaml/tests/registry_ecosystems.rs +++ b/pnpm/crates/config/src/workspace_yaml/tests/registry_ecosystems.rs @@ -26,9 +26,9 @@ fn an_entry_that_names_no_ecosystem_is_an_npm_registry() { } #[test] -fn the_indexes_are_read_in_the_order_the_configuration_declares_them() { +fn python_indexes_are_canonicalized_without_search_priority() { let config = load( - "registries:\n https://extra.example.com/simple/:\n ecosystem: pypi\n https://pypi.example.com/simple/:\n ecosystem: pypi\n", + "registries:\n https://extra.example.com/simple/:\n ecosystem: pypi\n packages: [alpha]\n https://pypi.example.com/simple/:\n ecosystem: pypi\n", ) .unwrap(); assert_eq!( @@ -38,18 +38,15 @@ fn the_indexes_are_read_in_the_order_the_configuration_declares_them() { assert!(config.registries_by_scope.is_empty()); } -/// The map is keyed by URL, so only an insertion-ordered one can answer which -/// index a lookup reaches first. Sorted by key, `zzz` would be searched after -/// `aaa` however the file was written. #[test] -fn declaration_order_survives_a_key_order_that_disagrees_with_it() { +fn python_index_urls_do_not_depend_on_declaration_order() { let config = load( - "registries:\n https://zzz.example.com/simple/:\n ecosystem: pypi\n https://aaa.example.com/simple/:\n ecosystem: pypi\n", + "registries:\n https://zzz.example.com/simple/:\n ecosystem: pypi\n packages: [alpha]\n https://aaa.example.com/simple/:\n ecosystem: pypi\n", ) .unwrap(); assert_eq!( config.python_indexes(), - ["https://zzz.example.com/simple/", "https://aaa.example.com/simple/"], + ["https://aaa.example.com/simple/", "https://zzz.example.com/simple/"], ); } @@ -103,7 +100,7 @@ fn an_unknown_ecosystem_is_refused() { #[test] fn the_declared_indexes_round_trip_through_the_resolved_view() { let config = load( - "registries:\n https://extra.example.com/simple/:\n ecosystem: pypi\n https://pypi.example.com/simple/:\n ecosystem: pypi\n", + "registries:\n https://extra.example.com/simple/:\n ecosystem: pypi\n packages: [alpha]\n https://pypi.example.com/simple/:\n ecosystem: pypi\n", ) .unwrap(); let declarations = config.resolved_registry_declarations(); @@ -115,31 +112,21 @@ fn the_declared_indexes_round_trip_through_the_resolved_view() { assert_eq!(indexes, ["https://extra.example.com/simple/", "https://pypi.example.com/simple/"]); } -/// The declarations a pnpr server is told about carry the search order too, -/// so a resolution it runs on the client's behalf reaches the same index -/// first. #[test] -fn the_order_survives_into_the_declarations_sent_to_a_server() { - let config = load( - "registries:\n https://zzz.example.com/simple/:\n ecosystem: pypi\n https://aaa.example.com/simple/:\n ecosystem: pypi\n", - ) - .unwrap(); - let declarations = config.registry_declarations(); - let declared: Vec<&str> = declarations - .iter() - .filter(|(_, entry)| entry.ecosystem() == Ecosystem::Pypi) - .map(|(registry, _)| registry.as_str()) - .collect(); - assert_eq!(declared, ["https://zzz.example.com/simple/", "https://aaa.example.com/simple/"]); +fn package_routes_survive_into_the_declarations_sent_to_a_server() { + let config = load("registries:\n https://private.example.com/:\n ecosystem: pypi\n packages: [alpha]\n https://public.example.com/:\n ecosystem: pypi\n").unwrap(); + for declarations in [config.registry_declarations(), config.resolved_registry_declarations()] { + assert_eq!( + declarations["https://private.example.com/"].packages, + Some(vec!["alpha".to_string()]), + ); + } } -/// Keyed by URL, the map cannot see that two spellings address one index, so -/// it would give them two places in the search order and never reach the -/// second. #[test] fn two_spellings_of_one_index_are_refused() { let error = load( - "registries:\n https://pypi.example.com/simple:\n ecosystem: pypi\n https://pypi.example.com/simple/:\n ecosystem: pypi\n", + "registries:\n https://pypi.example.com/simple:\n ecosystem: pypi\n packages: [alpha]\n https://pypi.example.com/simple/:\n ecosystem: pypi\n", ) .unwrap_err(); assert!(error.contains("declared twice"), "{error}"); @@ -200,7 +187,7 @@ fn a_url_a_layer_routes_to_npm_loses_the_index_role_it_had() { let mut config = Config::default(); config.indexes_by_ecosystem.insert( Ecosystem::Pypi, - vec!["https://one.example.com/".to_string()], + vec!["https://one.example.com/".to_string().into()], ); WorkspaceSettings::load_at(dir.path()) .unwrap() @@ -228,7 +215,7 @@ fn a_url_reclassified_to_another_ecosystem_leaves_the_first_one() { let mut config = Config::default(); config.indexes_by_ecosystem.insert( Ecosystem::Cargo, - vec!["https://one.example.com/".to_string()], + vec!["https://one.example.com/".to_string().into()], ); WorkspaceSettings::load_at(dir.path()) .unwrap() @@ -279,7 +266,7 @@ fn a_later_alias_does_not_address_an_index_an_earlier_layer_declared() { let mut config = Config::default(); config.indexes_by_ecosystem.insert( Ecosystem::Pypi, - vec!["https://pypi.example.com/simple/".to_string()], + vec!["https://pypi.example.com/simple/".to_string().into()], ); WorkspaceSettings::load_at(dir.path()) .unwrap() @@ -293,3 +280,47 @@ fn a_later_alias_does_not_address_an_index_an_earlier_layer_declared() { config.registries_by_prefix, ); } + +#[test] +fn overlapping_python_namespaces_are_rejected_before_resolution() { + for (first, second) in [ + ("alpha", "Alpha"), + ("Company-*", "company_tools"), + ("company-*", "company-tool*"), + ("*", "*"), + ("*", "**"), + ("**", "**"), + ] { + let error = load(&format!("registries:\n https://one.example.com/:\n ecosystem: pypi\n packages: ['{first}']\n https://two.example.com/:\n ecosystem: pypi\n packages: ['{second}']\n")).unwrap_err(); + assert!(error.contains("routed to two registries"), "{first}, {second}: {error}"); + } +} + +#[test] +fn invalid_python_package_patterns_are_rejected() { + for packages in [ + "[]", + "['company-**']", + "['@scope/*']", + "['alpha', 'ALPHA']", + "['*', 'alpha']", + "['**', 'alpha']", + ] { + let error = load(&format!("registries:\n https://private.example.com/:\n ecosystem: pypi\n packages: {packages}\n")).unwrap_err(); + assert!(error.contains("Invalid Python package routes"), "{packages}: {error}"); + } +} + +#[test] +fn multiple_python_defaults_require_explicit_package_routes() { + let error = load("registries:\n https://one.example.com/:\n ecosystem: pypi\n https://two.example.com/:\n ecosystem: pypi\n").unwrap_err(); + assert!(error.contains("routed to two registries"), "{error}"); +} + +#[test] +fn package_patterns_are_not_silently_ignored_by_other_ecosystems() { + for ecosystem in ["npm", "cargo"] { + let error = load(&format!("registries:\n https://one.example.com/:\n ecosystem: {ecosystem}\n packages: [alpha]\n")).unwrap_err(); + assert!(error.contains("only supported for pypi"), "{error}"); + } +} diff --git a/pnpm/crates/python-installer/src/lockfile.rs b/pnpm/crates/python-installer/src/lockfile.rs index 98ebc909f6..668a26079a 100644 --- a/pnpm/crates/python-installer/src/lockfile.rs +++ b/pnpm/crates/python-installer/src/lockfile.rs @@ -115,7 +115,7 @@ impl PythonPrepare<'_> { .any(|requirement| { matches!(requirement.version_or_url, Some(pep508_rs::VersionOrUrl::Url(_))) }) - || !self.index.extra_urls.is_empty() + || !self.index.can_resolve_remotely() { return Ok(None); } diff --git a/pnpm/crates/python-installer/src/registry.rs b/pnpm/crates/python-installer/src/registry.rs index 90fc6b38c8..9f58827d91 100644 --- a/pnpm/crates/python-installer/src/registry.rs +++ b/pnpm/crates/python-installer/src/registry.rs @@ -119,15 +119,10 @@ impl Registry<'_> { pub(super) async fn fetch_index(&mut self, name: &PackageName) -> Result<()> { self.resolution.packages.candidates.insert(name.clone(), BTreeMap::new()); self.resolution.packages.excluded.insert(name.clone(), Excluded::default()); - for index in self.index.extra_urls - .iter() - .chain(std::iter::once(&self.index.url)) - { - let page = self.read_index(index, name).await?; - if !page.missing { - self.resolution.offer(name, &page)?; - break; - } + let index = self.index.select(name.as_ref())?; + let page = self.read_index(index, name).await?; + if !page.missing { + self.resolution.offer(name, &page)?; } self.resolution.downloaded.insert(name.clone()); if self.resolution.packages.candidates[name] diff --git a/pnpm/crates/python-installer/src/settings.rs b/pnpm/crates/python-installer/src/settings.rs index 98565a8187..805e6bf140 100644 --- a/pnpm/crates/python-installer/src/settings.rs +++ b/pnpm/crates/python-installer/src/settings.rs @@ -4,31 +4,49 @@ use pnpm_diagnostics::miette::{Diagnostic, IntoDiagnostic, Result}; pub(crate) struct Index { pub(crate) url: url::Url, - pub(crate) extra_urls: Vec, + pub(crate) routes: Vec<(url::Url, pnpm_config::PythonRegistryRoute)>, pub(crate) auth: pnpm_network::AuthHeaders, } -/// The indexes `registries` declares for `PyPI`, in the order they are -/// searched, with the credentials the machine holds for them. -/// -/// Credentials are resolved by origin from the same auth sources every other -/// package source uses, which is why a `registries` key may carry none of its -/// own: the map lives in the committed `pnpm-workspace.yaml`. +/// Python namespace claims with the machine's existing registry credentials. pub(super) fn python_index(config: &pnpm_config::Config) -> Result { - let mut indexes = config - .python_indexes() + let configured = config.python_registry_indexes(); + let routes = pnpm_config::PythonRegistryRoute::from_indexes(&configured) + .into_diagnostic()? .into_iter() - .map(parse_index) + .map(|route| Ok((parse_index(&route.url)?, route))) .collect::>>()?; - // `Registry::fetch_index` reads `extra_urls` and then `url`, so the index - // declared last is the one that answers what none before it had. - let url = indexes.pop().expect("python_indexes answers with at least one index"); - let extra_urls = indexes; + let url = routes + .iter() + .find(|(_, route)| route.is_default()) + .unwrap_or(&routes[0]) + .0 + .clone(); let auth = (*config.auth_headers).clone().with_secure_transport(); - Ok(Index { url, extra_urls, auth }) + Ok(Index { url, routes, auth }) } impl Index { + pub(super) fn select(&self, name: &str) -> Result<&url::Url> { + self.routes + .iter() + .find(|(_, route)| !route.is_default() && route.matches(name)) + .or_else(|| self.routes.iter().find(|(_, route)| route.is_default())) + .map(|(url, _)| url) + .ok_or_else(|| UnclaimedPackage { name: name.to_string() }.into()) + } + + fn package_routes(&self) -> std::collections::BTreeMap> { + self.routes + .iter() + .map(|(url, route)| (url.to_string(), route.packages())) + .collect() + } + + pub(super) fn can_resolve_remotely(&self) -> bool { + self.routes.len() == 1 && self.routes[0].1.is_default() + } + pub(super) fn cache_key(&self, url: &url::Url) -> String { let key = self.auth .for_secure_url(url.as_str()) @@ -79,14 +97,10 @@ impl PythonPrepare<'_> { .chain(&rules.tool.uv.constraints) .map(|requirement| parse_rule(requirement)) .collect::>()?; - inputs.set_resolution_settings( - &self.index.extra_urls - .iter() - .map(ToString::to_string) - .collect::>(), - &packages.overrides, - &packages.constraints, - ); + inputs.set_resolution_settings(&[], &packages.overrides, &packages.constraints); + if !self.index.can_resolve_remotely() { + inputs.set_registry_packages(self.index.package_routes()); + } Ok(()) } @@ -121,5 +135,12 @@ struct UnsupportedRule { requirement: String, } +#[derive(Debug, Display, Error, Diagnostic)] +#[display("No Python registry claims package {name:?}")] +#[diagnostic(code(ERR_PNPM_UNCLAIMED_PYTHON_PACKAGE))] +struct UnclaimedPackage { + name: String, +} + #[cfg(test)] mod tests; diff --git a/pnpm/crates/python-installer/src/settings/tests.rs b/pnpm/crates/python-installer/src/settings/tests.rs index 3940565392..1bc413e0c9 100644 --- a/pnpm/crates/python-installer/src/settings/tests.rs +++ b/pnpm/crates/python-installer/src/settings/tests.rs @@ -8,36 +8,81 @@ fn config_with_pypi_indexes(indexes: &[&str]) -> Config { Ecosystem::Pypi, indexes .iter() - .map(|index| (*index).to_string()) + .map(|index| (*index).to_string().into()) .collect(), ); } + if let Some(indexes) = config.indexes_by_ecosystem.get_mut(&Ecosystem::Pypi) { + for (i, index) in indexes.iter_mut().enumerate().skip(1) { + index.packages = Some(vec![format!("package{i}")]); + } + } config } -/// `Registry::fetch_index` reads `extra_urls` and then `url`, so the index -/// declared last is the one that answers what none before it had. #[test] -fn the_index_declared_last_is_the_one_searched_last() { - let config = config_with_pypi_indexes(&[ - "https://first.test/simple/", - "https://second.test/simple/", - "https://last.test/simple/", - ]); +fn package_routes_select_one_index_independent_of_declaration_order() { + for reverse in [false, true] { + let mut config = Config::default(); + let mut indexes = vec![ + pnpm_config::EcosystemIndex { + url: "https://private.test/simple/".to_string(), + packages: Some(vec!["Company_*".to_string()]), + }, + pnpm_config::EcosystemIndex { + url: "https://public.test/simple/".to_string(), + packages: Some(vec!["*".to_string()]), + }, + ]; + if reverse { + indexes.reverse(); + } + config.indexes_by_ecosystem.insert(Ecosystem::Pypi, indexes); + let index = python_index(&config).unwrap(); + assert_eq!( + index + .select("company-tools") + .unwrap() + .as_str(), + "https://private.test/simple/", + ); + assert_eq!( + index + .select("requests") + .unwrap() + .as_str(), + "https://public.test/simple/", + ); + assert_eq!(index.url.as_str(), "https://public.test/simple/"); + assert!(!index.can_resolve_remotely()); + } +} + +#[test] +fn restricted_indexes_do_not_implicitly_enable_pypi() { + let mut config = Config::default(); + config.indexes_by_ecosystem.insert( + Ecosystem::Pypi, + vec![pnpm_config::EcosystemIndex { + url: "https://private.test/simple/".to_string(), + packages: Some(vec!["alpha".to_string()]), + }], + ); let index = python_index(&config).unwrap(); - assert_eq!(index.url.as_str(), "https://last.test/simple/"); - let extras: Vec<&str> = index.extra_urls - .iter() - .map(url::Url::as_str) - .collect(); - assert_eq!(extras, ["https://first.test/simple/", "https://second.test/simple/"]); + assert!( + index + .select("beta") + .unwrap_err() + .to_string() + .contains("No Python registry claims"), + ); } #[test] fn a_configuration_naming_no_index_resolves_from_pypi() { let index = python_index(&config_with_pypi_indexes(&[])).unwrap(); assert_eq!(index.url.as_str(), pnpm_config::DEFAULT_PYPI_INDEX_URL); - assert!(index.extra_urls.is_empty()); + assert!(index.can_resolve_remotely()); } #[test] diff --git a/pnpm/crates/python-resolver/src/lockfile/inputs.rs b/pnpm/crates/python-resolver/src/lockfile/inputs.rs index a761a8c331..0e169c0010 100644 --- a/pnpm/crates/python-resolver/src/lockfile/inputs.rs +++ b/pnpm/crates/python-resolver/src/lockfile/inputs.rs @@ -4,6 +4,7 @@ use super::Target; use pep508_rs::{MarkerEnvironment, Requirement}; use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; /// Everything a resolution depended on: what the project asked for, and /// the environments it was answered for. A server's answer is accepted @@ -42,6 +43,8 @@ pub struct Inputs { extra_indexes: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] overrides: Vec, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + registry_packages: BTreeMap>, #[serde(default, skip_serializing_if = "Vec::is_empty")] constraints: Vec, } @@ -53,7 +56,10 @@ impl Inputs { if self.requirements != wanted.requirements { return Some("the project's Python requirements changed"); } - if self.index != wanted.index || self.extra_indexes != wanted.extra_indexes { + if self.index != wanted.index + || self.extra_indexes != wanted.extra_indexes + || self.registry_packages != wanted.registry_packages + { return Some("the Python index changed"); } if self.overrides != wanted.overrides || self.constraints != wanted.constraints { @@ -79,6 +85,11 @@ impl Inputs { self.constraints = normalized(constraints); } + /// Record authoritative package routes; changes invalidate lockfile replay. + pub fn set_registry_packages(&mut self, packages: BTreeMap>) { + self.registry_packages = packages; + } + pub fn set_requirements(&mut self, requirements: &[Requirement]) { self.requirements = normalized(requirements); } @@ -108,6 +119,7 @@ impl Inputs { index: index.to_string(), extra_indexes: Vec::new(), overrides: Vec::new(), + registry_packages: BTreeMap::new(), constraints: Vec::new(), } } @@ -131,6 +143,7 @@ impl Inputs { index: index.to_string(), extra_indexes: Vec::new(), overrides: Vec::new(), + registry_packages: BTreeMap::new(), constraints: Vec::new(), } } diff --git a/pnpm/crates/python-resolver/src/lockfile/inputs/tests.rs b/pnpm/crates/python-resolver/src/lockfile/inputs/tests.rs index 6911675792..3fa52d2fd4 100644 --- a/pnpm/crates/python-resolver/src/lockfile/inputs/tests.rs +++ b/pnpm/crates/python-resolver/src/lockfile/inputs/tests.rs @@ -35,3 +35,22 @@ fn the_members_sharing_an_environment_invalidate_replay() { Some("the projects sharing the Python environment changed"), ); } + +#[test] +fn changing_package_routes_invalidates_lockfile_replay() { + let requirements = vec!["alpha".parse::().unwrap()]; + let mut previous = Inputs::declared(&requirements, &[], &[], "https://public.test/simple/"); + let mut wanted = Inputs::declared(&requirements, &[], &[], "https://public.test/simple/"); + previous.set_registry_packages(std::collections::BTreeMap::from([( + "https://private.test/simple/".to_string(), + vec!["alpha".to_string()], + )])); + wanted.set_registry_packages(std::collections::BTreeMap::from([( + "https://private.test/simple/".to_string(), + vec!["beta".to_string()], + )])); + assert_eq!(previous.differs_from(&wanted), Some("the Python index changed")); + let serialized = serde_json::to_string(&previous).unwrap(); + let round_trip: Inputs = serde_json::from_str(&serialized).unwrap(); + assert_eq!(previous, round_trip); +}