diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2c5ee44387c1..43e2f80050db 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -48,10 +48,13 @@ jobs: name: "cargo test | ${{ matrix.os }}" steps: - uses: actions/checkout@v4 + # TODO(konstin): Mock the venv in the installer test so we don't need 3.8 anymore - name: "Install Python" uses: actions/setup-python@v4 with: - python-version: ${{ env.PYTHON_VERSION }} + python-version: | + 3.8 + ${{ env.PYTHON_VERSION }} - name: "Install Rust toolchain" run: rustup show - name: "Install cargo insta" diff --git a/.gitignore b/.gitignore index ea73bbd3e4db..da5e94a6c326 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,6 @@ target/ # Use e.g. `--cache-dir cache-docker` to keep a cache across container invocations cache-* + +# python tmp files +__pycache__ diff --git a/crates/pep440-rs/src/version_specifier.rs b/crates/pep440-rs/src/version_specifier.rs index e4f96b298c94..142bd487f6ec 100644 --- a/crates/pep440-rs/src/version_specifier.rs +++ b/crates/pep440-rs/src/version_specifier.rs @@ -540,6 +540,9 @@ impl Display for VersionSpecifier { /// ``` pub fn parse_version_specifiers(spec: &str) -> Result, Pep440Error> { let mut version_ranges = Vec::new(); + if spec.is_empty() { + return Ok(version_ranges); + } let mut start: usize = 0; let separator = ","; for version_range_spec in spec.split(separator) { @@ -1301,4 +1304,11 @@ mod test { ">=3.7, <4.0, !=3.9.0" ); } + + /// These occur in the simple api, e.g. + /// + #[test] + fn test_version_specifiers_empty() { + assert_eq!(VersionSpecifiers::from_str("").unwrap().to_string(), ""); + } } diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index 04e9c41bea47..b943db706026 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -125,6 +125,11 @@ pub(crate) async fn pip_compile( || Cow::Borrowed(venv.interpreter_info().markers()), |python_version| Cow::Owned(python_version.markers(venv.interpreter_info().markers())), ); + // Inject the fake python version if necessary + let interpreter_info = venv + .interpreter_info() + .clone() + .patch_markers(markers.clone().into_owned()); // Instantiate a client. let client = { @@ -143,7 +148,7 @@ pub(crate) async fn pip_compile( let build_dispatch = BuildDispatch::new( client.clone(), cache.to_path_buf(), - venv.interpreter_info().clone(), + interpreter_info, fs::canonicalize(venv.python_executable())?, no_build, ); diff --git a/crates/puffin-cli/src/commands/pip_sync.rs b/crates/puffin-cli/src/commands/pip_sync.rs index 00c189fc0d99..7a4ccaa8dc80 100644 --- a/crates/puffin-cli/src/commands/pip_sync.rs +++ b/crates/puffin-cli/src/commands/pip_sync.rs @@ -128,8 +128,9 @@ pub(crate) async fn sync_requirements( } else { let start = std::time::Instant::now(); - let wheel_finder = puffin_resolver::DistFinder::new(&tags, &client) - .with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64)); + let wheel_finder = + puffin_resolver::DistFinder::new(&tags, &client, venv.interpreter_info()) + .with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64)); let resolution = wheel_finder.resolve(&remote).await?; let s = if resolution.len() == 1 { "" } else { "s" }; diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index 170e20fe780a..a3716e7fe923 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -630,6 +630,55 @@ fn compile_python_37() -> Result<()> { Ok(()) } +/// Test that we select the last 3.8 compatible numpy version instead of trying to compile an +/// incompatible sdist +#[test] +fn compile_numpy_py38() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.touch()?; + requirements_in.write_str("numpy")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("requirements.in") + .arg("--python-version") + .arg("py38") + .arg("--cache-dir") + .arg(cache_dir.path()) + // Check that we select the wheel and not the sdist + .arg("--no-build") + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to build distribution: numpy-1.24.4.tar.gz + Caused by: Building source distributions is disabled + "###); + }); + + Ok(()) +} + /// Resolve a specific Flask wheel via a URL dependency. #[test] fn compile_wheel_url_dependency() -> Result<()> { diff --git a/crates/puffin-cli/tests/pip_sync.rs b/crates/puffin-cli/tests/pip_sync.rs index 6d66c72f58f5..001fd730e79b 100644 --- a/crates/puffin-cli/tests/pip_sync.rs +++ b/crates/puffin-cli/tests/pip_sync.rs @@ -2,7 +2,7 @@ use std::process::Command; -use anyhow::Result; +use anyhow::{Context, Result}; use assert_cmd::prelude::*; use assert_fs::prelude::*; use insta_cmd::_macro_support::insta; @@ -928,3 +928,50 @@ fn install_version_then_install_url() -> Result<()> { Ok(()) } + +/// Test that we select the last 3.8 compatible numpy version instead of trying to compile an +/// incompatible sdist +#[test] +fn install_numpy_py38() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--python") + // TODO(konstin): Mock the venv in the installer test so we don't need this anymore + .arg(which::which("python3.8").context("python3.8 must be installed")?) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let requirements_txt = temp_dir.child("requirements.txt"); + requirements_txt.touch()?; + requirements_txt.write_str("numpy")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-sync") + .arg("requirements.txt") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir)); + }); + + Command::new(venv.join("bin").join("python")) + .arg("-c") + .arg("import numpy") + .current_dir(&temp_dir) + .assert() + .success(); + + Ok(()) +} diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_markers.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_markers.snap index 070d5bf3201b..042ad37b078c 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_markers.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_markers.snap @@ -8,9 +8,9 @@ info: - "--constraint" - constraints.txt - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1Ts53o + - /tmp/.tmp81zzAa env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1vzBQa/.venv + VIRTUAL_ENV: /tmp/.tmpFXMFLd/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_txt.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_txt.snap index e2b3adf725e5..49cd0a9e4d9b 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_txt.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_txt.snap @@ -8,9 +8,9 @@ info: - "--constraint" - constraints.txt - "--cache-dir" - - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpvjdHOb + - /tmp/.tmpuS4Jn4 env: - VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpSAaWi3/.venv + VIRTUAL_ENV: /tmp/.tmpnFovuD/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml.snap index 1ace0bbb35f3..42b7e1d50e90 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml.snap @@ -6,9 +6,9 @@ info: - pip-compile - pyproject.toml - "--cache-dir" - - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpOOTFwj + - /tmp/.tmpWzFzu7 env: - VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpU0VXyY/.venv + VIRTUAL_ENV: /tmp/.tmpSiRk6d/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap index 2e65eb521d31..d37d801d099c 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap @@ -7,9 +7,9 @@ info: - pyproject.toml - "--all-extras" - "--cache-dir" - - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpFzJKRe + - /tmp/.tmpQHVFqr env: - VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpdadcmu/.venv + VIRTUAL_ENV: /tmp/.tmpmKXqaF/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra.snap index 78e32bbacef9..66f648165ac4 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra.snap @@ -8,9 +8,9 @@ info: - "--extra" - foo - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpAYEAdM + - /tmp/.tmpLjFVr0 env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1xuOcV/.venv + VIRTUAL_ENV: /tmp/.tmpDT0oRC/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra_name_normalization.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra_name_normalization.snap index 5eac4f49f1de..36f83c9a5158 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra_name_normalization.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra_name_normalization.snap @@ -8,9 +8,9 @@ info: - "--extra" - FRiENDlY-...-_-BARd - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpFyLXQn + - /tmp/.tmpga6JO1 env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpxfZatv/.venv + VIRTUAL_ENV: /tmp/.tmpwmJIym/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_python_37.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_python_37.snap index ec4e94eb422b..288ba44826d0 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_python_37.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_python_37.snap @@ -8,38 +8,16 @@ info: - "--python-version" - py37 - "--cache-dir" - - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpzwzUVe + - /tmp/.tmpHz7Dc2 env: - VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpqFv4YL/.venv + VIRTUAL_ENV: /tmp/.tmpoMjDJT/.venv --- -success: true -exit_code: 0 +success: false +exit_code: 1 ----- stdout ----- -# This file was autogenerated by Puffin v0.0.1 via the following command: -# [BIN_PATH] pip-compile requirements.in --python-version py37 --cache-dir [CACHE_DIR] -black==23.10.1 -click==8.1.7 - # via black -importlib-metadata==6.8.0 - # via click -mypy-extensions==1.0.0 - # via black -packaging==23.2 - # via black -pathspec==0.11.2 - # via black -platformdirs==4.0.0 - # via black -tomli==2.0.1 - # via black -typing-extensions==4.8.0 - # via - # black - # importlib-metadata - # platformdirs -zipp==3.17.0 - # via importlib-metadata ----- stderr ----- -Resolved 10 packages in [TIME] + × No solution found when resolving dependencies: + ╰─▶ Because there is no version of black available matching ==23.10.1 and + root depends on black==23.10.1, version solving failed. diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_requirements_in.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_requirements_in.snap index 04474bfc932c..da69b17702cd 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_requirements_in.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_requirements_in.snap @@ -6,9 +6,9 @@ info: - pip-compile - requirements.in - "--cache-dir" - - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpxF1zFY + - /tmp/.tmpQB4Ze4 env: - VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmp33QZdv/.venv + VIRTUAL_ENV: /tmp/.tmpSYcMgG/.venv --- success: true exit_code: 0 diff --git a/crates/puffin-cli/tests/snapshots/pip_sync__install_numpy_py38.snap b/crates/puffin-cli/tests/snapshots/pip_sync__install_numpy_py38.snap new file mode 100644 index 000000000000..a7d0072a3f63 --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_sync__install_numpy_py38.snap @@ -0,0 +1,23 @@ +--- +source: crates/puffin-cli/tests/pip_sync.rs +info: + program: puffin + args: + - pip-sync + - requirements.txt + - "--cache-dir" + - /tmp/.tmpL9E9fl + env: + VIRTUAL_ENV: /tmp/.tmp0yDHj5/.venv +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +Resolved 1 package in [TIME] +Downloaded 1 package in [TIME] +Unzipped 1 package in [TIME] +Installed 1 package in [TIME] + + numpy==1.24.4 + diff --git a/crates/puffin-dispatch/src/lib.rs b/crates/puffin-dispatch/src/lib.rs index ad6577a26b50..f275a266b2c9 100644 --- a/crates/puffin-dispatch/src/lib.rs +++ b/crates/puffin-dispatch/src/lib.rs @@ -135,7 +135,7 @@ impl BuildContext for BuildDispatch { if remote.len() == 1 { "" } else { "s" }, remote.iter().map(ToString::to_string).join(", ") ); - let resolution = DistFinder::new(&tags, &self.client) + let resolution = DistFinder::new(&tags, &self.client, self.interpreter_info()) .resolve(&remote) .await .context("Failed to resolve build dependencies")?; diff --git a/crates/puffin-distribution/src/traits.rs b/crates/puffin-distribution/src/traits.rs index 858f228b1506..a40bdda6a3ee 100644 --- a/crates/puffin-distribution/src/traits.rs +++ b/crates/puffin-distribution/src/traits.rs @@ -12,8 +12,8 @@ pub trait Metadata { /// Return the normalized [`PackageName`] of the distribution. fn name(&self) -> &PackageName; - /// Return a [`Version`], for registry-based distributions, or a [`Url`], for URL-based - /// distributions. + /// Return a [`pep440_rs::Version`], for registry-based distributions, or a [`url::Url`], + /// for URL-based distributions. fn version_or_url(&self) -> VersionOrUrl; /// Returns a unique identifier for the package. diff --git a/crates/puffin-interpreter/src/interpreter_info.rs b/crates/puffin-interpreter/src/interpreter_info.rs index 7f06a1ba6e1f..9fdbe47bc360 100644 --- a/crates/puffin-interpreter/src/interpreter_info.rs +++ b/crates/puffin-interpreter/src/interpreter_info.rs @@ -44,6 +44,21 @@ impl InterpreterInfo { base_prefix: info.base_prefix, }) } + + // TODO(konstin): Find a better way mocking the fields + pub fn artificial( + platform: Platform, + markers: MarkerEnvironment, + base_exec_prefix: PathBuf, + base_prefix: PathBuf, + ) -> Self { + Self { + platform: PythonPlatform(platform), + markers, + base_exec_prefix, + base_prefix, + } + } } impl InterpreterInfo { @@ -76,6 +91,12 @@ impl InterpreterInfo { pub fn base_prefix(&self) -> &Path { &self.base_prefix } + + /// Inject markers of a fake python version + #[must_use] + pub fn patch_markers(self, markers: MarkerEnvironment) -> Self { + Self { markers, ..self } + } } #[derive(Debug, Error)] diff --git a/crates/puffin-resolver/Cargo.toml b/crates/puffin-resolver/Cargo.toml index c3af190bcfa5..7e07fcbdfbdc 100644 --- a/crates/puffin-resolver/Cargo.toml +++ b/crates/puffin-resolver/Cargo.toml @@ -21,6 +21,7 @@ puffin-cache = { path = "../puffin-cache" } puffin-client = { path = "../puffin-client" } puffin-distribution = { path = "../puffin-distribution" } puffin-git = { path = "../puffin-git" } +puffin-interpreter = { path = "../puffin-interpreter" } puffin-normalize = { path = "../puffin-normalize" } puffin-traits = { path = "../puffin-traits" } pypi-types = { path = "../pypi-types" } diff --git a/crates/puffin-resolver/src/finder.rs b/crates/puffin-resolver/src/finder.rs index 98ac10b327f8..7f26d1f6f2e4 100644 --- a/crates/puffin-resolver/src/finder.rs +++ b/crates/puffin-resolver/src/finder.rs @@ -14,6 +14,7 @@ use pep508_rs::{Requirement, VersionOrUrl}; use platform_tags::Tags; use puffin_client::RegistryClient; use puffin_distribution::Dist; +use puffin_interpreter::InterpreterInfo; use puffin_normalize::PackageName; use pypi_types::{File, SimpleJson}; @@ -24,15 +25,21 @@ pub struct DistFinder<'a> { tags: &'a Tags, client: &'a RegistryClient, reporter: Option>, + interpreter_info: &'a InterpreterInfo, } impl<'a> DistFinder<'a> { /// Initialize a new distribution finder. - pub fn new(tags: &'a Tags, client: &'a RegistryClient) -> Self { + pub fn new( + tags: &'a Tags, + client: &'a RegistryClient, + interpreter_info: &'a InterpreterInfo, + ) -> Self { Self { tags, client, reporter: None, + interpreter_info, } } @@ -129,6 +136,20 @@ impl<'a> DistFinder<'a> { fn select(&self, requirement: &Requirement, files: Vec) -> Option { let mut fallback = None; for file in files.into_iter().rev() { + // Only add dists compatible with the python version. + // This is relevant for source dists which give no other indication of their + // compatibility and wheels which may be tagged `py3-none-any` but + // have `requires-python: ">=3.9"` + // TODO(konstin): https://github.com/astral-sh/puffin/issues/406 + if !file + .requires_python + .as_ref() + .map_or(true, |requires_python| { + requires_python.contains(self.interpreter_info.version()) + }) + { + continue; + } if let Ok(wheel) = WheelFilename::from_str(file.filename.as_str()) { if !wheel.is_compatible(self.tags) { continue; diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 18bd03533594..17aceb395d97 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -536,6 +536,21 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { // distributions. let mut version_map: VersionMap = BTreeMap::new(); for file in metadata.files { + // Only add dists compatible with the python version. + // This is relevant for source dists which give no other indication of their + // compatibility and wheels which may be tagged `py3-none-any` but + // have `requires-python: ">=3.9"` + // TODO(konstin): https://github.com/astral-sh/puffin/issues/406 + if !file + .requires_python + .as_ref() + .map_or(true, |requires_python| { + requires_python + .contains(self.build_context.interpreter_info().version()) + }) + { + continue; + } if let Ok(filename) = WheelFilename::from_str(file.filename.as_str()) { if filename.is_compatible(self.tags) { let version = PubGrubVersion::from(filename.version.clone()); diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index 87451691699d..f8550a5eba0a 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -4,7 +4,7 @@ //! `PyPI` directly. use std::future::Future; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::pin::Pin; use std::str::FromStr; @@ -17,10 +17,12 @@ use platform_host::{Arch, Os, Platform}; use platform_tags::Tags; use puffin_client::RegistryClientBuilder; use puffin_interpreter::{InterpreterInfo, Virtualenv}; -use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, Resolver}; +use puffin_resolver::{Graph, Manifest, PreReleaseMode, ResolutionMode, Resolver}; use puffin_traits::BuildContext; -struct DummyContext; +struct DummyContext { + interpreter_info: InterpreterInfo, +} impl BuildContext for DummyContext { fn cache(&self) -> &Path { @@ -28,7 +30,7 @@ impl BuildContext for DummyContext { } fn interpreter_info(&self) -> &InterpreterInfo { - panic!("The test should not need to build source distributions") + &self.interpreter_info } fn base_python(&self) -> &Path { @@ -61,13 +63,29 @@ impl BuildContext for DummyContext { } } +async fn resolve( + manifest: Manifest, + markers: &'static MarkerEnvironment, + tags: &Tags, +) -> Result { + let tempdir = tempdir()?; + let client = RegistryClientBuilder::new(tempdir.path()).build(); + let build_context = DummyContext { + interpreter_info: InterpreterInfo::artificial( + Platform::current()?, + markers.clone(), + PathBuf::from("/dev/null"), + PathBuf::from("/dev/null"), + ), + }; + let resolver = Resolver::new(manifest, markers, tags, &client, &build_context); + Ok(resolver.resolve().await?) +} + #[tokio::test] async fn black() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], @@ -77,8 +95,7 @@ async fn black() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -89,9 +106,6 @@ async fn black() -> Result<()> { async fn black_colorama() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black[colorama]<=23.9.1").unwrap()], vec![], @@ -101,8 +115,7 @@ async fn black_colorama() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -113,9 +126,6 @@ async fn black_colorama() -> Result<()> { async fn black_python_310() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], @@ -125,8 +135,7 @@ async fn black_python_310() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_310, &TAGS_310, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_310, &TAGS_310).await?; insta::assert_display_snapshot!(resolution); @@ -139,9 +148,6 @@ async fn black_python_310() -> Result<()> { async fn black_mypy_extensions() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("mypy-extensions<0.4.4").unwrap()], @@ -151,8 +157,7 @@ async fn black_mypy_extensions() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -165,9 +170,6 @@ async fn black_mypy_extensions() -> Result<()> { async fn black_mypy_extensions_extra() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("mypy-extensions[extra]<0.4.4").unwrap()], @@ -177,8 +179,7 @@ async fn black_mypy_extensions_extra() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -191,9 +192,6 @@ async fn black_mypy_extensions_extra() -> Result<()> { async fn black_flake8() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("flake8<1").unwrap()], @@ -203,8 +201,7 @@ async fn black_flake8() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -215,9 +212,6 @@ async fn black_flake8() -> Result<()> { async fn black_lowest() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black>21").unwrap()], vec![], @@ -227,8 +221,7 @@ async fn black_lowest() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -239,9 +232,6 @@ async fn black_lowest() -> Result<()> { async fn black_lowest_direct() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black>21").unwrap()], vec![], @@ -251,8 +241,7 @@ async fn black_lowest_direct() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -263,9 +252,6 @@ async fn black_lowest_direct() -> Result<()> { async fn black_respect_preference() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], @@ -275,8 +261,7 @@ async fn black_respect_preference() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -287,9 +272,6 @@ async fn black_respect_preference() -> Result<()> { async fn black_ignore_preference() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], @@ -299,8 +281,7 @@ async fn black_ignore_preference() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -311,9 +292,6 @@ async fn black_ignore_preference() -> Result<()> { async fn black_disallow_prerelease() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=20.0").unwrap()], vec![], @@ -323,8 +301,9 @@ async fn black_disallow_prerelease() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let err = resolver.resolve().await.unwrap_err(); + let err = resolve(manifest, &MARKERS_311, &TAGS_311) + .await + .unwrap_err(); insta::assert_display_snapshot!(err); @@ -335,9 +314,6 @@ async fn black_disallow_prerelease() -> Result<()> { async fn black_allow_prerelease_if_necessary() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("black<=20.0").unwrap()], vec![], @@ -347,10 +323,11 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await.unwrap_err(); + let err = resolve(manifest, &MARKERS_311, &TAGS_311) + .await + .unwrap_err(); - insta::assert_display_snapshot!(resolution); + insta::assert_display_snapshot!(err); Ok(()) } @@ -359,9 +336,6 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> { async fn pylint_disallow_prerelease() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("pylint==2.3.0").unwrap()], vec![], @@ -371,8 +345,7 @@ async fn pylint_disallow_prerelease() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -383,9 +356,6 @@ async fn pylint_disallow_prerelease() -> Result<()> { async fn pylint_allow_prerelease() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![Requirement::from_str("pylint==2.3.0").unwrap()], vec![], @@ -395,8 +365,7 @@ async fn pylint_allow_prerelease() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -407,9 +376,6 @@ async fn pylint_allow_prerelease() -> Result<()> { async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![ Requirement::from_str("pylint==2.3.0").unwrap(), @@ -422,8 +388,7 @@ async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); @@ -434,9 +399,6 @@ async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> { async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> { colored::control::set_override(false); - let tempdir = tempdir()?; - let client = RegistryClientBuilder::new(tempdir.path()).build(); - let manifest = Manifest::new( vec![ Requirement::from_str("pylint==2.3.0").unwrap(), @@ -449,8 +411,7 @@ async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> { None, ); - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; + let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?; insta::assert_display_snapshot!(resolution); diff --git a/crates/pypi-types/src/lenient_requirement.rs b/crates/pypi-types/src/lenient_requirement.rs new file mode 100644 index 000000000000..fc597682e0f3 --- /dev/null +++ b/crates/pypi-types/src/lenient_requirement.rs @@ -0,0 +1,64 @@ +use pep440_rs::{Pep440Error, VersionSpecifiers}; +use serde::{de, Deserialize, Deserializer, Serialize}; +use std::str::FromStr; +use tracing::warn; + +/// Like [`VersionSpecifiers`], but attempts to correct some common errors in user-provided requirements. +/// +/// We turn `>=3.x.*` into `>=3.x` +#[derive(Debug, Clone, Serialize, Eq, PartialEq)] +pub struct LenientVersionSpecifiers(VersionSpecifiers); + +impl FromStr for LenientVersionSpecifiers { + type Err = Pep440Error; + + fn from_str(s: &str) -> Result { + match VersionSpecifiers::from_str(s) { + Ok(specifiers) => Ok(Self(specifiers)), + Err(err) => { + // Given `>=3.5.*`, rewrite to `>=3.5`. + let patched = match s { + ">=3.12.*" => Some(">=3.12"), + ">=3.11.*" => Some(">=3.11"), + ">=3.10.*" => Some(">=3.10"), + ">=3.9.*" => Some(">=3.9"), + ">=3.8.*" => Some(">=3.8"), + ">=3.7.*" => Some(">=3.7"), + ">=3.6.*" => Some(">=3.6"), + ">=3.5.*" => Some(">=3.5"), + ">=3.4.*" => Some(">=3.4"), + ">=3.3.*" => Some(">=3.3"), + ">=3.2.*" => Some(">=3.2"), + ">=3.1.*" => Some(">=3.1"), + ">=3.0.*" => Some(">=3.0"), + _ => None, + }; + if let Some(patched) = patched { + if let Ok(specifier) = VersionSpecifiers::from_str(patched) { + warn!( + "Correcting invalid wildcard bound on version specifier (before: `{s}`; after: `{patched}`)", + ); + return Ok(Self(specifier)); + } + } + Err(err) + } + } + } +} + +impl From for VersionSpecifiers { + fn from(specifiers: LenientVersionSpecifiers) -> Self { + specifiers.0 + } +} + +impl<'de> Deserialize<'de> for LenientVersionSpecifiers { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + Self::from_str(&s).map_err(de::Error::custom) + } +} diff --git a/crates/pypi-types/src/lib.rs b/crates/pypi-types/src/lib.rs index 04393f84889e..6fccd14755ec 100644 --- a/crates/pypi-types/src/lib.rs +++ b/crates/pypi-types/src/lib.rs @@ -1,7 +1,9 @@ pub use direct_url::{ArchiveInfo, DirectUrl, VcsInfo, VcsKind}; +pub use lenient_requirement::LenientVersionSpecifiers; pub use metadata::{Error, Metadata21}; pub use simple_json::{File, SimpleJson}; mod direct_url; +mod lenient_requirement; mod metadata; mod simple_json; diff --git a/crates/pypi-types/src/metadata.rs b/crates/pypi-types/src/metadata.rs index e4d7258c4d85..f63140be99f9 100644 --- a/crates/pypi-types/src/metadata.rs +++ b/crates/pypi-types/src/metadata.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use tracing::warn; +use crate::lenient_requirement::LenientVersionSpecifiers; use pep440_rs::{Pep440Error, Version, VersionSpecifiers}; use pep508_rs::{Pep508Error, Requirement}; use puffin_normalize::{ExtraName, InvalidNameError, PackageName}; @@ -271,54 +272,6 @@ impl From for Requirement { } } -/// Like [`VersionSpecifiers`], but attempts to correct some common errors in user-provided requirements. -#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] -struct LenientVersionSpecifiers(VersionSpecifiers); - -impl FromStr for LenientVersionSpecifiers { - type Err = Pep440Error; - - fn from_str(s: &str) -> Result { - match VersionSpecifiers::from_str(s) { - Ok(specifiers) => Ok(Self(specifiers)), - Err(err) => { - // Given `>=3.5.*`, rewrite to `>=3.5`. - let patched = match s { - ">=3.12.*" => Some(">=3.12"), - ">=3.11.*" => Some(">=3.11"), - ">=3.10.*" => Some(">=3.10"), - ">=3.9.*" => Some(">=3.9"), - ">=3.8.*" => Some(">=3.8"), - ">=3.7.*" => Some(">=3.7"), - ">=3.6.*" => Some(">=3.6"), - ">=3.5.*" => Some(">=3.5"), - ">=3.4.*" => Some(">=3.4"), - ">=3.3.*" => Some(">=3.3"), - ">=3.2.*" => Some(">=3.2"), - ">=3.1.*" => Some(">=3.1"), - ">=3.0.*" => Some(">=3.0"), - _ => None, - }; - if let Some(patched) = patched { - if let Ok(specifier) = VersionSpecifiers::from_str(patched) { - warn!( - "Correcting invalid wildcard bound on version specifier (before: `{s}`; after: `{patched}`)", - ); - return Ok(Self(specifier)); - } - } - Err(err) - } - } - } -} - -impl From for VersionSpecifiers { - fn from(specifiers: LenientVersionSpecifiers) -> Self { - specifiers.0 - } -} - #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/crates/pypi-types/src/simple_json.rs b/crates/pypi-types/src/simple_json.rs index 4f424b28011f..56138b8f3b0d 100644 --- a/crates/pypi-types/src/simple_json.rs +++ b/crates/pypi-types/src/simple_json.rs @@ -1,4 +1,8 @@ -use serde::{Deserialize, Serialize}; +use pep440_rs::VersionSpecifiers; +use serde::{de, Deserialize, Deserializer, Serialize}; +use std::str::FromStr; + +use crate::lenient_requirement::LenientVersionSpecifiers; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SimpleJson { @@ -15,13 +19,30 @@ pub struct File { pub data_dist_info_metadata: Metadata, pub filename: String, pub hashes: Hashes, - pub requires_python: Option, + /// Note: Deserialized with [`LenientVersionSpecifiers`] since there are a number of invalid + /// versions on pypi + #[serde(deserialize_with = "deserialize_version_specifiers_lenient")] + pub requires_python: Option, pub size: usize, pub upload_time: String, pub url: String, pub yanked: Yanked, } +fn deserialize_version_specifiers_lenient<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let maybe_string: Option = Option::deserialize(deserializer)?; + let Some(string) = maybe_string else { + return Ok(None); + }; + let lenient = LenientVersionSpecifiers::from_str(&string).map_err(de::Error::custom)?; + Ok(Some(lenient.into())) +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] pub enum Metadata {