classification
Title: put opcode information in one place
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: kushal.das Nosy List: benjamin.peterson, db3l, eric.araujo, ezio.melotti, haypo, kushal.das, loewis, ned.deily, python-dev, twouters
Priority: normal Keywords: easy, patch

Created on 2013-04-28 15:14 by benjamin.peterson, last changed 2014-04-29 08:46 by ned.deily.

Files
File name Uploaded Description Edit
generate_opcode_h.py kushal.das, 2013-04-28 16:25 opcode.h generator
generate_opcode_h.py kushal.das, 2013-04-28 16:51 second version of the patch
generate_opcode_h.py kushal.das, 2013-04-28 17:05 Third revision of the patch.
issue17861_v1.patch kushal.das, 2013-04-29 16:43 patch with code and makefile update review
issue17861_v2.patch kushal.das, 2013-04-29 17:34 version two of the patch review
issue17861_v3.patch kushal.das, 2014-04-14 19:09 New patch with .hgtouch file details. review
issue17861_v4.patch kushal.das, 2014-04-14 22:09 New patch with proper changesets. review
issue17861_v5.patch kushal.das, 2014-04-15 14:23 New patch from a clean repo, with changed opcode.h review
issue17861_v6.patch kushal.das, 2014-04-15 17:53 New patch with changes as suggested by Brett. review
generate_opcode_h.diff twouters, 2014-04-16 21:34 review
generate_opcode_h.diff db3l, 2014-04-28 19:54 Support running under Python 2.5 review
Messages (21)
msg187986 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-04-28 15:14
Right now, opcode information is duplicated in Include/opcode.h and Lib/opcode.py. I should only have to update one manually and have the other one generated automatically. opcode.h should probably be generated by a script from opcode.py.
msg187998 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2013-04-28 16:25
Here is a simple script which prints the opcode.h in console. We can redirect it as required.
msg188000 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2013-04-28 16:51
Second version of the script with the fix for HAVE_ARGUMENT
msg188005 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2013-04-28 17:05
Third revision with fixed empty spaces at the end of the lines.
msg188074 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2013-04-29 16:43
As we discussed on #python-dev channel this new patch includes the script in Tools/scripts/generate_opcode_h.py and it also contains the required Makefile.pre.in change so that it gets auto(re)generated at compile time if required.
msg188079 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2013-04-29 17:34
Version 2 of the patchset edited as per review.
msg215915 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-04-11 06:15
Patch looks good to me.
msg216004 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-04-13 09:03
Please add the file to .hgtouch as well, and verify that "make touch" works correctly.
msg216179 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2014-04-14 19:09
New patch with .hgtouch file details.
msg216233 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2014-04-14 22:09
New patch with proper changesets.
msg216294 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2014-04-15 14:23
New patch from a clean repo, with changed opcode.h
msg216348 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2014-04-15 17:53
New patch with changes as suggested by Brett.
msg216358 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-15 18:22
New changeset 56e7fa27f979 by Kushal Das in branch 'default':
Closes Issue 17861: Autogenerate Include/opcode.h from opcode.py.
http://hg.python.org/cpython/rev/56e7fa27f979
msg216591 - (view) Author: Thomas Wouters (twouters) * (Python committer) Date: 2014-04-16 21:06
FYI, this broke building in a separate object directory (again!) for multiple reasons: it's running the script without specifying $(srcdir), and it's writing to $(srcdir)/Include/opcode.h (where $(srcdir) may be unwritable), and it's picking up the wrong opcode module: the sys.path mucking in Tools/generate_opcode_h.py isn't inserting the right directory for 'import opcode' to pick up the target-Python's opcode.py. It should probably be using execfile() and have the name of the file passed in, instead.

Even if it were importing the right opcode.py, relying on 'import' or 'execfile' here sounds like a bad idea: it means the opcode module of the *target* Python needs to be compatible with the Python that the generate_opcode_h.py script runs with. Since the syntax of opcode.h is much more constrained, a more flexible approach, in my opinion, would be to parse opcode.h to produce opcode.py (or an _opcode.py with just the definitions, that opcode.py would then import.)
msg216599 - (view) Author: Thomas Wouters (twouters) * (Python committer) Date: 2014-04-16 21:34
Here's a minimal patch to at least make the current mechanism work when using a separate build directory.

I still don't like the fact that this is importing opcode.py in a different Python than the target Python. Nor do I like that the script hardcodes information about the opcodes (specifically, the first opcode to HAVE_ARGUMENT.) This feels like it's not actually better for maintenance than the previous solution.
msg216620 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-16 23:15
New changeset 2b187c9e3e92 by Thomas Wouters in branch 'default':
Fix Tools/scripts/generate_opcode_h.py from issue #17861 to work correctly
http://hg.python.org/cpython/rev/2b187c9e3e92
msg217409 - (view) Author: David Bolen (db3l) Date: 2014-04-28 19:54
This generator script breaks the daily OSX DMG builds on my bolen-dmg buildslave for the 3.x branch.

The issue is with the use of "with" as the slave uses Python 2.5 to build the installer.  Now, that's old, and I'm not even sure how necessary the daily builds are to keep running, but I believe the Mac build-installer script still tries to target 2.5 as a minimum - though I'm not sure if that's true for all build tools.  But the fix is fairly simple (a __future__ import).

See attached patch.

Alternatively, if the minimum Python to build the installer should be bumped, I can work on that for the slave.
msg217415 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-28 20:47
New changeset 1fd9c3f6cf68 by Ned Deily in branch 'default':
Issue #17861: Allow generate_opcode_h to run with a system Python 2.5.
http://hg.python.org/cpython/rev/1fd9c3f6cf68
msg217417 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2014-04-28 20:53
Thanks, David. Ideally, the generator script shouldn't run during an installer build since presumably the generated file should be up-to-date in the repo.  "make touch" could handle that but the installer build does use a separate build/object directory and doesn't currently use "make touch".  For Python 3.5, the installer build now does need at least a Python 2.6 to build the documentation but there's no reason to unnecessarily break the main build with such a simple fix available.
msg217469 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-04-29 05:36
It would be possible to have the daily DMG builder run "make touch".
msg217500 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2014-04-29 08:46
Martin, it could if "make touch" worked when building outside of the source directory (Issue21383).
History
Date User Action Args
2014-04-29 08:46:51ned.deilysetmessages: + msg217500
2014-04-29 05:36:21loewissetmessages: + msg217469
2014-04-28 20:53:45ned.deilysetnosy: + ned.deily
messages: + msg217417
2014-04-28 20:47:25python-devsetmessages: + msg217415
2014-04-28 19:54:55db3lsetfiles: + generate_opcode_h.diff
nosy: + db3l
messages: + msg217409

2014-04-16 23:15:22python-devsetmessages: + msg216620
2014-04-16 21:34:29twouterssetstatus: closed -> open
files: + generate_opcode_h.diff
messages: + msg216599

assignee: kushal.das
resolution: fixed ->
2014-04-16 21:06:23twouterssetnosy: + twouters
messages: + msg216591
2014-04-15 18:22:27python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg216358

resolution: fixed
stage: patch review -> resolved
2014-04-15 17:53:23kushal.dassetfiles: + issue17861_v6.patch

messages: + msg216348
2014-04-15 14:24:03kushal.dassetfiles: + issue17861_v5.patch

messages: + msg216294
2014-04-14 22:09:59kushal.dassetfiles: + issue17861_v4.patch

messages: + msg216233
2014-04-14 19:09:34kushal.dassetfiles: + issue17861_v3.patch

messages: + msg216179
versions: + Python 3.4, - Python 3.5
2014-04-13 09:03:11loewissetnosy: + loewis
messages: + msg216004
2014-04-11 06:15:14eric.araujosetnosy: + eric.araujo

messages: + msg215915
versions: + Python 3.5, - Python 3.4
2013-04-29 17:34:12kushal.dassetfiles: + issue17861_v2.patch

messages: + msg188079
2013-04-29 17:08:36ezio.melottisetnosy: + ezio.melotti

stage: needs patch -> patch review
2013-04-29 16:43:33kushal.dassetfiles: + issue17861_v1.patch
keywords: + patch
messages: + msg188074
2013-04-29 14:33:25hayposetnosy: + haypo
2013-04-28 17:05:33kushal.dassetfiles: + generate_opcode_h.py

messages: + msg188005
2013-04-28 16:51:21kushal.dassetfiles: + generate_opcode_h.py

messages: + msg188000
2013-04-28 16:25:05kushal.dassetfiles: + generate_opcode_h.py
nosy: + kushal.das
messages: + msg187998

2013-04-28 15:14:49benjamin.petersoncreate