Filter out incompatible dists (#398)

Filter out source dists and wheels whose `requires-python` from the
simple api is incompatible with the current python version.

This change showed an important problem: When we use a fake python
version for resolving, building source distributions breaks down because
we can only build with versions we actually have.

This change became surprisingly big. The tests now require python 3.7 to
be installed, but changing that would mean an even bigger change.

Fixes #388
This commit is contained in:
konsti 2023-11-13 17:14:07 +01:00 committed by GitHub
parent 81c9cd0d4a
commit 76a41066ac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
27 changed files with 365 additions and 187 deletions

View file

@ -48,10 +48,13 @@ jobs:
name: "cargo test | ${{ matrix.os }}" name: "cargo test | ${{ matrix.os }}"
steps: steps:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
# TODO(konstin): Mock the venv in the installer test so we don't need 3.8 anymore
- name: "Install Python" - name: "Install Python"
uses: actions/setup-python@v4 uses: actions/setup-python@v4
with: with:
python-version: ${{ env.PYTHON_VERSION }} python-version: |
3.8
${{ env.PYTHON_VERSION }}
- name: "Install Rust toolchain" - name: "Install Rust toolchain"
run: rustup show run: rustup show
- name: "Install cargo insta" - name: "Install cargo insta"

3
.gitignore vendored
View file

@ -13,3 +13,6 @@ target/
# Use e.g. `--cache-dir cache-docker` to keep a cache across container invocations # Use e.g. `--cache-dir cache-docker` to keep a cache across container invocations
cache-* cache-*
# python tmp files
__pycache__

View file

@ -540,6 +540,9 @@ impl Display for VersionSpecifier {
/// ``` /// ```
pub fn parse_version_specifiers(spec: &str) -> Result<Vec<VersionSpecifier>, Pep440Error> { pub fn parse_version_specifiers(spec: &str) -> Result<Vec<VersionSpecifier>, Pep440Error> {
let mut version_ranges = Vec::new(); let mut version_ranges = Vec::new();
if spec.is_empty() {
return Ok(version_ranges);
}
let mut start: usize = 0; let mut start: usize = 0;
let separator = ","; let separator = ",";
for version_range_spec in spec.split(separator) { for version_range_spec in spec.split(separator) {
@ -1301,4 +1304,11 @@ mod test {
">=3.7, <4.0, !=3.9.0" ">=3.7, <4.0, !=3.9.0"
); );
} }
/// These occur in the simple api, e.g.
/// <https://pypi.org/simple/geopandas/?format=application/vnd.pypi.simple.v1+json>
#[test]
fn test_version_specifiers_empty() {
assert_eq!(VersionSpecifiers::from_str("").unwrap().to_string(), "");
}
} }

View file

@ -125,6 +125,11 @@ pub(crate) async fn pip_compile(
|| Cow::Borrowed(venv.interpreter_info().markers()), || Cow::Borrowed(venv.interpreter_info().markers()),
|python_version| Cow::Owned(python_version.markers(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. // Instantiate a client.
let client = { let client = {
@ -143,7 +148,7 @@ pub(crate) async fn pip_compile(
let build_dispatch = BuildDispatch::new( let build_dispatch = BuildDispatch::new(
client.clone(), client.clone(),
cache.to_path_buf(), cache.to_path_buf(),
venv.interpreter_info().clone(), interpreter_info,
fs::canonicalize(venv.python_executable())?, fs::canonicalize(venv.python_executable())?,
no_build, no_build,
); );

View file

@ -128,8 +128,9 @@ pub(crate) async fn sync_requirements(
} else { } else {
let start = std::time::Instant::now(); let start = std::time::Instant::now();
let wheel_finder = puffin_resolver::DistFinder::new(&tags, &client) let wheel_finder =
.with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64)); 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 resolution = wheel_finder.resolve(&remote).await?;
let s = if resolution.len() == 1 { "" } else { "s" }; let s = if resolution.len() == 1 { "" } else { "s" };

View file

@ -630,6 +630,55 @@ fn compile_python_37() -> Result<()> {
Ok(()) Ok(())
} }
/// Test that we select the last 3.8 compatible numpy version instead of trying to compile an
/// incompatible sdist <https://github.com/astral-sh/puffin/issues/388>
#[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. /// Resolve a specific Flask wheel via a URL dependency.
#[test] #[test]
fn compile_wheel_url_dependency() -> Result<()> { fn compile_wheel_url_dependency() -> Result<()> {

View file

@ -2,7 +2,7 @@
use std::process::Command; use std::process::Command;
use anyhow::Result; use anyhow::{Context, Result};
use assert_cmd::prelude::*; use assert_cmd::prelude::*;
use assert_fs::prelude::*; use assert_fs::prelude::*;
use insta_cmd::_macro_support::insta; use insta_cmd::_macro_support::insta;
@ -928,3 +928,50 @@ fn install_version_then_install_url() -> Result<()> {
Ok(()) Ok(())
} }
/// Test that we select the last 3.8 compatible numpy version instead of trying to compile an
/// incompatible sdist <https://github.com/astral-sh/puffin/issues/388>
#[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(())
}

View file

@ -8,9 +8,9 @@ info:
- "--constraint" - "--constraint"
- constraints.txt - constraints.txt
- "--cache-dir" - "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1Ts53o - /tmp/.tmp81zzAa
env: env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1vzBQa/.venv VIRTUAL_ENV: /tmp/.tmpFXMFLd/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -8,9 +8,9 @@ info:
- "--constraint" - "--constraint"
- constraints.txt - constraints.txt
- "--cache-dir" - "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpvjdHOb - /tmp/.tmpuS4Jn4
env: env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpSAaWi3/.venv VIRTUAL_ENV: /tmp/.tmpnFovuD/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -6,9 +6,9 @@ info:
- pip-compile - pip-compile
- pyproject.toml - pyproject.toml
- "--cache-dir" - "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpOOTFwj - /tmp/.tmpWzFzu7
env: env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpU0VXyY/.venv VIRTUAL_ENV: /tmp/.tmpSiRk6d/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -7,9 +7,9 @@ info:
- pyproject.toml - pyproject.toml
- "--all-extras" - "--all-extras"
- "--cache-dir" - "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpFzJKRe - /tmp/.tmpQHVFqr
env: env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpdadcmu/.venv VIRTUAL_ENV: /tmp/.tmpmKXqaF/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -8,9 +8,9 @@ info:
- "--extra" - "--extra"
- foo - foo
- "--cache-dir" - "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpAYEAdM - /tmp/.tmpLjFVr0
env: env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1xuOcV/.venv VIRTUAL_ENV: /tmp/.tmpDT0oRC/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -8,9 +8,9 @@ info:
- "--extra" - "--extra"
- FRiENDlY-...-_-BARd - FRiENDlY-...-_-BARd
- "--cache-dir" - "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpFyLXQn - /tmp/.tmpga6JO1
env: env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpxfZatv/.venv VIRTUAL_ENV: /tmp/.tmpwmJIym/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -8,38 +8,16 @@ info:
- "--python-version" - "--python-version"
- py37 - py37
- "--cache-dir" - "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpzwzUVe - /tmp/.tmpHz7Dc2
env: env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpqFv4YL/.venv VIRTUAL_ENV: /tmp/.tmpoMjDJT/.venv
--- ---
success: true success: false
exit_code: 0 exit_code: 1
----- stdout ----- ----- 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 ----- ----- 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.

View file

@ -6,9 +6,9 @@ info:
- pip-compile - pip-compile
- requirements.in - requirements.in
- "--cache-dir" - "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpxF1zFY - /tmp/.tmpQB4Ze4
env: env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmp33QZdv/.venv VIRTUAL_ENV: /tmp/.tmpSYcMgG/.venv
--- ---
success: true success: true
exit_code: 0 exit_code: 0

View file

@ -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

View file

@ -135,7 +135,7 @@ impl BuildContext for BuildDispatch {
if remote.len() == 1 { "" } else { "s" }, if remote.len() == 1 { "" } else { "s" },
remote.iter().map(ToString::to_string).join(", ") 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) .resolve(&remote)
.await .await
.context("Failed to resolve build dependencies")?; .context("Failed to resolve build dependencies")?;

View file

@ -12,8 +12,8 @@ pub trait Metadata {
/// Return the normalized [`PackageName`] of the distribution. /// Return the normalized [`PackageName`] of the distribution.
fn name(&self) -> &PackageName; fn name(&self) -> &PackageName;
/// Return a [`Version`], for registry-based distributions, or a [`Url`], for URL-based /// Return a [`pep440_rs::Version`], for registry-based distributions, or a [`url::Url`],
/// distributions. /// for URL-based distributions.
fn version_or_url(&self) -> VersionOrUrl; fn version_or_url(&self) -> VersionOrUrl;
/// Returns a unique identifier for the package. /// Returns a unique identifier for the package.

View file

@ -44,6 +44,21 @@ impl InterpreterInfo {
base_prefix: info.base_prefix, 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 { impl InterpreterInfo {
@ -76,6 +91,12 @@ impl InterpreterInfo {
pub fn base_prefix(&self) -> &Path { pub fn base_prefix(&self) -> &Path {
&self.base_prefix &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)] #[derive(Debug, Error)]

View file

@ -21,6 +21,7 @@ puffin-cache = { path = "../puffin-cache" }
puffin-client = { path = "../puffin-client" } puffin-client = { path = "../puffin-client" }
puffin-distribution = { path = "../puffin-distribution" } puffin-distribution = { path = "../puffin-distribution" }
puffin-git = { path = "../puffin-git" } puffin-git = { path = "../puffin-git" }
puffin-interpreter = { path = "../puffin-interpreter" }
puffin-normalize = { path = "../puffin-normalize" } puffin-normalize = { path = "../puffin-normalize" }
puffin-traits = { path = "../puffin-traits" } puffin-traits = { path = "../puffin-traits" }
pypi-types = { path = "../pypi-types" } pypi-types = { path = "../pypi-types" }

View file

@ -14,6 +14,7 @@ use pep508_rs::{Requirement, VersionOrUrl};
use platform_tags::Tags; use platform_tags::Tags;
use puffin_client::RegistryClient; use puffin_client::RegistryClient;
use puffin_distribution::Dist; use puffin_distribution::Dist;
use puffin_interpreter::InterpreterInfo;
use puffin_normalize::PackageName; use puffin_normalize::PackageName;
use pypi_types::{File, SimpleJson}; use pypi_types::{File, SimpleJson};
@ -24,15 +25,21 @@ pub struct DistFinder<'a> {
tags: &'a Tags, tags: &'a Tags,
client: &'a RegistryClient, client: &'a RegistryClient,
reporter: Option<Box<dyn Reporter>>, reporter: Option<Box<dyn Reporter>>,
interpreter_info: &'a InterpreterInfo,
} }
impl<'a> DistFinder<'a> { impl<'a> DistFinder<'a> {
/// Initialize a new distribution finder. /// 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 { Self {
tags, tags,
client, client,
reporter: None, reporter: None,
interpreter_info,
} }
} }
@ -129,6 +136,20 @@ impl<'a> DistFinder<'a> {
fn select(&self, requirement: &Requirement, files: Vec<File>) -> Option<Dist> { fn select(&self, requirement: &Requirement, files: Vec<File>) -> Option<Dist> {
let mut fallback = None; let mut fallback = None;
for file in files.into_iter().rev() { 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 let Ok(wheel) = WheelFilename::from_str(file.filename.as_str()) {
if !wheel.is_compatible(self.tags) { if !wheel.is_compatible(self.tags) {
continue; continue;

View file

@ -536,6 +536,21 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
// distributions. // distributions.
let mut version_map: VersionMap = BTreeMap::new(); let mut version_map: VersionMap = BTreeMap::new();
for file in metadata.files { 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 let Ok(filename) = WheelFilename::from_str(file.filename.as_str()) {
if filename.is_compatible(self.tags) { if filename.is_compatible(self.tags) {
let version = PubGrubVersion::from(filename.version.clone()); let version = PubGrubVersion::from(filename.version.clone());

View file

@ -4,7 +4,7 @@
//! `PyPI` directly. //! `PyPI` directly.
use std::future::Future; use std::future::Future;
use std::path::Path; use std::path::{Path, PathBuf};
use std::pin::Pin; use std::pin::Pin;
use std::str::FromStr; use std::str::FromStr;
@ -17,10 +17,12 @@ use platform_host::{Arch, Os, Platform};
use platform_tags::Tags; use platform_tags::Tags;
use puffin_client::RegistryClientBuilder; use puffin_client::RegistryClientBuilder;
use puffin_interpreter::{InterpreterInfo, Virtualenv}; use puffin_interpreter::{InterpreterInfo, Virtualenv};
use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, Resolver}; use puffin_resolver::{Graph, Manifest, PreReleaseMode, ResolutionMode, Resolver};
use puffin_traits::BuildContext; use puffin_traits::BuildContext;
struct DummyContext; struct DummyContext {
interpreter_info: InterpreterInfo,
}
impl BuildContext for DummyContext { impl BuildContext for DummyContext {
fn cache(&self) -> &Path { fn cache(&self) -> &Path {
@ -28,7 +30,7 @@ impl BuildContext for DummyContext {
} }
fn interpreter_info(&self) -> &InterpreterInfo { fn interpreter_info(&self) -> &InterpreterInfo {
panic!("The test should not need to build source distributions") &self.interpreter_info
} }
fn base_python(&self) -> &Path { fn base_python(&self) -> &Path {
@ -61,13 +63,29 @@ impl BuildContext for DummyContext {
} }
} }
async fn resolve(
manifest: Manifest,
markers: &'static MarkerEnvironment,
tags: &Tags,
) -> Result<Graph> {
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] #[tokio::test]
async fn black() -> Result<()> { async fn black() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![], vec![],
@ -77,8 +95,7 @@ async fn black() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -89,9 +106,6 @@ async fn black() -> Result<()> {
async fn black_colorama() -> Result<()> { async fn black_colorama() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black[colorama]<=23.9.1").unwrap()], vec![Requirement::from_str("black[colorama]<=23.9.1").unwrap()],
vec![], vec![],
@ -101,8 +115,7 @@ async fn black_colorama() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -113,9 +126,6 @@ async fn black_colorama() -> Result<()> {
async fn black_python_310() -> Result<()> { async fn black_python_310() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![], vec![],
@ -125,8 +135,7 @@ async fn black_python_310() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_310, &TAGS_310, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_310, &TAGS_310).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -139,9 +148,6 @@ async fn black_python_310() -> Result<()> {
async fn black_mypy_extensions() -> Result<()> { async fn black_mypy_extensions() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![Requirement::from_str("mypy-extensions<0.4.4").unwrap()], vec![Requirement::from_str("mypy-extensions<0.4.4").unwrap()],
@ -151,8 +157,7 @@ async fn black_mypy_extensions() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -165,9 +170,6 @@ async fn black_mypy_extensions() -> Result<()> {
async fn black_mypy_extensions_extra() -> Result<()> { async fn black_mypy_extensions_extra() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![Requirement::from_str("mypy-extensions[extra]<0.4.4").unwrap()], vec![Requirement::from_str("mypy-extensions[extra]<0.4.4").unwrap()],
@ -177,8 +179,7 @@ async fn black_mypy_extensions_extra() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -191,9 +192,6 @@ async fn black_mypy_extensions_extra() -> Result<()> {
async fn black_flake8() -> Result<()> { async fn black_flake8() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![Requirement::from_str("flake8<1").unwrap()], vec![Requirement::from_str("flake8<1").unwrap()],
@ -203,8 +201,7 @@ async fn black_flake8() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -215,9 +212,6 @@ async fn black_flake8() -> Result<()> {
async fn black_lowest() -> Result<()> { async fn black_lowest() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black>21").unwrap()], vec![Requirement::from_str("black>21").unwrap()],
vec![], vec![],
@ -227,8 +221,7 @@ async fn black_lowest() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -239,9 +232,6 @@ async fn black_lowest() -> Result<()> {
async fn black_lowest_direct() -> Result<()> { async fn black_lowest_direct() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black>21").unwrap()], vec![Requirement::from_str("black>21").unwrap()],
vec![], vec![],
@ -251,8 +241,7 @@ async fn black_lowest_direct() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -263,9 +252,6 @@ async fn black_lowest_direct() -> Result<()> {
async fn black_respect_preference() -> Result<()> { async fn black_respect_preference() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![], vec![],
@ -275,8 +261,7 @@ async fn black_respect_preference() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -287,9 +272,6 @@ async fn black_respect_preference() -> Result<()> {
async fn black_ignore_preference() -> Result<()> { async fn black_ignore_preference() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("black<=23.9.1").unwrap()],
vec![], vec![],
@ -299,8 +281,7 @@ async fn black_ignore_preference() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -311,9 +292,6 @@ async fn black_ignore_preference() -> Result<()> {
async fn black_disallow_prerelease() -> Result<()> { async fn black_disallow_prerelease() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=20.0").unwrap()], vec![Requirement::from_str("black<=20.0").unwrap()],
vec![], vec![],
@ -323,8 +301,9 @@ async fn black_disallow_prerelease() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let err = resolve(manifest, &MARKERS_311, &TAGS_311)
let err = resolver.resolve().await.unwrap_err(); .await
.unwrap_err();
insta::assert_display_snapshot!(err); insta::assert_display_snapshot!(err);
@ -335,9 +314,6 @@ async fn black_disallow_prerelease() -> Result<()> {
async fn black_allow_prerelease_if_necessary() -> Result<()> { async fn black_allow_prerelease_if_necessary() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("black<=20.0").unwrap()], vec![Requirement::from_str("black<=20.0").unwrap()],
vec![], vec![],
@ -347,10 +323,11 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let err = resolve(manifest, &MARKERS_311, &TAGS_311)
let resolution = resolver.resolve().await.unwrap_err(); .await
.unwrap_err();
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(err);
Ok(()) Ok(())
} }
@ -359,9 +336,6 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> {
async fn pylint_disallow_prerelease() -> Result<()> { async fn pylint_disallow_prerelease() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("pylint==2.3.0").unwrap()], vec![Requirement::from_str("pylint==2.3.0").unwrap()],
vec![], vec![],
@ -371,8 +345,7 @@ async fn pylint_disallow_prerelease() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -383,9 +356,6 @@ async fn pylint_disallow_prerelease() -> Result<()> {
async fn pylint_allow_prerelease() -> Result<()> { async fn pylint_allow_prerelease() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![Requirement::from_str("pylint==2.3.0").unwrap()], vec![Requirement::from_str("pylint==2.3.0").unwrap()],
vec![], vec![],
@ -395,8 +365,7 @@ async fn pylint_allow_prerelease() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);
@ -407,9 +376,6 @@ async fn pylint_allow_prerelease() -> Result<()> {
async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> { async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![ vec![
Requirement::from_str("pylint==2.3.0").unwrap(), Requirement::from_str("pylint==2.3.0").unwrap(),
@ -422,8 +388,7 @@ async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); 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<()> { async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> {
colored::control::set_override(false); colored::control::set_override(false);
let tempdir = tempdir()?;
let client = RegistryClientBuilder::new(tempdir.path()).build();
let manifest = Manifest::new( let manifest = Manifest::new(
vec![ vec![
Requirement::from_str("pylint==2.3.0").unwrap(), Requirement::from_str("pylint==2.3.0").unwrap(),
@ -449,8 +411,7 @@ async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> {
None, None,
); );
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); let resolution = resolve(manifest, &MARKERS_311, &TAGS_311).await?;
let resolution = resolver.resolve().await?;
insta::assert_display_snapshot!(resolution); insta::assert_display_snapshot!(resolution);

View file

@ -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<Self, Self::Err> {
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<LenientVersionSpecifiers> for VersionSpecifiers {
fn from(specifiers: LenientVersionSpecifiers) -> Self {
specifiers.0
}
}
impl<'de> Deserialize<'de> for LenientVersionSpecifiers {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
Self::from_str(&s).map_err(de::Error::custom)
}
}

View file

@ -1,7 +1,9 @@
pub use direct_url::{ArchiveInfo, DirectUrl, VcsInfo, VcsKind}; pub use direct_url::{ArchiveInfo, DirectUrl, VcsInfo, VcsKind};
pub use lenient_requirement::LenientVersionSpecifiers;
pub use metadata::{Error, Metadata21}; pub use metadata::{Error, Metadata21};
pub use simple_json::{File, SimpleJson}; pub use simple_json::{File, SimpleJson};
mod direct_url; mod direct_url;
mod lenient_requirement;
mod metadata; mod metadata;
mod simple_json; mod simple_json;

View file

@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
use thiserror::Error; use thiserror::Error;
use tracing::warn; use tracing::warn;
use crate::lenient_requirement::LenientVersionSpecifiers;
use pep440_rs::{Pep440Error, Version, VersionSpecifiers}; use pep440_rs::{Pep440Error, Version, VersionSpecifiers};
use pep508_rs::{Pep508Error, Requirement}; use pep508_rs::{Pep508Error, Requirement};
use puffin_normalize::{ExtraName, InvalidNameError, PackageName}; use puffin_normalize::{ExtraName, InvalidNameError, PackageName};
@ -271,54 +272,6 @@ impl From<LenientRequirement> 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<Self, Self::Err> {
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<LenientVersionSpecifiers> for VersionSpecifiers {
fn from(specifiers: LenientVersionSpecifiers) -> Self {
specifiers.0
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::str::FromStr; use std::str::FromStr;

View file

@ -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)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SimpleJson { pub struct SimpleJson {
@ -15,13 +19,30 @@ pub struct File {
pub data_dist_info_metadata: Metadata, pub data_dist_info_metadata: Metadata,
pub filename: String, pub filename: String,
pub hashes: Hashes, pub hashes: Hashes,
pub requires_python: Option<String>, /// 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<VersionSpecifiers>,
pub size: usize, pub size: usize,
pub upload_time: String, pub upload_time: String,
pub url: String, pub url: String,
pub yanked: Yanked, pub yanked: Yanked,
} }
fn deserialize_version_specifiers_lenient<'de, D>(
deserializer: D,
) -> Result<Option<VersionSpecifiers>, D::Error>
where
D: Deserializer<'de>,
{
let maybe_string: Option<String> = 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)] #[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(untagged)] #[serde(untagged)]
pub enum Metadata { pub enum Metadata {