Skip to content

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

Merged
gpshead merged 3 commits intopython:mainfrom
akulakov:43219-Shutil-copy-fix-is-a-dir-errors
Jul 10, 2021
Merged

bpo-43219: shutil.copyfile, raise a less confusing exception instead of IsADirectoryError#27049
gpshead merged 3 commits intopython:mainfrom
akulakov:43219-Shutil-copy-fix-is-a-dir-errors

Conversation

@akulakov
Copy link
Copy Markdown
Contributor

@akulakov akulakov commented Jul 7, 2021 •
edited by bedevere-bot
Loading

@akulakov akulakov marked this pull request as draft July 7, 2021 02:30
@akulakov akulakov marked this pull request as ready for review July 8, 2021 13:42
@akulakov
Copy link
Copy Markdown
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
Copy Markdown

bedevere-bot commented Jul 9, 2021

🤖 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
Copy Markdown
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
Copy Markdown

bedevere-bot commented Jul 10, 2021

🤖 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 reviewed Jul 10, 2021
View reviewed changes
@gpshead gpshead added needs backport to 3.8 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
Copy Markdown
Contributor

miss-islington commented Jul 10, 2021

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
@akulakov @miss-islington
bpo-43219: shutil.copyfile, raise a less confusing exception instead …
c264d9d 
…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
Copy Markdown

bedevere-bot commented Jul 10, 2021

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
@akulakov @miss-islington
bpo-43219: shutil.copyfile, raise a less confusing exception instead …
274e595 
…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
Copy Markdown

bedevere-bot commented Jul 10, 2021

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

miss-islington added a commit that referenced this pull request Jul 10, 2021
@miss-islington @akulakov
bpo-43219: shutil.copyfile, raise a less confusing exception instead …
1577259 
…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
@miss-islington
[3.9] bpo-43219: shutil.copyfile, raise a less confusing exception in…
c89f0b2 
…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
Copy Markdown
Contributor Author

akulakov commented Jul 10, 2021

@gpshead Thanks for the review :)

@pablogsal pablogsal mentioned this pull request Jul 18, 2021
Closed
@Flamefire
Copy link
Copy Markdown

Flamefire commented Sep 17, 2021

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

@akulakov
Copy link
Copy Markdown
Contributor Author

akulakov commented Sep 17, 2021

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
Copy Markdown

Flamefire commented Sep 17, 2021

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
Copy Markdown
Contributor Author

akulakov commented Sep 17, 2021

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

@Flamefire
Copy link
Copy Markdown

Flamefire commented Sep 17, 2021

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

Reviewers

@gpshead gpshead gpshead approved these changes

Assignees

@gpshead gpshead

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@akulakov @bedevere-bot @miss-islington @Flamefire @gpshead @the-knights-who-say-ni