Enforce modification freshness checks against virtual environment (#959)

## Summary

This PR is like #957, but for validating the virtual environment, rather
than the cache. So, if you have a local wheel, and you rebuild it, we'll
now correctly uninstall and reinstall it in the virtual environment.
This commit is contained in:
Charlie Marsh 2024-01-18 15:21:16 -05:00 committed by GitHub
parent 96a61fb351
commit a883de4fb0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 65 additions and 8 deletions

View File

@ -1174,6 +1174,39 @@ fn install_local_wheel() -> Result<()> {
check_command(&venv, "import tomli", &temp_dir);
// "Modify" the wheel.
let archive_file = std::fs::File::open(&archive)?;
archive_file.set_modified(std::time::SystemTime::now())?;
// Reinstall into the same virtual environment. The wheel should be reinstalled.
insta::with_settings!({
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip")
.arg("sync")
.arg("requirements.txt")
.arg("--strict")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.current_dir(&temp_dir), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- tomli==2.0.1 (from file://[TEMP_DIR]/tomli-2.0.1-py3-none-any.whl)
+ tomli==2.0.1 (from file://[TEMP_DIR]/tomli-2.0.1-py3-none-any.whl)
"###);
});
check_command(&venv, "import tomli", &temp_dir);
Ok(())
}

View File

@ -7,8 +7,8 @@ use rustc_hash::FxHashSet;
use tracing::{debug, warn};
use distribution_types::{
git_reference, BuiltDist, CachedDirectUrlDist, CachedDist, Dist, IndexLocations, InstalledDist,
Name, SourceDist,
git_reference, BuiltDist, CachedDirectUrlDist, CachedDist, Dist, IndexLocations,
InstalledDirectUrlDist, InstalledDist, Name, SourceDist,
};
use pep508_rs::{Requirement, VersionOrUrl};
use platform_tags::Tags;
@ -151,10 +151,18 @@ impl<'a> Planner<'a> {
// If the requirement comes from a direct URL, check by URL.
Some(VersionOrUrl::Url(url)) => {
if let InstalledDist::Url(distribution) = &distribution {
// TODO(charlie): Check freshness for path dependencies.
if &distribution.url == url.raw() {
debug!("Requirement already satisfied: {distribution}");
continue;
// If the requirement came from a local path, check freshness.
if let Ok(archive) = url.to_file_path() {
if is_fresh_install(distribution, &archive)? {
debug!("Requirement already satisfied (and up-to-date): {distribution}");
continue;
}
} else {
// Otherwise, assume the requirement is up-to-date.
debug!("Requirement already satisfied (assumed up-to-date): {distribution}");
continue;
}
}
}
}
@ -244,7 +252,7 @@ impl<'a> Planner<'a> {
wheel.filename.stem(),
);
if is_fresh(&cache_entry, &wheel.path)? {
if is_fresh_cache(&cache_entry, &wheel.path)? {
let cached_dist = CachedDirectUrlDist::from_url(
wheel.filename,
wheel.url,
@ -281,7 +289,7 @@ impl<'a> Planner<'a> {
);
if let Some(wheel) = BuiltWheelIndex::find(&cache_shard, tags) {
if is_fresh(&wheel.entry, &sdist.path)? {
if is_fresh_cache(&wheel.entry, &sdist.path)? {
let cached_dist = wheel.into_url_dist(url.clone());
debug!("Path source requirement already cached: {cached_dist}");
local.push(CachedDist::Url(cached_dist));
@ -346,7 +354,7 @@ impl<'a> Planner<'a> {
///
/// A cache entry is considered fresh if it exists and is newer than the file at the given path.
/// If the cache entry is stale, it will be removed from the cache.
fn is_fresh(cache_entry: &CacheEntry, artifact: &Path) -> Result<bool, io::Error> {
fn is_fresh_cache(cache_entry: &CacheEntry, artifact: &Path) -> Result<bool, io::Error> {
match fs_err::metadata(cache_entry.path()).and_then(|metadata| metadata.modified()) {
Ok(cache_mtime) => {
// Determine the modification time of the wheel.
@ -375,6 +383,22 @@ fn is_fresh(cache_entry: &CacheEntry, artifact: &Path) -> Result<bool, io::Error
}
}
/// Returns `true` if the installed distribution linked to the file at the given [`Path`] is fresh,
/// based on the modification time of the installed distribution.
fn is_fresh_install(dist: &InstalledDirectUrlDist, artifact: &Path) -> Result<bool, io::Error> {
// Determine the modification time of the installed distribution.
let dist_metadata = fs_err::metadata(&dist.path)?;
let dist_mtime = dist_metadata.modified()?;
// Determine the modification time of the wheel.
let Some(artifact_mtime) = puffin_cache::archive_mtime(artifact)? else {
// The artifact doesn't exist, so it's not fresh.
return Ok(false);
};
Ok(dist_mtime >= artifact_mtime)
}
#[derive(Debug, Default)]
pub struct Plan {
/// The distributions that are not already installed in the current environment, but are