Skip to content

bpo-43219: shutil.copyfile, raise a less confusing exception instead of IsADirectoryError #27049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 10, 2021

Conversation

akulakov
Copy link
Contributor

@akulakov akulakov commented Jul 7, 2021

@akulakov akulakov marked this pull request as ready for review July 8, 2021 13:42
@akulakov
Copy link
Contributor Author

akulakov commented Jul 8, 2021

Note that before merging, this should be run with builds on other OSes like Aix, FreeBSD, etc.

@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 9, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 624093b 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 9, 2021
@gpshead gpshead self-assigned this Jul 9, 2021
@akulakov
Copy link
Contributor Author

akulakov commented Jul 9, 2021

For reviewing: note that the code block wrapped in try/except is unchanged except for indentation.

@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @gpshead for commit b2a681b 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2021
@gpshead gpshead added needs backport to 3.8 needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes type-bug An unexpected behavior, bug, or error and removed needs backport to 3.8 labels Jul 10, 2021
@gpshead gpshead merged commit 248173c into python:main Jul 10, 2021
@miss-islington
Copy link
Contributor

Thanks @akulakov for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2021
…of IsADirectoryError (pythonGH-27049)

Fixes the misleading IsADirectoryError to be FileNotFoundError.
(cherry picked from commit 248173c)

Co-authored-by: andrei kulakov <andrei.avk@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 10, 2021
@bedevere-bot
Copy link

GH-27081 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2021
…of IsADirectoryError (pythonGH-27049)

Fixes the misleading IsADirectoryError to be FileNotFoundError.
(cherry picked from commit 248173c)

Co-authored-by: andrei kulakov <andrei.avk@gmail.com>
@bedevere-bot
Copy link

GH-27082 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Jul 10, 2021
miss-islington added a commit that referenced this pull request Jul 10, 2021
…of IsADirectoryError (GH-27049)

Fixes the misleading IsADirectoryError to be FileNotFoundError.
(cherry picked from commit 248173c)

Co-authored-by: andrei kulakov <andrei.avk@gmail.com>
miss-islington added a commit that referenced this pull request Jul 10, 2021
…stead of IsADirectoryError (GH-27049) (GH-27082)

Fixes the misleading IsADirectoryError to be FileNotFoundError.
(cherry picked from commit 248173c)


Co-authored-by: andrei kulakov <andrei.avk@gmail.com>

Automerge-Triggered-By: GH:gpshead
@akulakov akulakov deleted the 43219-Shutil-copy-fix-is-a-dir-errors branch July 10, 2021 04:51
@akulakov
Copy link
Contributor Author

@gpshead Thanks for the review :)

@Flamefire
Copy link

This now raises a wrong FileNotFoundError when the src is a directory because only the dst existance is checked.

@akulakov
Copy link
Contributor Author

This now raises a wrong FileNotFoundError when the src is a directory because only the dst existance is checked.

FileNotFoundError is a common exception for "file or directory does not exist". If you feel it's wrongly raised in some scenario, please open a new issue on bugs.python.org and add an example and expected / observed output.

@Flamefire
Copy link

But the path exists. It only is a directory. This PR changed the correct IsADirectoryError to a now incorrect FileNotFoundError for that case

@akulakov
Copy link
Contributor Author

@Flamefire I think you're right, I can reproduce it. I will reopen the issue.

@Flamefire
Copy link

I opened https://bugs.python.org/issue45234 but mentioned the new failure in 43219

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants