NotificationsYou must be signed in to change notification settings
Fork33.3k
Star69.7k

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

hauntsaninja merged 15 commits intopython:mainfromhauntsaninja:gh-127651

Dec 10, 2024

Merged

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

hauntsaninja merged 15 commits intopython:mainfromhauntsaninja:gh-127651

Dec 10, 2024

Conversation

Copy link

Contributor

hauntsaninja commentedDec 6, 2024•
edited by bedevere-appbot
Loading

See the left hand side inhttps://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

pythongh-127651: Use __file__ in diagnostics if origin is missing

363567c

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

hauntsaninja requested review frombrettcannon,ericsnowcurrently,markshannon,ncoghlan andwarsaw ascode 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.13bugs 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 OutdatedShow resolvedHide resolved

fix news entry

e5bab4c

serhiy-storchaka reviewed

Dec 6, 2024

View reviewed changes

Python/ceval.c OutdatedShow resolvedHide resolved

Lib/test/test_import/__init__.py OutdatedShow resolvedHide resolved

hauntsaninja added3 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-buildbotsTest PR w/ buildbots; report in status section label

Dec 9, 2024

Copy link

bedevere-bot commentedDec 9, 2024

🤖 New build scheduled with the buildbot fleet by@hauntsaninja for commit9a3dbbf 🤖

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

bedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

Dec 9, 2024

Copy link

ContributorAuthor

hauntsaninja commentedDec 9, 2024

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

serhiy-storchaka reviewed

Dec 9, 2024

View reviewed changes

Copy link

Member

serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

There is yet one bug in this code. Iferrmsg is NULL,Py_DECREF(errmsg) (or even_PyErr_SetImportErrorWithNameFrom) can crash.

Lib/test/test_import/__init__.py OutdatedShow resolvedHide resolved

Lib/test/test_import/__init__.pyShow resolvedHide resolved

Python/ceval.c OutdatedShow resolvedHide resolved

Lib/test/test_import/__init__.py OutdatedShow resolvedHide resolved

Python/ceval.c OutdatedShow resolvedHide resolved

hauntsaninja added6 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 OutdatedShow resolvedHide resolved

Lib/test/test_import/__init__.py OutdatedShow resolvedHide resolved

improve test

e8da759

hauntsaninjaand others added2 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

Copy link

Member

serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM.

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

bedevere-appbot added the awaiting merge label

Dec 9, 2024

Copy link

ContributorAuthor

hauntsaninja commentedDec 10, 2024

Thank you for the review!

hauntsaninja merged commit3983527 intopython:main

Dec 10, 2024

43 checks passed

bedevere-appbot removed the awaiting merge label

Dec 10, 2024

Copy link

miss-islington-appbot commentedDec 10, 2024

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

Copy link

miss-islington-appbot commentedDec 10, 2024

Sorry,@hauntsaninja, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 3983527c3a6b389e373a233e514919555853ccb3 3.13

miss-islington-appbot assignedhauntsaninja

Dec 10, 2024

hauntsaninja deleted the gh-127651 branch

December 10, 2024 04:55

hauntsaninja added a commit to hauntsaninja/cpython that referenced this pull request

Dec 10, 2024

pythongh-127651: Use __file__ in diagnostics if origin is missing (py…

7427012

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

Copy link

bedevere-appbot commentedDec 10, 2024

GH-127775 is a backport of this pull request to the3.13 branch.

bedevere-appbot removed the needs backport to 3.13bugs and security fixes label

Dec 10, 2024

hauntsaninja added a commit that referenced this pull request

Dec 10, 2024

[3.13]gh-127651: Use __file__ in diagnostics if origin is missing (#…

6441d42

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

Copy link

bedevere-bot commentedDec 10, 2024

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

Hi! The buildbotARM64 MacOS M1 Refleaks NoGIL 3.x has failed when building commit3983527.

What do you need to do:

Don't panic.
Checkthe 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", line132, intest_free_referenceself.assertIsNone(wr())~~~~~~~~~~~~~~~~~^^^^^^AssertionError:<test.test_concurrent_futures.executor.MyObject object at 0x200061c0160> is not NoneTraceback (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", line132, intest_free_referenceself.assertIsNone(wr())~~~~~~~~~~~~~~~~~^^^^^^AssertionError:<test.test_concurrent_futures.executor.MyObject object at 0x200061c0090> is not None

hauntsaninja added a commit to hauntsaninja/cpython that referenced this pull request

Dec 17, 2024

pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…

256e3c3

…moduleI missed the extra `PyModule_Check` inpython#127660 because I was looking at3.12 as the base implementation for import from. This meant that Imissed the `PyModuleCheck` introduced inpython#112661.

hauntsaninja mentioned this pull request

Dec 17, 2024

gh-128030: Avoid error from PyModule_GetFilenameObject for non-module#128047

Merged

hauntsaninja added a commit that referenced this pull request

Dec 20, 2024

gh-128030: Avoid error from PyModule_GetFilenameObject for non-module (…

45e6dd6

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Dec 20, 2024

pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…

76b88e6

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

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

hauntsaninja added a commit that referenced this pull request

Dec 20, 2024

[3.13]gh-128030: Avoid error from PyModule_GetFilenameObject for non…

f320f74

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

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request

Dec 23, 2024

pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…

a548261

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

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request

Jan 8, 2025

pythongh-127651: Use __file__ in diagnostics if origin is missing (py…

f93cc07

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

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request

Jan 8, 2025

pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…

628b0d2

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