gh-117467: Add preserving of mailbox owner on flush #117510
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was a problem hiding this comment.
Since it is a user visible change, it needs a NEWS entry. You can read above how to create it.
Could you please also add a test? It should create a mailbox, change its user and group, modify it, and check that its user and group are preserved. This test can only be run as root, but it is better than nothing.
Lib/mailbox.py
Outdated
| os.chmod(new_file.name, info.st_mode) | ||
| try: | ||
| os.chown(new_file.name, info.st_uid, info.st_gid) | ||
| except: |
There was a problem hiding this comment.
Silently ignoring arbitrary exceptions is never a good idea. KeyboardInterrupt, MemoryError and RecursionError can be raised by virtually any code.
It is better to make the exception check more specific, for example: (PermissionError, AttributeError).
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Done. Hope it's ok.
As a novice contributor, I will have to work out how to add a test. Is there any tutorial? |
|
!buildbot BSD |
|
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit c8eb82c 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
https://devguide.python.org/testing/run-write-tests/ may help. Usually you find an appropriate file in the |
|
!buildbot root |
|
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit c8eb82c 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
That is brilliant, thank you very much indeed! Although I've programmed for decades, Python is still relatively new to me. |
c8eb82c to
c78971c
Compare
|
Thank you for your contribution @softins. We usually do not use rebase and force push. It makes iterative reviewing more difficult. All consequent changes are just stacked one over other during review, and squashed into a single changeset right before merging. |
|
Thanks @softins for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
|
GH-117537 is a backport of this pull request to the 3.12 branch. |
Ok, thank you. I'll remember that for any future contributions. |
…17510) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Fixes #117467.
Copy owner and group from the old file to the new when rewriting a mailbox. Ignore any failure from
os.chown(), which may be platform-dependent.