Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3.13.0b1: exec() does not populate locals() #118888

Closed
hroncok opened this issue May 10, 2024 · 10 comments · Fixed by #119893
Closed

Python 3.13.0b1: exec() does not populate locals() #118888

hroncok opened this issue May 10, 2024 · 10 comments · Fixed by #119893
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes docs Documentation in the Doc dir

Comments

@hroncok
Copy link
Contributor

hroncok commented May 10, 2024

Bug report

Bug description:

x.py

xxx = 118888

readx.py

def f():
    with open("x.py", encoding="utf-8") as f:
        exec(compile(f.read(), "x.py", "exec"))
    return locals()["xxx"]

print(f())

shell

$ python3.12 readx.py
118888

$ python3.13 readx.py
Traceback (most recent call last):
  File ".../readx.py", line 6, in <module>
    print(f())
          ~^^
  File ".../readx.py", line 4, in f
    return locals()["xxx"]
           ~~~~~~~~^^^^^^^
KeyError: 'xxx'

This breaks e.g. pillow 10.3.0 which has:

def get_version():
    version_file = "src/PIL/_version.py"
    with open(version_file, encoding="utf-8") as f:
        exec(compile(f.read(), version_file, "exec"))
    return locals()["__version__"]

In https://github.com/python-pillow/Pillow/blob/10.3.0/setup.py#L23

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

@hroncok hroncok added the type-bug An unexpected behavior, bug, or error label May 10, 2024
@Eclips4
Copy link
Member

Eclips4 commented May 10, 2024

Bisected to b034f14
cc @gaogaotiantian

@gaogaotiantian
Copy link
Member

This is an expected and intentional behavior change due to PEP 667. locals() now has a clear semantic when called inside a function - a snapshot of the local variables, and xxx (or __version__) is not one of them.

I won't even consider this is "breaking" as the docs clearly states:

modifications to the default locals dictionary should not be attempted. Pass an explicit locals dictionary if you need to see effects of the code on locals after function exec() returns.

So this is an illegal usage that happens to work in a favored way to begin with.

If you want the result of the local changes, pass in an explicit dictionary:

def get_version():
    version_file = "src/PIL/_version.py"
    d = {}
    with open(version_file, encoding="utf-8") as f:
        exec(compile(f.read(), version_file, "exec"), globals(), d)
    return d["__version__"]

I'm aware that this might be a bit inconvenience to the library maintainers, but this is the right way to go and we are making efforts to make locals() more consistent and predictable.

@terryjreedy terryjreedy added docs Documentation in the Doc dir and removed type-bug An unexpected behavior, bug, or error labels May 10, 2024
@terryjreedy
Copy link
Member

It it true that the 3.12 docs say that readx.py should not be expected to work. But it did then and previously, even though not now. What's New 3.13 only says

PEP 667: FrameType.f_locals when used in a function now returns a write-through proxy to the frame’s locals, rather than a dict. See the PEP for corresponding C API changes and deprecations.

From this, I would not expect changes in how locals() behaves, in particular in the effect of exec bindings. I think this should be mention.

Even the Python subsection of the PEP's Back Compatibility section says nothing. It only mentions a couple of things that do not change.

@gaogaotiantian
Copy link
Member

We are aware that the docs are not fully ready for beta 1, but this behavior is described in detail in locals. https://docs.python.org/3.13/library/functions.html#locals . We can add some notes to exec or eval or whatsnews, but the key change is actually on the locals() function, which the first version of docs is written for.

@hugovk
Copy link
Member

hugovk commented May 25, 2024

Can this be closed now the docs were updated in #119201?

@terryjreedy
Copy link
Member

@ncoghlan Does #119201 make this obsolete?

@ncoghlan
Copy link
Contributor

ncoghlan commented May 30, 2024

Yeah, the function level snapshot behaviour is now covered in the What's New porting guide: https://docs.python.org/3.13/whatsnew/3.13.html#changes-in-the-python-api

It is also mentioned in a versionchanged note on exec itself: https://docs.python.org/3.13/library/functions.html#exec

