Skip to content

Uninstall files recorded by legacy egg metadata - #251

Closed
zaniebot wants to merge 1 commit into
zb/bughunt200/148-entry-points-parserfrom
zb/bughunt200/148-uninstall-egg-info-files
Closed

Uninstall files recorded by legacy egg metadata#251
zaniebot wants to merge 1 commit into
zb/bughunt200/148-entry-points-parserfrom
zb/bughunt200/148-uninstall-egg-info-files

Conversation

@zaniebot

@zaniebot zaniebot commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Legacy egg-info uninstall consults only top_level.txt, ignoring installed-files.txt and entry_points.txt, so executable and data files that pip removes remain installed. Prefer the contained recorded-file list over recursive top-level removal (preserving shared namespaces), remove matching bytecode and console/GUI launchers without validating irrelevant target syntax, and protect core environment files while preflighting deletion. This retains the egg-uninstall safety work in uv#19114 and uv#19340 and follows pip's legacy uninstall.

Moved to: astral-sh/uv-dev#223.

Comment on lines +390 to +404
let entry_points = match fs_err::read_to_string(egg_info.join("entry_points.txt")) {
Ok(entry_points) => Some(entry_points),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => None,
Err(err) => return Err(err.into()),
};
if let Some(entry_points) = entry_points {
let (console_scripts, gui_scripts) =
scripts_from_ini(None, layout.python_version.1, entry_points)?;
for (script, is_gui) in console_scripts
.into_iter()
.map(|script| (script, false))
.chain(gui_scripts.into_iter().map(|script| (script, true)))
{
let validated = ValidatedScript::try_from_script(&script, layout)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a dedicated entry points parser? Can we add a struct that does so?

@zaniebot zaniebot Jul 15, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: extracted an EntryPoints reader that owns the INI parsing and missing-file behavior, and reused it for both wheel install and legacy uninstall.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also preflighted all entry-point parsing and launcher validation before the first filesystem mutation, with a regression ensuring malformed metadata cannot leave a partial uninstall.

Comment on lines +276 to +288
let installed_files = match fs_err::read_to_string(egg_info.join("installed-files.txt")) {
Ok(installed_files) => Some(installed_files),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => None,
Err(err) => return Err(err.into()),
};
if let Some(installed_files) = installed_files.as_deref() {
let schemes = [
layout.scheme.data.as_path(),
layout.scheme.purelib.as_path(),
layout.scheme.platlib.as_path(),
layout.scheme.scripts.as_path(),
layout.scheme.include.as_path(),
];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a dedicated installed-files.txt parser? Should we add one?

@zaniebot zaniebot Jul 15, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: added a dedicated InstalledFiles parser for legacy metadata, including whitespace/CRLF and missing-file coverage, so uninstall no longer parses the file inline.

@zaniebot
zaniebot force-pushed the zb/bughunt200/148-uninstall-egg-info-files branch 2 times, most recently from fc8d7c4 to 64235dc Compare July 15, 2026 15:46
@zanieb

zanieb commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Let's stage refactors separately from the fixes and stack the branches

@zaniebot
zaniebot force-pushed the zb/bughunt200/148-uninstall-egg-info-files branch from 64235dc to b861972 Compare July 15, 2026 15:56
@zaniebot
zaniebot changed the base branch from main to zb/bughunt200/148-entry-points-parser July 15, 2026 15:56
@zaniebot

Copy link
Copy Markdown
Owner Author

Done — I split the behavior-neutral EntryPoints reader/refactor into #255 and stacked this fix on it. This PR now contains only the legacy-uninstall behavior, the InstalledFiles parser and consumer, containment/preflight handling, and its regressions; the final tree is unchanged.

@zaniebot
zaniebot force-pushed the zb/bughunt200/148-uninstall-egg-info-files branch from b861972 to 9dc9972 Compare July 15, 2026 17:41
@zaniebot zaniebot closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants