Repository navigation
"modernize" makeopcodetargets.py #64220
Description
| BPO | 20021 |
|---|---|
| Nosy | @brettcannon, @vstinner, @bitdancer, @ericsnowcurrently, @berkerpeksag, @serhiy-storchaka |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2016-03-26.06:30:27.520>
created_at = <Date 2013-12-19.04:11:25.741>
labels = ['easy', 'type-feature', 'library']
title = '"modernize" makeopcodetargets.py'
updated_at = <Date 2016-03-26.06:30:27.519>
user = 'https://github.com/ericsnowcurrently'bugs.python.org fields:
activity = <Date 2016-03-26.06:30:27.519>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2016-03-26.06:30:27.520>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2013-12-19.04:11:25.741>
creator = 'eric.snow'
dependencies = []
files = ['36745', '40670']
hgrepos = []
issue_num = 20021
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['206577', '227703', '240387', '240393', '252257', '262425', '262427', '262431', '262432', '262471', '262472', '262486']
nosy_count = 8.0
nosy_names = ['brett.cannon', 'vstinner', 'Arfrever', 'r.david.murray', 'python-dev', 'eric.snow', 'berker.peksag', 'serhiy.storchaka']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20021'
versions = ['Python 3.6']
Activity
ericsnowcurrently commentedon Dec 19, 2013
Python/makeopcodetargets.py uses deprecated imp APIs. It should be refactored to use newer import-related APIs.
berkerpeksag commentedon Sep 27, 2014
Here's a patch.
bitdancer commentedon Apr 9, 2015
The comment about staying compatibler with 2.3 is now clearly out of date :)
berkerpeksag commentedon Apr 9, 2015
I think makeopcodetargets.py should still be compatible with Python 2 (2.6 or 2.7 at least) since it uses the system Python.
I'd suggest closing this as "rejected" (or just commit the ``with open(...):`` part of the patch and update the outdated comment).
serhiy-storchaka commentedon Oct 4, 2015
Some of the buildbots have Python 2.5 as their system Python.
Here is a patch that is compatible with older Python versions.
berkerpeksag commentedon Mar 25, 2016
Victor went with a different solution in f682b620c64d.
serhiy-storchaka commentedon Mar 25, 2016
f682b620c64d looks incorrect to me. It makes makeopcodetargets.py to use the opcode module of Python that executes the script. Instead the script should use the opcode module from the same source tree as the script.
I think f682b620c64d should be reverted.
vstinner commentedon Mar 25, 2016
Oh sorry, I wasn't aware of this old issue. My motivation was to get ride of the deprecated imp module, this script looks to be the latest part of the Python which uses the imp module (except of deliberate unit tests on the imp module).
Ah, this was unclear to me. It looks overcomplicated code to just import a module. Can't we simply use import, but ensure that the running python executable belongs to the current source tree?
Or does it matter to be able to use an external python program?
Don't hesitate to rework the code to importlib if you want to ensure that Lib/opcode.py is imported.
But please don't revert the whole change, I changed other things.
vstinner commentedon Mar 25, 2016
issue20021_2.diff looks good to me.
python-dev commentedon Mar 26, 2016
New changeset 6ceceb052394 by Victor Stinner in branch 'default':
makeopcodetargets.py: we need to import Lib/opcode.py
https://hg.python.org/cpython/rev/6ceceb052394
vstinner commentedon Mar 26, 2016
Oh wait, I misunderstood how the script is used. I understood that the script must be run manually only when a new opcode is added and so is rarely used and only used with the built Python program.
In fact, the script is part of the build process! I completely missed that, sorry. I searched how it was used but I missed the build process.
So yes, it matters to support Python 2 and Python 3 in Python/makeopcodetargets.py.
I pushed Serhiy's patch issue20021_2.diff.
serhiy-storchaka commentedon Mar 26, 2016
Thank you Victor.