The general write-up of PEP 667 also mentions locals() first before covering FrameType.f_locals: https://docs.python.org/3.13/whatsnew/3.13.html#whatsnew313-locals-semantics

The most minimal change to fix this kind of exec invocation is to pass explicit target namespaces as suggested in #118888 (comment) (this exec call is already implicitly being called with separate globals and locals namespaces, so explicitly calling it that way won't change the behaviour of the executed code)

Alternatively, for the examples given, https://docs.python.org/3/library/runpy.html#runpy.run_path is a better tool when the task is "run the Python file at this location and return its top level namespace" (it will respect Python source encoding declarations properly, while explicitly opening the files as utf-8 ignores them).

@ncoghlan
Copy link
Contributor

ncoghlan commented May 30, 2024

Considering this further, I'm thinking it may be worth tweaking the text in "What's New" a bit, as somebody reading even the updated What's New entry might not make the leap from "the mutation semantics of locals() have changed in optimised scopes" to "the semantics of exec(), eval(), and other code execution APIs that default to targeting locals() have changed in optimised scopes".

@ncoghlan ncoghlan reopened this May 30, 2024
@ncoghlan
Copy link
Contributor

ncoghlan commented May 30, 2024

Current plan for changes


  • Add this paragraph to the main PEP 667 description in the 3.13 "What's New?" doc:

The change to the semantics of locals() in optimized scopes also affects the default behaviour of code execution functions that implicitly target locals() if no explicit namespace is provided (such as exec and eval). In previous versions, whether or not the changes could be accessed by calling locals() after the code finished execution was implementation dependent. In CPython specifically, such code would often appear to work as desired, but could sometimes fail in optimized scopes based on other code (including debuggers and code execution tracing tools) potentially resetting the shared snapshot in that scope. Now, the code will always run against an independent snapshot of locals() in optimized scopes, and hence the changes will never be visible in subsequent calls to locals(). To access the changes made in these cases, an explicit namespace reference must now be passed to the relevant function. Alternatively, it may make sense to switch over to using a higher level code execution API that returns the resulting code execution namespace (such as runpy.run_path).

  • Add this sentence to the porting note:

Code execution functions that implicitly target locals() (such as exec and eval) must be passed an explicit namespace to access their results in an optimized scope.

@ncoghlan
Copy link
Contributor

No PR yet as I'll merge #119379 before making any further PEP 667 related updates.

ncoghlan added a commit that referenced this issue Jun 1, 2024
* Clarify impact on default behaviour of exec, eval, etc
* Update documentation for changes to PyEval_GetLocals (gh-74929)

Closes gh-11888
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 1, 2024
* Clarify impact on default behaviour of exec, eval, etc
* Update documentation for changes to PyEval_GetLocals (pythongh-74929)

Closes pythongh-11888
(cherry picked from commit 2180991)

Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
ncoghlan added a commit that referenced this issue Jun 2, 2024
* Clarify impact on default behaviour of exec, eval, etc
* Update documentation for changes to PyEval_GetLocals (gh-74929)

Closes gh-118888
(cherry picked from commit 2180991)

Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
barneygale pushed a commit to barneygale/cpython that referenced this issue Jun 5, 2024
* Clarify impact on default behaviour of exec, eval, etc
* Update documentation for changes to PyEval_GetLocals (pythongh-74929)

Closes pythongh-11888
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
* Clarify impact on default behaviour of exec, eval, etc
* Update documentation for changes to PyEval_GetLocals (pythongh-74929)

Closes pythongh-11888
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
* Clarify impact on default behaviour of exec, eval, etc
* Update documentation for changes to PyEval_GetLocals (pythongh-74929)

Closes pythongh-11888
mikem23 pushed a commit to koji-project/koji that referenced this issue Aug 14, 2024
Accessing __version__ from locals() no longer works.

This was reported to Python in python/cpython#118888
but according to Python developers, it:

 - is an intended change of behavior described in PEP 667
 - was an illegal usage that happens to work in a favored way to begin with
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants