diff --git a/crates/puffin-cli/src/commands/pip_sync.rs b/crates/puffin-cli/src/commands/pip_sync.rs index 7a4ccaa8d..5cc5d300f 100644 --- a/crates/puffin-cli/src/commands/pip_sync.rs +++ b/crates/puffin-cli/src/commands/pip_sync.rs @@ -3,10 +3,10 @@ use std::path::Path; use anyhow::{Context, Result}; use colored::Colorize; +use fs_err as fs; use itertools::{Either, Itertools}; use tracing::debug; -use fs_err as fs; use install_wheel_rs::linker::LinkMode; use pep508_rs::Requirement; use platform_host::Platform; @@ -16,6 +16,7 @@ use puffin_dispatch::BuildDispatch; use puffin_distribution::{AnyDist, Metadata}; use puffin_installer::{Builder, InstallPlan}; use puffin_interpreter::Virtualenv; +use pypi_types::Yanked; use crate::commands::reporters::{ BuildReporter, DownloadReporter, FinderReporter, InstallReporter, UnzipReporter, @@ -148,6 +149,33 @@ pub(crate) async fn sync_requirements( resolution.into_distributions().collect::>() }; + // TODO(konstin): Also check the cache whether any cached or installed dist is already known to + // have been yanked, we currently don't show this message on the second run anymore + for dist in &remote { + let Some(file) = dist.file() else { + continue; + }; + match &file.yanked { + Yanked::Bool(false) => {} + Yanked::Bool(true) => { + writeln!( + printer, + "{}{} {dist} is yanked. Refresh your lockfile to pin an un-yanked version.", + "warning".yellow().bold(), + ":".bold(), + )?; + } + Yanked::Reason(reason) => { + writeln!( + printer, + "{}{} {dist} is yanked (reason: \"{reason}\"). Refresh your lockfile to pin an un-yanked version.", + "warning".yellow().bold(), + ":".bold(), + )?; + } + } + } + // Download any missing distributions. let downloads = if remote.is_empty() { vec![] diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index a3716e7fe..77ddf57c9 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -653,19 +653,19 @@ fn compile_numpy_py38() -> Result<()> { 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###" + 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 ----- @@ -674,7 +674,7 @@ fn compile_numpy_py38() -> Result<()> { error: Failed to build distribution: numpy-1.24.4.tar.gz Caused by: Building source distributions is disabled "###); - }); + }); Ok(()) } diff --git a/crates/puffin-cli/tests/pip_sync.rs b/crates/puffin-cli/tests/pip_sync.rs index 001fd730e..817e3442c 100644 --- a/crates/puffin-cli/tests/pip_sync.rs +++ b/crates/puffin-cli/tests/pip_sync.rs @@ -975,3 +975,39 @@ fn install_numpy_py38() -> Result<()> { Ok(()) } + +#[test] +fn warn_on_yanked_version() -> 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.txt"); + requirements_in.touch()?; + // This version is yanked + requirements_in.write_str("ipython==8.13.0")?; + + 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)); + }); + + Ok(()) +} diff --git a/crates/puffin-cli/tests/snapshots/pip_sync__warn_on_yanked_version.snap b/crates/puffin-cli/tests/snapshots/pip_sync__warn_on_yanked_version.snap new file mode 100644 index 000000000..2e0fdf829 --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_sync__warn_on_yanked_version.snap @@ -0,0 +1,24 @@ +--- +source: crates/puffin-cli/tests/pip_sync.rs +info: + program: puffin + args: + - pip-sync + - requirements.txt + - "--cache-dir" + - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpfXTdj6 + env: + VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpK8X9JT/.venv +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +Resolved 1 package in [TIME] +warning: ipython==8.13.0 is yanked (reason: "Wrong Python Pinning"). Refresh your lockfile to pin an un-yanked version. +Downloaded 1 package in [TIME] +Unzipped 1 package in [TIME] +Installed 1 package in [TIME] + + ipython==8.13.0 + diff --git a/crates/puffin-distribution/src/lib.rs b/crates/puffin-distribution/src/lib.rs index 3e9096d86..58aa7eaf6 100644 --- a/crates/puffin-distribution/src/lib.rs +++ b/crates/puffin-distribution/src/lib.rs @@ -127,6 +127,34 @@ impl Dist { Self::Source(SourceDist::DirectUrl(DirectUrlSourceDist { name, url })) } } + + /// Returns the [`File`] instance, if this dist is from a registry with simple json api support + pub fn file(&self) -> Option<&File> { + match self { + Dist::Built(built) => built.file(), + Dist::Source(source) => source.file(), + } + } +} + +impl BuiltDist { + /// Returns the [`File`] instance, if this dist is from a registry with simple json api support + pub fn file(&self) -> Option<&File> { + match self { + BuiltDist::Registry(registry) => Some(®istry.file), + BuiltDist::DirectUrl(_) => None, + } + } +} + +impl SourceDist { + /// Returns the [`File`] instance, if this dist is from a registry with simple json api support + pub fn file(&self) -> Option<&File> { + match self { + SourceDist::Registry(registry) => Some(®istry.file), + SourceDist::DirectUrl(_) | SourceDist::Git(_) => None, + } + } } impl Metadata for RegistryBuiltDist { diff --git a/crates/puffin-resolver/src/file.rs b/crates/puffin-resolver/src/file.rs index 64249244a..e2097173e 100644 --- a/crates/puffin-resolver/src/file.rs +++ b/crates/puffin-resolver/src/file.rs @@ -72,3 +72,14 @@ impl From for File { } } } + +impl Deref for DistFile { + type Target = File; + + fn deref(&self) -> &Self::Target { + match self { + DistFile::Wheel(file) => &file.0, + DistFile::Sdist(file) => &file.0, + } + } +} diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 17aceb395..832e2eddb 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -551,6 +551,12 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { { continue; } + // When resolving, exclude yanked files. TODO(konstin): When we fail + // resolving due to a dependency locked to yanked version, we should tell + // the user. + if file.yanked.is_yanked() { + 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/pypi-types/src/lib.rs b/crates/pypi-types/src/lib.rs index 6fccd1475..e5b7ffd1a 100644 --- a/crates/pypi-types/src/lib.rs +++ b/crates/pypi-types/src/lib.rs @@ -1,7 +1,7 @@ pub use direct_url::{ArchiveInfo, DirectUrl, VcsInfo, VcsKind}; pub use lenient_requirement::LenientVersionSpecifiers; pub use metadata::{Error, Metadata21}; -pub use simple_json::{File, SimpleJson}; +pub use simple_json::{File, SimpleJson, Yanked}; mod direct_url; mod lenient_requirement; diff --git a/crates/pypi-types/src/simple_json.rs b/crates/pypi-types/src/simple_json.rs index 56138b8f3..f30cad16e 100644 --- a/crates/pypi-types/src/simple_json.rs +++ b/crates/pypi-types/src/simple_json.rs @@ -66,6 +66,15 @@ pub enum Yanked { Reason(String), } +impl Yanked { + pub fn is_yanked(&self) -> bool { + match self { + Yanked::Bool(is_yanked) => *is_yanked, + Yanked::Reason(_) => true, + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Hashes { pub sha256: String,