gh-127651: Use file in diagnostics if origin is missing#127660

hauntsaninja · 2024-12-06T01:05:56Z

See the left hand side in https://github.com/python/cpython/pull/123929/files#diff-c22186367cbe20233e843261998dc027ae5f1f8c0d2e778abfa454ae74cc59deL2840-L2849

I missed this in part because I use debug builds for dev which I think don't freeze modules

Issue: Regression of 3.13.1 for module paths in from import #127651

See the left hand side in https://github.com/python/cpython/pull/123929/files#diff-c22186367cbe20233e843261998dc027ae5f1f8c0d2e778abfa454ae74cc59deL2840-L2849 I missed this in part because I use debug builds for dev which I think don't freeze modules

Misc/NEWS.d/next/Core_and_Builtins/2024-12-06-01-09-40.gh-issue-127651.80cm6j.rst

Python/ceval.c

Lib/test/test_import/__init__.py

bedevere-bot · 2024-12-09T08:10:05Z

🤖 New build scheduled with the buildbot fleet by @hauntsaninja for commit 9a3dbbf 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

hauntsaninja · 2024-12-09T08:11:20Z

(testing with buildbots because last time I made a similar change we needed a fix for iOS)

serhiy-storchaka

There is yet one bug in this code. If errmsg is NULL, Py_DECREF(errmsg) (or even _PyErr_SetImportErrorWithNameFrom) can crash.

Lib/test/test_import/__init__.py

Python/ceval.c

Lib/test/test_import/__init__.py

Python/ceval.c

Lib/test/test_import/__init__.py

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka

LGTM.

The code is so complicated that I am not sure that all cases are covered. But in any case this is an improvement.

hauntsaninja · 2024-12-10T04:55:06Z

Thank you for the review!

miss-islington-app · 2024-12-10T04:55:24Z

Thanks @hauntsaninja for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2024-12-10T04:55:36Z

Sorry, @hauntsaninja, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 3983527c3a6b389e373a233e514919555853ccb3 3.13

…thon#127660) See the left hand side in https://github.com/python/cpython/pull/123929/files#diff-c22186367cbe20233e843261998dc027ae5f1f8c0d2e778abfa454ae74cc59deL2840-L2849 --------- Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 3983527)

bedevere-app · 2024-12-10T04:58:10Z

GH-127775 is a backport of this pull request to the 3.13 branch.

…127660) (#127775) gh-127651: Use __file__ in diagnostics if origin is missing (#127660) See the left hand side in https://github.com/python/cpython/pull/123929/files#diff-c22186367cbe20233e843261998dc027ae5f1f8c0d2e778abfa454ae74cc59deL2840-L2849 --------- Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 3983527)

bedevere-bot · 2024-12-10T06:12:20Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 MacOS M1 Refleaks NoGIL 3.x has failed when building commit 3983527.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1368/builds/2283) and take a look at the build logs.
Check if the failure is related to this commit (3983527) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1368/builds/2283

Failed tests:

test.test_concurrent_futures.test_thread_pool

Failed subtests:

test_free_reference - test.test_concurrent_futures.test_thread_pool.ThreadPoolExecutorTest.test_free_reference

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last): File "/Users/ec2-user/buildbot/buildarea/3.x.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/test/test_concurrent_futures/executor.py", line 132, in test_free_referenceself.assertIsNone(wr()) ~~~~~~~~~~~~~~~~~^^^^^^AssertionError: <test.test_concurrent_futures.executor.MyObject object at 0x200061c0160> is not None Traceback (most recent call last): File "/Users/ec2-user/buildbot/buildarea/3.x.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/test/test_concurrent_futures/executor.py", line 132, in test_free_referenceself.assertIsNone(wr()) ~~~~~~~~~~~~~~~~~^^^^^^AssertionError: <test.test_concurrent_futures.executor.MyObject object at 0x200061c0090> is not None

…module I missed the extra `PyModule_Check` in python#127660 because I was looking at 3.12 as the base implementation for import from. This meant that I missed the `PyModuleCheck` introduced in python#112661.

…#128047) I missed the extra `PyModule_Check` in #127660 because I was looking at 3.12 as the base implementation for import from. This meant that I missed the `PyModuleCheck` introduced in #112661.

…module (pythonGH-128047) I missed the extra `PyModule_Check` in pythonGH-127660 because I was looking at 3.12 as the base implementation for import from. This meant that I missed the `PyModuleCheck` introduced in pythonGH-112661. (cherry picked from commit 45e6dd6) Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>

…-module (GH-128047) (#128114) gh-128030: Avoid error from PyModule_GetFilenameObject for non-module (GH-128047) I missed the extra `PyModule_Check` in GH-127660 because I was looking at 3.12 as the base implementation for import from. This meant that I missed the `PyModuleCheck` introduced in GH-112661. (cherry picked from commit 45e6dd6) Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>

…module (python#128047) I missed the extra `PyModule_Check` in python#127660 because I was looking at 3.12 as the base implementation for import from. This meant that I missed the `PyModuleCheck` introduced in python#112661.

…thon#127660) See the left hand side in https://github.com/python/cpython/pull/123929/files#diff-c22186367cbe20233e843261998dc027ae5f1f8c0d2e778abfa454ae74cc59deL2840-L2849 --------- Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…module (python#128047) I missed the extra `PyModule_Check` in python#127660 because I was looking at 3.12 as the base implementation for import from. This meant that I missed the `PyModuleCheck` introduced in python#112661.

hauntsaninja requested review from brettcannon, ericsnowcurrently, markshannon, ncoghlan and warsaw as code owners December 6, 2024 01:05

bedevere-appbot mentioned this pull request Dec 6, 2024
Regression of 3.13.1 for module paths in from import #127651
Closed

bedevere-appbot added the awaiting core review label Dec 6, 2024

hauntsaninja added needs backport to 3.13 bugs and security fixes and removed awaiting core review labels Dec 6, 2024

📜🤖 Added by blurb_it.
69be54a

picnixz reviewed Dec 6, 2024
View reviewed changes

Misc/NEWS.d/next/Core_and_Builtins/2024-12-06-01-09-40.gh-issue-127651.80cm6j.rst Outdated Show resolvedHide resolved

fix news entry
e5bab4c

serhiy-storchaka reviewed Dec 6, 2024
View reviewed changes

Python/ceval.c Outdated Show resolvedHide resolved
Python/ceval.c Outdated Show resolvedHide resolved
Lib/test/test_import/__init__.py Outdated Show resolvedHide resolved

hauntsaninja added 3 commits December 8, 2024 23:29

use os.__file__ in test
0688e77

add test for error path
7c30a5d

add test for missing __file__
9a3dbbf

hauntsaninja added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 9, 2024

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 9, 2024

serhiy-storchaka reviewed Dec 9, 2024
View reviewed changes

hauntsaninja added 6 commits December 9, 2024 01:21

use mortal objects
33e3fb6

remove regex use
fb1d381

remove tmp dir
247565e

move call earlier
2bd25a8

test with sys
8a5f76a

errmsg null check
c80120e

serhiy-storchaka reviewed Dec 9, 2024
View reviewed changes

Python/ceval.c Outdated Show resolvedHide resolved
Lib/test/test_import/__init__.py Outdated Show resolvedHide resolved

improve test
e8da759

hauntsaninjaand others added 2 commits December 9, 2024 02:23

Update Python/ceval.c
d5aa69c
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

more origin test
560a23c

serhiy-storchaka approved these changes Dec 9, 2024
View reviewed changes

bedevere-appbot added the awaiting merge label Dec 9, 2024

hauntsaninja merged commit 3983527 into python:mainDec 10, 2024
39 checks passed

bedevere-appbot removed the awaiting merge label Dec 10, 2024

miss-islington-appbot assigned hauntsaninja Dec 10, 2024

hauntsaninja deleted the gh-127651 branch December 10, 2024 04:55

bedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Dec 10, 2024

hauntsaninja mentioned this pull request Dec 17, 2024
gh-128030: Avoid error from PyModule_GetFilenameObject for non-module #128047
Merged

miss-islington mentioned this pull request Dec 20, 2024
[3.13] gh-128030: Avoid error from PyModule_GetFilenameObject for non-module (GH-128047) #128114
Merged

Uh oh!

gh-127651: Use __file__ in diagnostics if origin is missing#127660

gh-127651: Use __file__ in diagnostics if origin is missing #127660

Uh oh!

Conversation

hauntsaninja commented Dec 6, 2024• edited by bedevere-app botLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Dec 9, 2024

Uh oh!

hauntsaninja commented Dec 9, 2024

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

hauntsaninja commented Dec 10, 2024

Uh oh!

Uh oh!

miss-islington-appbot commented Dec 10, 2024

Uh oh!

miss-islington-appbot commented Dec 10, 2024

Uh oh!

bedevere-appbot commented Dec 10, 2024

Uh oh!

bedevere-bot commented Dec 10, 2024

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gh-127651: Use file in diagnostics if origin is missing#127660

gh-127651: Use file in diagnostics if origin is missing #127660

hauntsaninja commented Dec 6, 2024•
edited by bedevere-app bot
Loading