classification
Title: ASDL compatibility with Python 3 system interpreter
Type: compile error Stage: resolved
Components: Build Versions: Python 2.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, malthe, martin.panter, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-09-14 08:53 by malthe, last changed 2020-01-10 12:09 by cheryl.sabella. This issue is now closed.

Files
File name Uploaded Description Edit
0001-Allow-make-to-be-run-under-Python-3.patch malthe, 2016-09-14 08:52 Patch
PYTHON_FOR_GEN.patch martin.panter, 2016-09-18 00:54 review
0001-Allow-make-to-be-run-under-Python-3.patch malthe, 2016-09-24 07:29 Updated patch
0001-Allow-make-to-be-run-under-Python-3.patch malthe, 2016-10-15 21:21 Updated patch
Pull Requests
URL Status Linked Edit
PR 4082 closed malthe, 2017-10-23 10:58
Messages (14)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2017-10-23 10:43
Do you mind to create a pull request on GitHub Malthe?
History
Date User Action Args
2020-01-10 12:09:06cheryl.sabellasetstatus: open -> closed
resolution: wont fix
stage: patch review -> resolved
2017-10-23 10:58:51malthesetstage: needs patch -> patch review
pull_requests: + pull_request4052
2017-10-23 10:43:03serhiy.storchakasetmessages: + msg304791
stage: needs patch
2016-10-15 21:23:35serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg278734
2016-10-15 21:21:23malthesetfiles: + 0001-Allow-make-to-be-run-under-Python-3.patch

messages: + msg278733
2016-10-15 21:04:16martin.pantersetmessages: + msg278732
2016-09-24 07:29:44malthesetfiles: + 0001-Allow-make-to-be-run-under-Python-3.patch

messages: + msg277310
2016-09-24 03:28:06martin.pantersetmessages: + msg277307
2016-09-18 06:29:28malthesetmessages: + msg276871
2016-09-18 00:54:22martin.pantersetfiles: + PYTHON_FOR_GEN.patch

messages: + msg276863
2016-09-16 14:35:05r.david.murraysetmessages: + msg276721
2016-09-16 01:40:46martin.pantersetmessages: + msg276659
2016-09-16 01:36:56r.david.murraysetmessages: + msg276655
2016-09-16 01:34:33martin.pantersetnosy: + martin.panter
messages: + msg276654
2016-09-14 14:08:04r.david.murraysetnosy: + r.david.murray
messages: + msg276453
2016-09-14 10:18:15serhiy.storchakasetnosy: + benjamin.peterson
2016-09-14 08:53:00malthecreate