Skip to content

Fail renaming a file to or from a path with a trailing separator - #1344

Open
kratos0718 wants to merge 1 commit into
pytest-dev:mainfrom
kratos0718:rename-trailing-sep
Open

kratos0718 wants to merge 1 commit into
pytest-dev:mainfrom
kratos0718:rename-trailing-sep

Conversation

@kratos0718

Copy link
Copy Markdown

Describe the changes

Under Posix, rename() dropped the trailing separator in absnormpath before checking anything, so for a regular file foo:

  • os.rename("foo", "bar/") renamed the file to bar (the real call fails and leaves foo in place)
  • os.rename("foo", "foo/") succeeded as a no-op
  • os.rename("foo/", "bar") renamed the file

A trailing separator requires a directory. When the source is not a directory and the parent of the target exists, Linux raises ENOTDIR, while macOS raises ENOENT for a missing target, EISDIR for an existing directory and ENOTDIR otherwise. A trailing separator on a regular source file raises ENOTDIR on both. Windows is unchanged.

I ran the new tests with TEST_REAL_FS=1 on macOS and as a non-root user on Linux (Debian, python 3.13), and they fail without the change. The existing trailing-separator tests for symlinks and directories still pass.

Tasks

  • Unit tests added that reproduce the issue or prove feature is working
  • Fix or feature added
  • Entry to release notes added
  • Pre-commit CI shows no errors
  • Unit tests passing
  • For documentation changes: The Read the Docs preview builds and looks as expected

Under Posix, os.rename("file", "new/") renamed the file and
os.rename("file/", "new") succeeded, because the trailing separator was
dropped by absnormpath before any check. A trailing separator requires a
directory: Linux raises ENOTDIR, macOS ENOENT, ENOTDIR or EISDIR depending
on the target.
@mrbean-bremen

Copy link
Copy Markdown
Member

Thanks! I'm traveling right now, will review when I get back (in a bit over a week).

@davidlbaird

Copy link
Copy Markdown
Collaborator

fake_filesystem_vs_real_test.py should also be updated to validate the fake_filesystem behavior against os on the host system.

@davidlbaird

Copy link
Copy Markdown
Collaborator

Please also create an issue, that will be referenced in CHANGES.md for the next release.

This branch has not been deployed

No deployments
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.

3 participants