gh-75723: Idempotent .pth execution in site.addsitedir#147951
Conversation
|
Do you have any ideas how to explain the test failure? cpython/Lib/test/test_xmlrpc.py Lines 479 to 489 in 4810bed |
|
🤖 New build scheduled with the buildbot fleet by @FFY00 for commit 6c342df 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F147951%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
|
The |
|
Well, the test is still failing. It seems like the I am gonna sync with main, just to be sure this is not a known issue. |
FFY00
left a comment
There was a problem hiding this comment.
This looks okay, overall. There is just the test_xmlrpc failure we need to fix, and the news text that should be updated.
|
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 |
|
@FFY00 After looking into the code for a bit, this is the best test refinement I have come up with: def test_default(self):
# Patch BOTH time and localtime inside the client module
with mock.patch('xmlrpc.client.time.time') as time_mock, \
mock.patch('xmlrpc.client.time.localtime') as localtime_mock:
# 1. Force the 'current' time to be a specific float
time_mock.return_value = 1373847889.0
# 2. Force the conversion to be your specific struct
time_struct = time.struct_time([2013, 7, 15, 0, 24, 49, 0, 196, 0])
localtime_mock.return_value = time_struct
t = xmlrpclib.DateTime()
self.assertEqual(str(t), "20130715T00:24:49") |
|
I think this can be simplified as follows: @mock.patch('xmlrpc.client.time.time', return_value=1373847889.0)
@mock.patch(
'xmlrpc.client.time.localtime',
return_value=time.struct_time([2013, 7, 15, 0, 24, 49, 0, 196, 0]),
)
def test_default(self, *_):
t = xmlrpclib.DateTime()
self.assertEqual(str(t), "20130715T00:24:49") |
|
I didn't expect the Spanish Inquisition |
|
Nobody expects the Spanish Inquisition! @FFY00: please review the changes made to this pull request. |
| PROG = f'''\ | ||
| if {self.imported!r} in sys.modules: | ||
| open({self.idempotent_fail_path!r}, 'a+').close() | ||
| ''' | ||
| print(f"import sys; exec({PROG!r})", file=FILE) |
There was a problem hiding this comment.
Consider this (zero effort because dedent is already imported)
| PROG = f'''\ | |
| if {self.imported!r} in sys.modules: | |
| open({self.idempotent_fail_path!r}, 'a+').close() | |
| ''' | |
| print(f"import sys; exec({PROG!r})", file=FILE) | |
| PROG = dedent(f'''\ | |
| if {self.imported!r} in sys.modules: | |
| open({self.idempotent_fail_path!r}, 'a+').close() | |
| ''') | |
| print(f"import sys; exec({PROG!r})", file=FILE) |
|
I'm adding backport labels, but take them off if you see any room for surprise in the implications of this change and we want to sell this as a feature |
|
It looks like the macOS failure is an instance of #149156 |
|
@warsaw @brettcannon this also fixes the same double call bug with the new pep 829 implementation |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
I want to fix and test this differently in Python 3.15, but this looks pretty good for 3.14 and 3.13. Once I land that in Thanks for both finding and fixing this bug! |
…9583) * Idempotent `.pth` execution in `site.addsitedir` * potentially fix monkeypatch leak? * fix blind copy paste of recommendation * Update 2026-03-31-16-15-15.gh-issue-75723.BZ4Rsn.rst * fix implicit merge conflict with 24c4aec * Add failing tests for gh-75723 Based on @asottle branch !asottle-gh-75723 but refactored for `main`. This will need a different backport. * Repair gh-75723 The original fix is here: #147951 but I'm refactoring a bit for `main`. * Refactor _make_mod() so we can use it to create package modules too * Add myself to CODEOWNERS for the site module --------- Co-authored-by: anthony sottile <asottile@umich.edu> Co-authored-by: Filipe Laíns <lains@riseup.net>
Uh oh!
There was an error while loading. Please reload this page.