gh-127651: Use file in diagnostics if origin is missing by hauntsaninja · Pull Request #127660 · python/cpython · GitHub

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

🤖 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.

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

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

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

LGTM.

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

Thank you for the review!

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

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)

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)

⚠️⚠️⚠️ 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, markshannon, ncoghlan and warsaw 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 only security fixes and removed awaiting core review labels Dec 6, 2024

📜🤖 Added by blurb_it.

69be54a

picnixz reviewed Dec 6, 2024

View reviewed changes

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

fix news entry

e5bab4c

serhiy-storchaka reviewed Dec 6, 2024

View reviewed changes

Comment thread Python/ceval.c Outdated

Comment thread Python/ceval.c Outdated

Comment thread Lib/test/test_import/__init__.py Outdated

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

Comment thread Python/ceval.c Outdated

Comment thread Lib/test/test_import/__init__.py Outdated

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

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 only 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

gh-127651: Use __file__ in diagnostics if origin is missing by hauntsaninja · Pull Request #127660 · python/cpython · GitHub

Repository navigation

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

gh-127651: Use __file__ in diagnostics if origin is missing#127660hauntsaninja merged 15 commits intopython:mainpython/cpython:mainfrom hauntsaninja:gh-127651hauntsaninja/cpython:gh-127651Copy head branch name to clipboard

Conversation

hauntsaninja commented Dec 6, 2024 • edited by bedevere-app Bot Loading 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!

miss-islington-app Bot commented Dec 10, 2024

Uh oh!

miss-islington-app Bot commented Dec 10, 2024

Uh oh!

bedevere-app Bot commented Dec 10, 2024

Uh oh!

bedevere-bot commented Dec 10, 2024

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

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gh-127651: Use file in diagnostics if origin is missing by hauntsaninja · Pull Request #127660 · python/cpython · GitHub

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

gh-127651: Use file in diagnostics if origin is missing#127660
hauntsaninja merged 15 commits into
python:mainfrom
hauntsaninja:gh-127651

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