-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Parse _version contents instead of using exec() #8050
Conversation
Option 1 would be a bit annoying, I'm fine with either of 2 or 3. It'd be nice to ditch the Option 3 feels a bit brittle, but I doubt we'll change |
We can also explicitly pass in a diff --git a/setup.py b/setup.py
index 7d8e1c1ee..5ec2f4c01 100644
--- a/setup.py
+++ b/setup.py
@@ -23,8 +23,10 @@ from setuptools.command.build_ext import build_ext
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__"]
+ lcl = {}
+ exec(compile(f.read(), version_file, "exec"), locals=lcl)
+ print(locals())
+ return lcl["__version__"]
configuration = {} (this patch wont directly work as it uses the 3.13 feature of passing locals as keyword 🤦🏻 ) |
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.
This change allows me to build Pillow on Python 3.13 beta 1 on GitHub Actions. It also allows me to build and run imageio
which depends on Pillow.
% pip install git+https://github.com/radarhere/Pillow.git@exec ; python_version >= '3.13'
Discussion in the CPython issue is saying that this is "an expected and intentional behavior change", so the solution would be either this PR, or #8057 |
Which PR do you prefer? |
My preference would be this one, to remove the exec() |
Thanks! |
FWIW, and not that anyone asked, but I switched to using a JSON file for managing such variables that need to be acessed both from outside and within the package. This is very easy avoids the responsibility (exec()! eval()!) of parsing. I'm sure other solutions are equally valid. |
Pillow fails with Python 3.14 and 3.14 but a fix was done with python-pillow/Pillow#8050 which will be published in 10.4.0 python-pillow/Pillow#8076 Bug: T364840 Change-Id: Id3080f0e4e5d270c3bd03c56896af3cb61b609b8
This includes a backport of python-pillow/Pillow#8050 (python-pillow/Pillow@57399ce) from Pillow 10.4.0, necessary for Python 3.13 compatibility. See python/cpython#118888.
This includes a backport of python-pillow/Pillow#8050 (python-pillow/Pillow@57399ce) from Pillow 10.4.0, necessary for Python 3.13 compatibility. See python/cpython#118888.
This includes a backport of python-pillow/Pillow#8050 (python-pillow/Pillow@57399ce) from Pillow 10.4.0, necessary for Python 3.13 compatibility. See python/cpython#118888.
This includes a backport of python-pillow/Pillow#8050 (python-pillow/Pillow@57399ce) from Pillow 10.4.0, necessary for Python 3.13 compatibility. See python/cpython#118888.
Our Python 3.13 jobs have started failing in GitHub Actions - https://github.com/python-pillow/Pillow/actions/runs/9032048835/job/24819485997#step:9:46
This is referring to
Pillow/setup.py
Lines 23 to 27 in 0cad346
Python 3.13 is failing to populate the local variables from
exec()
- python/cpython#118888We have various options at this point.
exec()
for this (originally added in RFC: Specify Version in one place #2517), and instead just simply parse the version string out of_version.py
after reading the file contents.This PR suggests Option 3. I don't mind if this is declined, it merely seems like a straightforward solution with no obvious downsides - if
_version.py
ever changes in the future so that this parsing code doesn't work,get_version()
can be changed again.