GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-81881: Raise a shutil.SpecialFileError when copying a Unix socket by AWhetter · Pull Request #16575 · python/cpython

Conversation

@AWhetter

@AWhetter AWhetter commented Oct 4, 2019 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This used to raise an OSError with a platform dependent message.
This change always raises a SpecialFileError with a consistent message
no matter the platform.

Tackling special devices wasn't part of the original issue, but it is still technically an issue. Although devices are technically copyable (at least they are with cp), they will likely hang the same as a named pipe would even though a named pipe is technically copyable as well.

https://bugs.python.org/issue37700

  • Issue: shutil.copyfile does not raise SpecialFileError for socket files #81881

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
chrahunt and arhadthedev reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@AWhetter
bpo-37700: Raise a shutil.SpecialFileError when copying a Unix socket
b0011a1 
This used to raise an OSError with a platform dependent message.
This change always raises a SpecialFileError with a consistent message
no matter the platform.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 4, 2019
@bedevere-bot bedevere-bot added the awaiting review label Oct 4, 2019
@brandtbucher

brandtbucher commented Oct 4, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Thanks for the PR @AWhetter!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brandtbucher brandtbucher added the type-feature A feature request or enhancement label Oct 4, 2019
brandtbucher
brandtbucher requested changes Oct 4, 2019
View reviewed changes

@brandtbucher brandtbucher left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This looks good to me! I would maybe just change the NEWS entry to be less wordy:

:func:`shutil.copyfile` always raises a :exc:`shutil.SpecialFileError` when trying to copy a Unix socket.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Oct 4, 2019
@brandtbucher

brandtbucher commented Oct 4, 2019

Copy link
Copy Markdown
Member

CC @giampaolo @cjerdonek
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AWhetter

AWhetter commented Oct 4, 2019

Copy link
Copy Markdown
Contributor Author

Good idea. I'll get that changed.
When I make changes to a pull request, is it better for me to edit my existing commit, or should I make a new commit for squashing when the branch is merged?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brandtbucher

brandtbucher commented Oct 4, 2019

Copy link
Copy Markdown
Member

I think it helps everyone follow the flow of the PR when things aren't force pushed and history isn't rewritten. It will get squashed in the end, anyways!

I just break up my commits so that the individual diffs are easy to follow. So if I'm editing and moving a function, I'll do those in separate commits in order to preserve an easy-to-follow storyline.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AWhetter
fixup! bpo-37700: Raise a shutil.SpecialFileError when copying a Unix…
5fd5dc6 
… socket
brandtbucher
brandtbucher approved these changes Oct 4, 2019
View reviewed changes
@chrahunt

chrahunt commented Dec 12, 2019

Copy link
Copy Markdown
Contributor

Similar to the issue mentioned in bpo-37701, this raises SpecialFileError when src or dest are symlinks to a socket file.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
giampaolo
giampaolo requested changes Dec 25, 2019
View reviewed changes
Comment thread Lib/test/test_shutil.py Outdated
else:
self.fail("shutil.Error should have been raised")
finally:
shutil.rmtree(TESTFN2, ignore_errors=True)

@giampaolo giampaolo Dec 25, 2019

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This seems unnecessary as per:

 self.addCleanup(shutil.rmtree, TESTFN, ignore_errors=True)

...some lines above.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_shutil.py
shutil.rmtree(TESTFN2, ignore_errors=True)

@unittest.skipUnless(hasattr(socket, 'AF_UNIX'), 'requires socket.AF_UNIX')
def test_copytree_socket(self):

@giampaolo giampaolo Dec 25, 2019

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

IMO testing also copytree() is not very useful since internally it uses copyfile (which you are testing).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@AWhetter AWhetter Dec 30, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I was copying what was done for named pipes for consistency. But having a test for copytree() makes sense to me because if in the future copytree() stopped using copyfile() for some reason, we would still want copytree() to raise an error?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/shutil.rst Outdated
*dst* and return *dst* in the most efficient way possible.
*src* and *dst* are path-like objects or path names given as strings.

When *src* is a named pipe or a Unix socket, a :exc:`SpecialFileError`

@giampaolo giampaolo Dec 25, 2019

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

-   When *src* is a named pipe or a Unix socket, a :exc:`SpecialFileError`
+   When *src* is a named pipe or a Unix socket, :exc:`SpecialFileError`

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting core review labels Dec 25, 2019
@bedevere-bot

bedevere-bot commented Dec 25, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AWhetter
fixup! bpo-37700: Raise a shutil.SpecialFileError when copying a Unix…
5c4fec9 
… socket
@AWhetter

AWhetter commented May 18, 2020

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels May 18, 2020
@bedevere-bot

bedevere-bot commented May 18, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

@giampaolo: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from giampaolo May 18, 2020 23:02
@AWhetter

AWhetter commented Jul 12, 2020

Copy link
Copy Markdown
Contributor Author

Is there anything else that I can do to help progress this review?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@oz123

oz123 commented Mar 31, 2023

Copy link
Copy Markdown
Contributor

@giampaolo can this PR be unblocked?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev
Merge branch 'main' into fix-issue-37700
f28afb8
@arhadthedev arhadthedev changed the title bpo-37700: Raise a shutil.SpecialFileError when copying a Unix socket gh-81881: Raise a shutil.SpecialFileError when copying a Unix socket Mar 31, 2023
@arhadthedev arhadthedev added the stdlib Standard Library Python modules in the Lib/ directory label Mar 31, 2023
@arhadthedev
Update the versionchanged note
69c3260
@oz123

oz123 commented Mar 31, 2023

Copy link
Copy Markdown
Contributor

@arhadthedev thanks for the quick response!
arhadthedev reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev
Fix GitHub Actions/Docs warnings
15e3a52
@arhadthedev

arhadthedev commented Mar 31, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

shutil.SpecialFileError seems to be undocumented. Would anybody add it into the documentation via a separate PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AWhetter AWhetter closed this Jul 10, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Dec 14, 2025
shutil.copyfile does not raise SpecialFileError for socket files #81881
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandtbucher brandtbucher brandtbucher approved these changes
@giampaolo giampaolo Awaiting requested review from giampaolo giampaolo is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting change review stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@AWhetter @brandtbucher @chrahunt @bedevere-bot @oz123 @arhadthedev @giampaolo @the-knights-who-say-ni @ezio-melotti
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.