GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-127651: Use __file__ in diagnostics if origin is missing by hauntsaninja · Pull Request #127660 · python/cpython

Conversation

@hauntsaninja

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@hauntsaninja
pythongh-127651: Use __file__ in diagnostics if origin is missing
363567c 
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
@hauntsaninja
hauntsaninja requested review from brettcannon, ericsnowcurrently, markshannon, ncoghlan and warsaw as code owners December 6, 2024 01:05
@bedevere-app 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 bedevere-app Bot added the awaiting core review label Dec 6, 2024
@hauntsaninja hauntsaninja added needs backport to 3.13 only security fixes and removed awaiting core review labels Dec 6, 2024
@blurb-it
📜🤖 Added by blurb_it.
69be54a
picnixz
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 Resolved

Uh oh!

There was an error while loading. Please reload this page.
@hauntsaninja
fix news entry
e5bab4c
serhiy-storchaka
serhiy-storchaka reviewed Dec 6, 2024
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
hauntsaninja added 3 commits December 8, 2024 23:29
@hauntsaninja
use os.__file__ in test
0688e77
@hauntsaninja
add test for error path
7c30a5d
@hauntsaninja
add test for missing __file__
9a3dbbf
@hauntsaninja hauntsaninja added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 9, 2024
@bedevere-bot

bedevere-bot commented Dec 9, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 9, 2024
@hauntsaninja

hauntsaninja commented Dec 9, 2024

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Dec 9, 2024
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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. If errmsg is NULL, Py_DECREF(errmsg) (or even _PyErr_SetImportErrorWithNameFrom) can crash.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hauntsaninja reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
hauntsaninja added 6 commits December 9, 2024 01:21
@hauntsaninja
use mortal objects
33e3fb6
@hauntsaninja
remove regex use
fb1d381
@hauntsaninja
remove tmp dir
247565e
@hauntsaninja
move call earlier
2bd25a8
@hauntsaninja
test with sys
8a5f76a
@hauntsaninja
errmsg null check
c80120e
serhiy-storchaka
serhiy-storchaka reviewed Dec 9, 2024
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@hauntsaninja
improve test
e8da759
hauntsaninja and others added 2 commits December 9, 2024 02:23
@hauntsaninja @serhiy-storchaka
Update Python/ceval.c
d5aa69c 
Co-authored-by: Serhiy Storchaka <[email protected]>
@hauntsaninja
more origin test
560a23c
serhiy-storchaka
serhiy-storchaka approved these changes Dec 9, 2024
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added the awaiting merge label Dec 9, 2024
@hauntsaninja

hauntsaninja commented Dec 10, 2024

Copy link
Copy Markdown
Contributor Author

Thank you for the review!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hauntsaninja
hauntsaninja merged commit 3983527 into python:main Dec 10, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Dec 10, 2024
@miss-islington-app

miss-islington-app Bot commented Dec 10, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Dec 10, 2024

Copy link
Copy Markdown

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned hauntsaninja Dec 10, 2024
@hauntsaninja
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
@hauntsaninja @serhiy-storchaka
pythongh-127651: Use __file__ in diagnostics if origin is missing (py…
7427012 
…thon#127660)

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

---------

Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 3983527)
@bedevere-app

bedevere-app Bot commented Dec 10, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Dec 10, 2024
hauntsaninja added a commit that referenced this pull request Dec 10, 2024
@hauntsaninja @serhiy-storchaka
[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 in https://github.com/python/cpython/pull/123929/files#diff-c22186367cbe20233e843261998dc027ae5f1f8c0d2e778abfa454ae74cc59deL2840-L2849

---------

Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 3983527)
@bedevere-bot

bedevere-bot commented Dec 10, 2024

Copy link
Copy Markdown

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

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

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (3983527) or if it is a false positive.
  5. 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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hauntsaninja added a commit to hauntsaninja/cpython that referenced this pull request Dec 17, 2024
@hauntsaninja
pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…
256e3c3 
…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.
@hauntsaninja 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
@hauntsaninja
gh-128030: Avoid error from PyModule_GetFilenameObject for non-module (…
45e6dd6 
…#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.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 20, 2024
@hauntsaninja @miss-islington
pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…
76b88e6 
…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 <[email protected]>
@miss-islington 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
@miss-islington @hauntsaninja
[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` 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 <[email protected]>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Dec 23, 2024
@hauntsaninja @srinivasreddy
pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…
a548261 
…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.
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
@hauntsaninja @serhiy-storchaka @srinivasreddy
pythongh-127651: Use __file__ in diagnostics if origin is missing (py…
f93cc07 
…thon#127660)

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

---------

Co-authored-by: Serhiy Storchaka <[email protected]>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
@hauntsaninja @srinivasreddy
pythongh-128030: Avoid error from PyModule_GetFilenameObject for non-…
628b0d2 
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz left review comments
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@hauntsaninja hauntsaninja

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@hauntsaninja @bedevere-bot @ericsnowcurrently @serhiy-storchaka @picnixz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.