mirror of
https://github.com/astral-sh/uv.git
synced 2026-05-06 08:56:53 -04:00
Prevent uninstalling site-packages for empty top_level.txt in .egg-info (#19114)
uv pip uninstall `<pkg>` against a legacy` .egg-info` distribution with an empty` top_level.txt` could resolve "" back to the distribution location itself and end up deleting site-packages. In `uninstall_egg` crate, each line from `top_level.txt `is now trimmed and empty entries are skipped before iterating. Applied the same trim + non-empty handling to `namespace_packages.txt` so both files are parsed consistently. I did not also change is_path_in_scheme to reject paths equal to a scheme root, since that is a broader defense-in-depth change and outside the scope of this fix. Covered by `test_uninstall_egg_info_empty_top_level` next to the existing `test_uninstall_egg_info_path_traversal`, plus `test_uninstall_egg_info_blank_lines_in_top_level` for blank and whitespace-only lines between valid entries, which hits the same wipe path before the fix. It fails on main with uninstall must not remove site-packages itself and passes with this fix. Existing `uninstall_egg_info`, `dry_run_uninstall_egg_info`, and `uninstall_legacy_editable` integration tests still pass. Closes #19113. --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
committed by
GitHub
parent
3ba8059174
commit
fe7137f253
@@ -225,12 +225,14 @@ pub fn uninstall_egg(
|
||||
.parent()
|
||||
.expect("egg-info directory is not in a site-packages directory");
|
||||
|
||||
// Read the `namespace_packages.txt` file.
|
||||
// Read the `namespace_packages.txt` file, skipping empty or whitespace-only entries.
|
||||
let namespace_packages = {
|
||||
let namespace_packages_path = egg_info.join("namespace_packages.txt");
|
||||
match fs_err::read_to_string(namespace_packages_path) {
|
||||
Ok(namespace_packages) => namespace_packages
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|line| !line.is_empty())
|
||||
.map(ToString::to_string)
|
||||
.collect::<Vec<_>>(),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
@@ -241,13 +243,21 @@ pub fn uninstall_egg(
|
||||
};
|
||||
|
||||
// Read the `top_level.txt` file, ignoring anything in `namespace_packages.txt`.
|
||||
//
|
||||
// Empty or whitespace-only entries are skipped: legacy setuptools writes `top_level.txt`
|
||||
// with a trailing newline even when the package has no top-level modules, which
|
||||
// `str::lines` yields as an empty string. Joining that onto `dist_location` would
|
||||
// resolve back to `dist_location` itself (site-packages), and a subsequent
|
||||
// `remove_dir_all` would wipe out every installed package.
|
||||
let top_level = {
|
||||
let top_level_path = egg_info.join("top_level.txt");
|
||||
match fs_err::read_to_string(&top_level_path) {
|
||||
Ok(top_level) => top_level
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|line| !line.is_empty())
|
||||
.filter(|line| !namespace_packages.iter().any(|ns| ns.as_str() == *line))
|
||||
.map(ToString::to_string)
|
||||
.filter(|line| !namespace_packages.contains(line))
|
||||
.collect::<Vec<_>>(),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
return Err(Error::MissingTopLevel(top_level_path));
|
||||
@@ -535,4 +545,113 @@ mod tests {
|
||||
assert!(!init_py.exists());
|
||||
assert!(!egg_info.exists());
|
||||
}
|
||||
|
||||
/// Regression test for <https://github.com/astral-sh/uv/issues/19113>.
|
||||
///
|
||||
/// Legacy setuptools writes a `top_level.txt` that contains just a newline when the
|
||||
/// distribution has no top-level modules. Previously, [`uninstall_egg`] parsed that as a
|
||||
/// single empty entry, joined it onto `site-packages`, and called `remove_dir_all` on the
|
||||
/// result, wiping out every other package in the environment. Uninstalling such a package
|
||||
/// must leave its siblings untouched.
|
||||
#[test]
|
||||
fn test_uninstall_egg_info_empty_top_level() {
|
||||
let venv = assert_fs::TempDir::new().unwrap();
|
||||
let site_packages = venv.child("lib/python3.12/site-packages");
|
||||
site_packages.create_dir_all().unwrap();
|
||||
|
||||
// A sibling package that must survive the uninstall.
|
||||
let sibling_init = site_packages.child("sibling").child("__init__.py");
|
||||
sibling_init.touch().unwrap();
|
||||
let sibling_dist_info = site_packages.child("sibling-1.0.0.dist-info");
|
||||
sibling_dist_info.create_dir_all().unwrap();
|
||||
|
||||
// The egg-info for the package we're uninstalling, with a `top_level.txt` that
|
||||
// contains only a newline (as legacy setuptools writes for an empty package).
|
||||
let egg_info = site_packages.child("emptypkg-0.1.0.egg-info");
|
||||
egg_info.create_dir_all().unwrap();
|
||||
egg_info.child("top_level.txt").write_str("\n").unwrap();
|
||||
|
||||
let layout = Layout {
|
||||
sys_executable: venv.path().join("bin/python"),
|
||||
python_version: (3, 13),
|
||||
os_name: "posix".to_string(),
|
||||
scheme: Scheme {
|
||||
purelib: site_packages.to_path_buf(),
|
||||
platlib: site_packages.to_path_buf(),
|
||||
scripts: venv.path().join("bin"),
|
||||
data: venv.path().to_path_buf(),
|
||||
include: venv.path().join("include/python3.12"),
|
||||
},
|
||||
};
|
||||
|
||||
uninstall_egg(egg_info.path(), "emptypkg 0.1.0", &layout).unwrap();
|
||||
|
||||
// The egg-info is gone, but the rest of site-packages (including the sibling
|
||||
// package) survives.
|
||||
assert!(!egg_info.exists());
|
||||
assert!(
|
||||
site_packages.exists(),
|
||||
"uninstall must not remove site-packages itself"
|
||||
);
|
||||
assert!(sibling_init.exists(), "sibling package must not be removed");
|
||||
assert!(
|
||||
sibling_dist_info.exists(),
|
||||
"sibling dist-info must not be removed"
|
||||
);
|
||||
}
|
||||
|
||||
/// Same bug shape as #19113, but triggered by a blank or whitespace-only line embedded
|
||||
/// between valid entries in `top_level.txt`. Exercises the filter in combination with
|
||||
/// real entries to make sure they're still honored after skipping empties.
|
||||
#[test]
|
||||
fn test_uninstall_egg_info_blank_lines_in_top_level() {
|
||||
let venv = assert_fs::TempDir::new().unwrap();
|
||||
let site_packages = venv.child("lib/python3.12/site-packages");
|
||||
site_packages.create_dir_all().unwrap();
|
||||
|
||||
// A sibling package that must survive.
|
||||
let sibling_init = site_packages.child("sibling").child("__init__.py");
|
||||
sibling_init.touch().unwrap();
|
||||
|
||||
// Two real top-level modules that should be removed.
|
||||
let pkg_a_init = site_packages.child("pkg_a").child("__init__.py");
|
||||
pkg_a_init.touch().unwrap();
|
||||
let pkg_b_init = site_packages.child("pkg_b").child("__init__.py");
|
||||
pkg_b_init.touch().unwrap();
|
||||
|
||||
// `top_level.txt` with a leading blank line, a whitespace-only line between the two
|
||||
// valid entries, a trailing blank line, and `\r\n` line endings mixed in.
|
||||
let egg_info = site_packages.child("mixedpkg-0.1.0.egg-info");
|
||||
egg_info.create_dir_all().unwrap();
|
||||
egg_info
|
||||
.child("top_level.txt")
|
||||
.write_str("\npkg_a\n \r\npkg_b\n\n")
|
||||
.unwrap();
|
||||
|
||||
let layout = Layout {
|
||||
sys_executable: venv.path().join("bin/python"),
|
||||
python_version: (3, 13),
|
||||
os_name: "posix".to_string(),
|
||||
scheme: Scheme {
|
||||
purelib: site_packages.to_path_buf(),
|
||||
platlib: site_packages.to_path_buf(),
|
||||
scripts: venv.path().join("bin"),
|
||||
data: venv.path().to_path_buf(),
|
||||
include: venv.path().join("include/python3.12"),
|
||||
},
|
||||
};
|
||||
|
||||
uninstall_egg(egg_info.path(), "mixedpkg 0.1.0", &layout).unwrap();
|
||||
|
||||
// The two named packages are gone, the egg-info is gone, and site-packages plus
|
||||
// the sibling survive.
|
||||
assert!(!egg_info.exists());
|
||||
assert!(!pkg_a_init.exists(), "pkg_a must be removed");
|
||||
assert!(!pkg_b_init.exists(), "pkg_b must be removed");
|
||||
assert!(
|
||||
site_packages.exists(),
|
||||
"uninstall must not remove site-packages itself"
|
||||
);
|
||||
assert!(sibling_init.exists(), "sibling package must not be removed");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user