Filter out yanked files (#413)

Implement two behaviors for yanked versions:

* During `pip-compile`, yanked versions are filtered out entirely, we
currently treat them is if they don't exist. This is leads to confusing
error messages because a version that does exist seems to have suddenly
disappeared.
* During `pip-sync`, we warn when we fetch a remote distribution and it
has been yanked. We currently don't warn on cached or installed
distributions that have been yanked.
This commit is contained in:
konsti 2023-11-13 21:58:50 +01:00 committed by GitHub
parent 28ec4e79f0
commit bacf1dc911
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 158 additions and 16 deletions

View file

@ -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::<Vec<_>>()
};
// 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![]

View file

@ -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(())
}

View file

@ -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(())
}

View file

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

View file

@ -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(&registry.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(&registry.file),
SourceDist::DirectUrl(_) | SourceDist::Git(_) => None,
}
}
}
impl Metadata for RegistryBuiltDist {

View file

@ -72,3 +72,14 @@ impl From<DistFile> 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,
}
}
}

View file

@ -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());

View file

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

View file

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