Python/makeopcodetargets.py uses deprecated imp APIs.  It should be refactored to use newer import-related APIs.

Here's a patch.

The comment about staying compatibler with 2.3 is now clearly out of date :)

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

Some of the buildbots have Python 2.5 as their system Python.

Here is a patch that is compatible with older Python versions.

Victor went with a different solution in f682b620c64d.

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.

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

issue20021_2.diff looks good to me.

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

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

Thank you Victor.

stage: resolved

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)

nosy: + vstinner
messages: + msg262425

resolution: out of date
stage: patch review -> resolved

messages: + msg252257

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

keywords: + patch
stage: needs patch -> patch review