classification
Title: "modernize" makeopcodetargets.py
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, berker.peksag, brett.cannon, eric.snow, python-dev, r.david.murray, serhiy.storchaka, vstinner
Priority: high Keywords: easy, patch

Created on 2013-12-19 04:11 by eric.snow, last changed 2016-03-26 06:30 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue20021.diff berker.peksag, 2014-09-27 17:50 review
issue20021_2.diff serhiy.storchaka, 2015-10-04 04:58 review
Messages (12)
msg206577 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-12-19 04:11
Python/makeopcodetargets.py uses deprecated imp APIs.  It should be refactored to use newer import-related APIs.
msg227703 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-09-27 17:50
Here's a patch.
msg240387 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-04-09 20:49
The comment about staying compatibler with 2.3 is now clearly out of date :)
msg240393 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-04-09 21:32
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).
msg252257 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-04 04:58
Some of the buildbots have Python 2.5 as their system Python.

Here is a patch that is compatible with older Python versions.
msg262425 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-03-25 11:45
Victor went with a different solution in f682b620c64d.
msg262427 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-25 11:52
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.
msg262431 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-25 12:07
> Victor went with a different solution in f682b620c64d.

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).


> 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.

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?


> I think f682b620c64d should be reverted.

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.
msg262432 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-25 12:08
issue20021_2.diff looks good to me.
msg262471 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-26 00:05
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
msg262472 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-03-26 00:06
> Or does it matter to be able to use an external python program?

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.
msg262486 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-26 06:30
Thank you Victor.
History
Date User Action Args
2016-03-26 06:30:27serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg262486

stage: resolved
2016-03-26 00:06:24vstinnersetmessages: + msg262472
2016-03-26 00:05:08python-devsetnosy: + python-dev
messages: + msg262471
2016-03-25 12:08:50vstinnersetmessages: + msg262432
2016-03-25 12:07:25vstinnersetmessages: + msg262431
2016-03-25 11:52:02serhiy.storchakasetstatus: closed -> open

nosy: brett.cannon, vstinner, Arfrever, r.david.murray, eric.snow, berker.peksag, serhiy.storchaka
messages: + msg262427
priority: low -> high
resolution: out of date -> (no value)
stage: resolved -> (no value)
2016-03-25 11:45:12berker.peksagsetstatus: open -> closed

nosy: + vstinner
messages: + msg262425

resolution: out of date
stage: patch review -> resolved
2015-10-04 04:58:03serhiy.storchakasetfiles: + issue20021_2.diff
versions: + Python 3.6, - Python 3.5
nosy: + serhiy.storchaka

messages: + msg252257
2015-10-04 02:14:40Arfreversetnosy: + Arfrever
2015-04-09 21:32:12berker.peksagsetmessages: + msg240393
2015-04-09 20:49:18r.david.murraysetnosy: + r.david.murray
messages: + msg240387
2014-09-27 17:50:57berker.peksagsetfiles: + issue20021.diff

nosy: + brett.cannon, berker.peksag
messages: + msg227703

keywords: + patch
stage: needs patch -> patch review
2014-09-27 14:09:05serhiy.storchakasetkeywords: + easy
stage: needs patch
2013-12-19 04:11:25eric.snowcreate