msg276401 - (view) |
Author: Malthe Borch (malthe) * |
Date: 2016-09-14 08:52 |
Many systems today use Python 3 as the default interpreter "python". The Python 2.7 build fails because of syntax incompatibility.
Attached patch fixes this.
|
msg276453 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2016-09-14 14:08 |
It's probably not a bad thing to fix this, but you should be able to avoid the problem by using 'make touch' before building. It should not be necessary to have a running python to build python; all the build artifacts are checked in. Specifically, the release tarballs should build without an existing python, and if they don't, that's a release packaging problem.
|
msg276654 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-16 01:34 |
It seems a terrible idea to require Python 3 to be installed in order to regenerate the boot files for a Python 2 build. Maybe if we can figure out the minimum installed Python version expected for these ASDL scripts in 2.7, and maintain that while adding Python 3 support. But even that seems risky.
Your changes obviously make the files incompatible with Python 2:
print() # Prints an empty tuple
print("Error visiting", repr(object)) # Prints a 2-tuple
f = open(p, "w") # Writes CRLFs on Windows (I think; it’s been a while)
If we do go down this road, it may be worth looking at what has already been done to the corresponding scripts in Python 3.
But you may not actually need to regenerate the files. If you are building from the source directory, try “make touch” after generating the makefile. In general, try “touch Include/Python-ast.h Python/Python-ast.c”.
I would like to see this be made easier for both Python 2 and 3. I suggested some other ideas in Issue 23404.
|
msg276655 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2016-09-16 01:36 |
Yes, by "fix this" I meant make it work under both python2 and python3. But failing that, the status quo wins.
|
msg276659 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-16 01:40 |
Another option I forgot to point out is that some of the other regeneration Python scripts (in Py 2 and/or 3; I forget which) have some autoconf magic to figure out the right installed python command to use. We should probably use that for Py 2’s ASDL script.
|
msg276721 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2016-09-16 14:35 |
If we go that route it doesn't, from a certain point of view, solve the problem for systems that *only* ship python3...but I think we can ignore that issue: IMO it would be fine in that scenario to say that if you want to run the python2 regen scripts you have to install python2. Especially since you can do that by building from source using the checked in artifacts.
|
msg276863 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-18 00:54 |
This patch ports the logic written for Issue 26662 to Python 2. Basically, configure searches for commands called python2.7, python2, and python (in order of priority), and sets PYTHON_FOR_GEN to the result. PYTHON_FOR_GEN could be overridden by the user if desired. If no Python command is found, the build fails with an error message:
Cannot generate /home/proj/python/cpython/Python/Python-ast.c, python not found !
To skip re-generation of /home/proj/python/cpython/Python/Python-ast.c run <make touch> or <make -t /home/proj/python/cpython/Python/Python-ast.c>.
Otherwise, set python in PATH and run configure or run <make PYTHON_FOR_GEN=python>.
--- Python/Python-ast.c ---
*** [Python/Python-ast.c] Error code 1
However if it finds “python” and that turns out to be version 3, it will still have the same problem as now, and you will have to manually use “make touch” or similar.
|
msg276871 - (view) |
Author: Malthe Borch (malthe) * |
Date: 2016-09-18 06:29 |
I forgot to add "from __future__ import print_function" to the beginning of "asdl.py" and "spark.py". It should then work on Python 2.
That is, with the imported print-function, the incompatibilities that Martin pointed out are no longer there.
As for Windows, I would think that printing out CRLFs is the correct behavior? I don't use Windows personally.
|
msg277307 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-09-24 03:28 |
That would get the patch mostly working with 2.6+. Is it okay to break Python < 2.6 support? I know there was an OS X Tiger buildbot using 2.5 until recently; see Issue 28039.
Personally, I would rather focus on the problems with make dependencies, build configuration, etc, rather than changing what versions of Python the code runs on.
Regarding CRLFs on Windows: the existing code opens files in binary mode (wb), so Windows will not translate \n into CRLF. Changing this behaviour is separate to the goal of Python 3 compatibility. IMO the build process should not change the contents of checked-in files, regardless of whether there is a version control system like Mercurial or Git in use or not. See also Issue 27425.
On the other hand, the Python 3 code does use text mode with CRLF translation, so maybe it is not a big deal. (I never built Python on Windows, so I don’t know.)
+++ b/Parser/spark.py
@@ -171,7 +171,7 @@ class GenericParser:
- rules = string.split(doc)
+ rules = str.split(doc)
This would read better as doc.split(); see <http://svn.python.org/view?view=revision&revision=55143>.
@@ -825,15 +828,15 @@ class GenericASTMatcher(GenericParser):
- print '\t', item
+ print('\t', item)
for (lhs, rhs), pos in states[item[0]].items:
- print '\t\t', lhs, '::=',
+ print('\t\t', lhs, '::=', end=" ")
. . .
+ print(string.join(rhs[:pos]), end=" ")
+ print('.', end=" ")
+ print(string.join(rhs[pos:]))
The string quoting is inconsistent. Also, I suspect string.join() is not right for Python 3. And you are adding an extra space after the '\t' characters.
For what it’s worth, here are some similar changes made to the Py 3 branch:
r57749: raise syntax
r51578: `. . .` → repr()
http://svn.python.org/view?view=revision&revision=55329: Unpack tuples inside functions (Malthe’s patch unpacks as the function is called instead); map() → list comprehension
http://svn.python.org/view?view=revision&revision=55143: Various Py 3 changes
r59154: Write files in text mode
r53189: Avoid has_key()
http://svn.python.org/view?view=revision&revision=55167: xrange() → range()
|
msg277310 - (view) |
Author: Malthe Borch (malthe) * |
Date: 2016-09-24 07:29 |
I have updated the patch with requested changes.
Note that the original code also added space after '\t' characters. I have not changed this on purpose.
|
msg278732 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-10-15 21:04 |
Are you sure about adding the space after tab? I am no Python 2 expert, but I don’t see it:
$ python2 -c 'print "\t", "<item>"' | cat -A
^I<item>$
Anyway, I’m not happy applying this patch unless it is clear that raising the Python version required to rebuild ASDL stuff is okay in the 2.7 branch. Maybe a query to the python-dev mail list might help.
|
msg278733 - (view) |
Author: Malthe Borch (malthe) * |
Date: 2016-10-15 21:21 |
You're right. The trailing comma just suppresses the newline. I updated the patch.
|
msg278734 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-10-15 21:23 |
print in Python 2 doesn't add a space after whitespace characters except a space ('\t', '\n', '\r', etc).
|
msg304791 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-23 10:43 |
Do you mind to create a pull request on GitHub Malthe?
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:36 | admin | set | github: 72330 |
2020-01-10 12:09:06 | cheryl.sabella | set | status: open -> closed resolution: wont fix stage: patch review -> resolved |
2017-10-23 10:58:51 | malthe | set | stage: needs patch -> patch review pull_requests:
+ pull_request4052 |
2017-10-23 10:43:03 | serhiy.storchaka | set | messages:
+ msg304791 stage: needs patch |
2016-10-15 21:23:35 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg278734
|
2016-10-15 21:21:23 | malthe | set | files:
+ 0001-Allow-make-to-be-run-under-Python-3.patch
messages:
+ msg278733 |
2016-10-15 21:04:16 | martin.panter | set | messages:
+ msg278732 |
2016-09-24 07:29:44 | malthe | set | files:
+ 0001-Allow-make-to-be-run-under-Python-3.patch
messages:
+ msg277310 |
2016-09-24 03:28:06 | martin.panter | set | messages:
+ msg277307 |
2016-09-18 06:29:28 | malthe | set | messages:
+ msg276871 |
2016-09-18 00:54:22 | martin.panter | set | files:
+ PYTHON_FOR_GEN.patch
messages:
+ msg276863 |
2016-09-16 14:35:05 | r.david.murray | set | messages:
+ msg276721 |
2016-09-16 01:40:46 | martin.panter | set | messages:
+ msg276659 |
2016-09-16 01:36:56 | r.david.murray | set | messages:
+ msg276655 |
2016-09-16 01:34:33 | martin.panter | set | nosy:
+ martin.panter messages:
+ msg276654
|
2016-09-14 14:08:04 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg276453
|
2016-09-14 10:18:15 | serhiy.storchaka | set | nosy:
+ benjamin.peterson
|
2016-09-14 08:53:00 | malthe | create | |