Skip to content
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

bpo-41843: Reenable use of sendfile in shutil module on Solaris #23893

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

kulikjak
Copy link
Contributor

@kulikjak kulikjak commented Dec 22, 2020

With the integration of #22040, os.sendfile now works as expected and hence its use in shutil as a fast-copy syscall for file copies should be reenabled (it was disabled with #13675 due to small differences in offset handling and return values).

I am not sure what exactly to write into shutil.rst as if backported, the versionchanged:: 3.10 part would not be correct, but it wasn't there from the beginning of 3.9 either, and it seems that micro versions are not specified there (or maybe this is not a change that should be backported) ??

Also, I enabled the Solaris here explicitly, but maybe reverting the #13675 (and by doing so enabling the use of sendfile on every system that has it available, which might actually be just Linux and Solaris...) is a better option.

https://bugs.python.org/issue41843

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 22, 2021
@kulikjak
Copy link
Contributor Author

No activity, but still relevant...

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Jan 23, 2021
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 23, 2021
@kulikjak
Copy link
Contributor Author

This is still relevant.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Feb 24, 2021
@kulikjak kulikjak force-pushed the reenable-Solaris-sendfile branch from c0eff8f to b9b0797 Compare May 21, 2021 10:21
@kulikjak
Copy link
Contributor Author

I rebased my changes onto changes introduced with #26024.

Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

This looks correct to me.

@@ -465,6 +465,9 @@ file then shutil will silently fallback on using less efficient

.. versionchanged:: 3.8

.. versionchanged:: 3.10
Copy link
Member

Choose a reason for hiding this comment

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

Should now be changed to 3.11.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, it's fixed now. Thanks.

@kulikjak kulikjak force-pushed the reenable-Solaris-sendfile branch 2 times, most recently from 4f658dd to 5aee88a Compare August 19, 2022 10:29
@iritkatriel iritkatriel added 3.12 bugs and security fixes and removed 3.12 bugs and security fixes labels Sep 10, 2022
kulikjak added a commit to kulikjak/cpython that referenced this pull request Mar 20, 2023
@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Mar 22, 2023
@arhadthedev
Copy link
Member

arhadthedev commented Mar 22, 2023

@giampaolo (as an shutil expert), @jcea (as a Solaris platform expert)

kulikjak added a commit to kulikjak/cpython that referenced this pull request May 9, 2023
@kulikjak kulikjak force-pushed the reenable-Solaris-sendfile branch from 46b9f2b to 1a3a1b4 Compare November 7, 2023 10:13
@kulikjak kulikjak force-pushed the reenable-Solaris-sendfile branch from 1a3a1b4 to 944c17e Compare September 18, 2024 13:24
Doc/library/shutil.rst Outdated Show resolved Hide resolved
@kulikjak
Copy link
Contributor Author

I am sorry, I just realized that when rebasing my changes, I accidentally used 'solaris' rather than 'sunos' in the platform check. I created #124289 to fix this.

savannahostrowski pushed a commit to savannahostrowski/cpython that referenced this pull request Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants