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_reference
    self.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_reference
    self.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, ncoghlan, warsaw and markshannon as code owners December 6, 2024 01:05

bedevere-app bot mentioned this pull request Dec 6, 2024

Regression of 3.13.1 for module paths in from import #127651

Closed

bedevere-app bot 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 resolved Hide resolved

fix news entry

e5bab4c

serhiy-storchaka reviewed Dec 6, 2024

View reviewed changes

Python/ceval.c Outdated Show resolved Hide resolved

Python/ceval.c Outdated Show resolved Hide resolved

Lib/test/test_import/__init__.py Outdated Show resolved Hide 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 resolved Hide resolved

Lib/test/test_import/__init__.py Outdated Show resolved Hide resolved

improve test

e8da759

hauntsaninja and 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-app bot added the awaiting merge label Dec 9, 2024

hauntsaninja merged commit 3983527 into python:main Dec 10, 2024
43 checks passed

bedevere-app bot removed the awaiting merge label Dec 10, 2024

miss-islington-app bot assigned hauntsaninja Dec 10, 2024

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

bedevere-app bot 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

bedevere-bot commented Dec 9, 2024

hauntsaninja commented Dec 9, 2024

serhiy-storchaka left a comment

serhiy-storchaka left a comment

hauntsaninja commented Dec 10, 2024

miss-islington-app bot commented Dec 10, 2024

miss-islington-app bot commented Dec 10, 2024

bedevere-app bot commented Dec 10, 2024

bedevere-bot commented Dec 10, 2024

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

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

Conversation

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

bedevere-bot commented Dec 9, 2024

hauntsaninja commented Dec 9, 2024

serhiy-storchaka left a comment

Choose a reason for hiding this comment

serhiy-storchaka left a comment

Choose a reason for hiding this comment

hauntsaninja commented Dec 10, 2024

miss-islington-app bot commented Dec 10, 2024

miss-islington-app bot commented Dec 10, 2024

bedevere-app bot commented Dec 10, 2024

bedevere-bot commented Dec 10, 2024

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

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