Fix ensure_empty_dir leaving directory symlinks behind - #300
Open
Str0k wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ensure_empty_dir in pyproject_api/_util.py used rmtree(sub_path, ignore_errors=True) for every entry that is_dir(). When an entry is a symlink pointing to a directory, rmtree raises OSError('Cannot call rmtree on a symbolic link'), which ignore_errors=True swallows, so the symlink is silently left in place and the directory is not emptied. The callers, e.g. _check_metadata_dir in _frontend.py:348, rely on it to 'start with fresh' before prepare_metadata_for_build_wheel/editable, so a stale symlink in the metadata directory survives and the subsequent mkdir(exist_ok=True) does not restore a truly fresh state. After the fix the rmtree call is guarded with 'and not sub_path.is_symlink()', so symlinks to directories are removed via unlink like any other non-directory entry, without following or traversing the link target.
shutil.rmtree documents that calling it on a symlink raises OSError; combined with ignore_errors=True this turned a hard error into a silent no-op, violating the function's documented contract of leaving the path empty. Trigger: run prepare_metadata_for_build_editable twice with the same metadata_directory that contains a symlink to a directory; on the second run the directory is not emptied. The fix is one guard clause in the existing branch, keeping rmtree for real directories and unlink for everything else, including symlinks; the regression test asserts both that the link is removed and that its target's contents are untouched.
Validation:
AI assistance: implementation and independent review used Hermes with GLM 5.3. Test evidence was reproduced in clean checkouts. This does not represent a human review.
An additional model reviewed the supplied patch and evidence without executing tools.