Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Parse _version contents instead of using exec() - #8050

Merged
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec
May 21, 2024
Merged

Parse _version contents instead of using exec()#8050
hugovk merged 1 commit into
python-pillow:mainfrom
radarhere:exec

Conversation

@radarhere

@radarhereradarhere commented May 10, 2024

Copy link
Copy Markdown
Member

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

 File "/private/var/folders/3m/p59k4qdj0f17st0gn2cmj3640000gn/T/pip-build-env-v125k6ei/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 311, in run_setup
exec(code, locals())
~~~~^^^^^^^^^^^^^^^^
File "<string>", line 33, in<module>
File "<string>", line 27, in get_version
KeyError: '__version__'

This is referring to

Pillow/setup.py

Lines 23 to 27 in 0cad346

defget_version():
version_file="src/PIL/_version.py"
withopen(version_file, encoding="utf-8") asf:
exec(compile(f.read(), version_file, "exec"))
returnlocals()["__version__"]

Python 3.13 is failing to populate the local variables from exec() - python/cpython#118888

We have various options at this point.

  1. Presuming the bug report is accepted and fixed, the next beta will be out in about three weeks. We could just deal with these failing jobs until then.
  2. We could disable these jobs until then.
  3. We could stop using 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.

@hugovk

Copy link
Copy Markdown
Member

Option 1 would be a bit annoying, I'm fine with either of 2 or 3.

It'd be nice to ditch the exec.

Option 3 feels a bit brittle, but I doubt we'll change _version.py much and if we do it'd probably break the build in an obvious way, especially as we have the version check in the Python and C sides.

@tacaswell

Copy link
Copy Markdown
Contributor

We can also explicitly pass in a locals to be populated:

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 🤦🏻 )

@cclausscclauss left a comment

Copy link
Copy Markdown
Contributor

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'

@radarhere

Copy link
Copy Markdown
MemberAuthor

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

@hugovk

Copy link
Copy Markdown
Member

Which PR do you prefer?

@radarhere

Copy link
Copy Markdown
MemberAuthor

My preference would be this one, to remove the exec()

@hugovk
hugovk merged commit d879f39 into python-pillow:mainMay 21, 2024
@hugovk

Copy link
Copy Markdown
Member

Thanks!

@radarhere
radarhere deleted the exec branch May 21, 2024 13:20
@Yay295Yay295 mentioned this pull request May 22, 2024
@Themanwithoutaplan

Copy link
Copy Markdown

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.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this pull request May 26, 2024
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
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 15, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this pull request Aug 4, 2024
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 5, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 6, 2024
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.
markmentovai added a commit to markmentovai/macports-ports that referenced this pull request Nov 7, 2024
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.
reneeotten pushed a commit to macports/macports-ports that referenced this pull request Nov 7, 2024
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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@radarhere@hugovk@tacaswell@Themanwithoutaplan@cclauss