node 🚀 - Dependency v8 is not Python 3 compatible

PR has landed so I assume this can be closed. Comment (or re-open if GitHub allows) if I'm wrong about that! Thanks!!!

Trott on 26 Nov 2018

PR has landed so I assume this can be closed.

The files in deps/v8 and tools/inspector_protocol + jinja2 + markupsafe where excluded from https://github.com/nodejs/node/pull/24486. So this is still an issue.

refack on 26 Nov 2018

/CC @nodejs/v8 @nodejs/v8-inspector

FTR on 1.1.2020 Python 2 hits it's EOL https://pythonclock.org/

refack on 26 Nov 2018

If you do not mind.
I would like to try to take up this task.
I'll try it this weekend. If I have any problems, I'll write.

bmsdave on 28 Nov 2018

I definitely don't mind any contributions here. Do you have a list of scripts required by Node.js that need to be migrated?

hashseed on 28 Nov 2018

There are now less that 400 days until the end of life of Python 2 (aka _legacy_ Python). The v8 repo is just 1.4% Python but that is _all legacy_ Python and at least 76 files need to be modified just to fix the print statement which is merely the _start_ of a Python 3 port. v8 is a venerable codebase and I often hear that it has been a godsend to the JavaScript community but its Python code needs to be modernized, removed, or replaced with JavaScript, Go, etc. in the days remain until Python 2 end of life.

I would recommend that someone uses http://python-future.org to do the following:

Run v8 tests on Python 2.7 and make sure they pass
Drop commitment to support for all Python versions < 2.7 (they are EOL for 5+ years anyway)
Run __futurize -f libfuturize.fixes.fix_print_with_import -w .__ at the root of the v8 repo (v8/v8#26)
Run v8 tests on Python 2.7 and make sure they pass
Run __futurize --stage1 -w .__ at the root of the v8 repo
Run v8 tests on Python 2.7 and make sure they pass
Run v8 tests on Python 3.7 in __allow_failures__ mode and fix as you go...

cclauss on 28 Nov 2018

👍3

Do you have a list of scripts required by Node.js that need to be migrated?

@hashseed I don't have that list. I was planning on finding it through the codebase. But if you provide it, it will be very cool.

@cclauss thank you very much for your clear recommendations. I will try to do it and provide for consideration.

bmsdave on 28 Nov 2018

@bmsdave any changes to inspector_protocol should be upstreamed to inspector_protocol repo first. I can help you with it as soon as your are ready.

ak239 on 28 Nov 2018

@ak239 @bmsdave @hashseed Some low hanging fruit in the upstream __inspector_protocol__ __repo__.

flake8 testing of https://chromium.googlesource.com/deps/inspector_protocol on Python 3.7.1

./check_protocol_compatibility.py:478:46: E999 SyntaxError: invalid syntax
            print "  Public changes since %s:" % version
                                             ^
./code_generator.py:43:18: F821 undefined name 'xrange'
        for i in xrange(len(keys)):
                 ^
./pdl.py:162:51: E999 SyntaxError: invalid syntax
        print 'Error in %s:%s, illegal token: \t%s' % (file_name, i, line)
                                                  ^
2     E999 SyntaxError: invalid syntax
1     F821 undefined name 'xrange'
3

cclauss on 12 Dec 2018

👍1

The bug reports (and proposed fixes) on chromium.org:

Please _star_ these issues and fixes.

cclauss on 14 Dec 2018

👍1

Any progress on this issues?

cclauss on 19 Jan 2019

👍2

I think we are now waiting for the adoption of changes from Dmitry Gozman:
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1358520
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1387524

Is there anything else you need to do before Dmitry accepts the changes?

bmsdave on 21 Jan 2019

There is nothing that I need to do. On January 18th, Dmitry Gozman passed the ball on 1358520 to Johannes Henkel who gave a positive review.

cclauss on 21 Jan 2019

I'm sorry, I misspelled it.
Is there anything else I need to do before Dmitry accepts the changes? :)

As I understand it, I can try to build the entire node.js + v8 + inspector_protocol chain locally and test the functionality of the tools in python2.7 + python3 with specified edits?

bmsdave on 21 Jan 2019

Let's get ___inspector_protocol___ to a good state first because v8 is a much bigger problem and given v8/v8#26, I doubt that v8 will be successfully ported to Py3 in the days remaining before Py2 end of life.

cclauss on 21 Jan 2019

@cclauss what do you mean "given https://github.com/v8/v8/pull/26"? Did you try going through the chromium review instead of GitHub as suggested?

That said... it seems there's little/no buy-in from Google/v8 to push py3 changes through, and they would need to be pushed through...

hayd on 21 Jan 2019

I meant the latter (no buy-in), not the former (I did not re-try).

cclauss on 21 Jan 2019

@cclauss please consider resubmitting/retrying (not on GitHub). It seems like this discussion has the potential of being much more fruitful on the v8 mailing list https://v8.dev/docs/contribute

hayd on 21 Jan 2019

Forward progress on upstream __inspector_protocol__.
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1358520 has been merged.

cclauss on 28 Jan 2019

👍1

But the __xrange()__ fix in https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1387524 was ___not___ included.

cclauss on 28 Jan 2019

😕1

This task include fix with xrange():
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1440581
It looks like it will soon pass

bmsdave on 29 Jan 2019

As I understand it, the next step is to add changes to the v8. @cclauss, you have not yet transferred edits from https://github.com/v8/v8/pull/26 in chromium-review?

bmsdave on 29 Jan 2019

That is correct @bmsdave. It would be great if you could work on that part of the puzzle.

cclauss on 29 Jan 2019

@cclauss OK. I got this.

bmsdave on 29 Jan 2019

🚀3 👍3 🎉2

@bmsdave Please let me know if you need any help. I also started looking at V8's Python scripts, I am parking it now.

thefourtheye on 30 Jan 2019

👍1

I tried to figure out python3 compatibility.
It turned out to be quite a few dependencies of depot_tools are not python 3 compatible.
for example boto.
I'm not going to try to do anything about it yet. But it seems that later it may complicate the process of updating to python3.
I run flake8 check. Reports can be found here. -> https://travis-ci.com/bmsdave/v8/builds (thanks @cclauss !)
@thefourtheye could you advise me on ways to check python code?
Apparently launch tools/run-tests.py weakly covers python code and, i think, is not a trusted source that python2 is all right.

bmsdave on 4 Feb 2019

From https://pypi.org/project/boto ... _Boto3, the next version of Boto, is now stable and recommended for general use. It can be used side-by-side with Boto in the same project, so it is easy to start using Boto3 in your existing projects as well as new projects. Going forward, API updates and all new feature work will be focused on Boto3._

cclauss on 4 Feb 2019

👍1

You can make the following two issues disappear by adding "__from __future__ import print_function__" to the top of the file (before other imports).

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics --builtins="gdb,offset,slot"
./tools/locs.py:128:25: E999 SyntaxError: invalid syntax
        build_dir), file=sys.stderr)
                        ^
./tools/avg.py:198:57: E999 SyntaxError: invalid syntax
    print("{:<{}}".format("", self.last_status_len), end="\r")
                                                        ^

cclauss on 4 Feb 2019

__print_function__ also here...

./tools/gen-inlining-tests.py:457:14: E999 SyntaxError: invalid syntax
  return print(*args, file=FILE)
             ^

cclauss on 4 Feb 2019

Something went wrong here =|

I run git cl presubmit -v with changes https://github.com/bmsdave/v8/tree/b651ebc4b73f6425fe8557fc13415dd6f3593df1

and got brocken tests:

Running presubmit commit checks ...
Running /opt/node/tmp/v8/PRESUBMIT.py
No changes in C/C++ files detected. Skipping check
No changes in Torque files detected. Skipping check
Total violating files: 0
Running /opt/node/tmp/v8/infra/testing/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/clusterfuzz/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/mb/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/release/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/testrunner/PRESUBMIT.py

** Presubmit Messages **
unittests/predictable_wrapper_test.py (0.35s)

unittests/v8_presubmit_test.py (0.21s)

./v8_foozzie_test.py (0.27s)

Pylint (3 files using ['--disable=all', '--enable=cyclic-import']) (0.86s)

./test_scripts.py (0.26s)

testproc/variant_unittest.py (0.16s)

** Presubmit ERRORS **
OWNERS check failed: this CL has no Gerrit change number, so we can't check it for approvals.

[email protected] is not in AUTHORS file. If you are a new contributor, please visit
https://www.chromium.org/developers/contributing-code and read the "Legal" section
If you are a chromite, verify the contributor signed the CLA.

unittests/run_perf_test.py (0.30s) failed
ERROR

======================================================================
ERROR: setUpClass (__main__.PerfTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "unittests/run_perf_test.py", line 101, in setUpClass
    import run_perf
  File "/opt/node/tmp/v8/tools/run_perf.py", line 105, in <module>
    from past.builtins import basestring
ImportError: No module named past.builtins

----------------------------------------------------------------------
Ran 0 tests in 0.003s

FAILED (errors=1)


unittests/run_tests_test.py (0.21s) failed
ERROR

======================================================================
ERROR: setUpClass (__main__.SystemTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "unittests/run_tests_test.py", line 150, in setUpClass
    from testrunner import standard_runner
  File "/opt/node/tmp/v8/tools/testrunner/standard_runner.py", line 16, in <module>
    from . import base_runner
  File "/opt/node/tmp/v8/tools/testrunner/base_runner.py", line 27, in <module>
    from testrunner.local import testsuite
  File "/opt/node/tmp/v8/tools/testrunner/local/testsuite.py", line 37, in <module>
    from ..objects.testcase import TestCase
  File "/opt/node/tmp/v8/tools/testrunner/objects/testcase.py", line 29, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins

----------------------------------------------------------------------
Ran 0 tests in 0.048s

FAILED (errors=1)


Pylint (3 files using ['--disable=cyclic-import'] on 4 cores) (3.31s) failed
************* Module mb
W: 15, 0: Redefining built-in 'cmp' (redefined-builtin)


./mb_unittest.py (0.17s) failed
Traceback (most recent call last):
  File "./mb_unittest.py", line 14, in <module>
    import mb
  File "/opt/node/tmp/v8/tools/mb/mb.py", line 15, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins


mb_validate (0.15s) failed
Traceback (most recent call last):
  File "mb.py", line 15, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins


local/testsuite_unittest.py (0.19s) failed
Traceback (most recent call last):
  File "local/testsuite_unittest.py", line 16, in <module>
    from testrunner.local.testsuite import TestSuite
  File "/opt/node/tmp/v8/tools/testrunner/local/testsuite.py", line 37, in <module>
    from ..objects.testcase import TestCase
  File "/opt/node/tmp/v8/tools/testrunner/objects/testcase.py", line 29, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins


local/statusfile_unittest.py (0.17s) failed
Traceback (most recent call last):
  File "local/statusfile_unittest.py", line 11, in <module>
    from . import statusfile
ValueError: Attempted relative import in non-package


local/pool_unittest.py (0.18s) failed
testAdd (__main__.PoolTest) ... ERROR
testException (__main__.PoolTest) ... ERROR
testNormal (__main__.PoolTest) ... ERROR

======================================================================
ERROR: testAdd (__main__.PoolTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "local/pool_unittest.py", line 49, in testAdd
    pool = Pool(3)
  File "/opt/node/tmp/v8/tools/testrunner/local/pool.py", line 127, in __init__
    self.work_queue = Queue()
NameError: global name 'Queue' is not defined

======================================================================
ERROR: testException (__main__.PoolTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "local/pool_unittest.py", line 35, in testException
    pool = Pool(3)
  File "/opt/node/tmp/v8/tools/testrunner/local/pool.py", line 127, in __init__
    self.work_queue = Queue()
NameError: global name 'Queue' is not defined

======================================================================
ERROR: testNormal (__main__.PoolTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "local/pool_unittest.py", line 25, in testNormal
    pool = Pool(3)
  File "/opt/node/tmp/v8/tools/testrunner/local/pool.py", line 127, in __init__
    self.work_queue = Queue()
NameError: global name 'Queue' is not defined

----------------------------------------------------------------------
Ran 3 tests in 0.001s

FAILED (errors=3)


Presubmit checks took 7.5s to calculate.

Was the presubmit check useful? If not, run "git cl presubmit -v"
to figure out which PRESUBMIT.py was run, then run git blame
on the file to figure out who to ask for help.

I can not yet figure out where the dependencies come from, since globally I have future installed, but I still get ImportError: No module named past.builtins

bmsdave on 4 Feb 2019

__pip install future__ will give you __past__.

try:
  from queue import Queue  # Python 3
except NameError:
  from Queue import Queue  # Python 2

cclauss on 4 Feb 2019

python3 -m pip install future
python2 -m pip install future

cclauss on 4 Feb 2019

Yes. I have the future package installed locally. But the error remains.
If i run tests manually -> all pass successfully. But with git cl presubmit -v -> the tests do not pass.
I have an assumption that this command runs in some virtual environment.

bmsdave@bmsdave:/opt/node/tmp/v8$ git cl presubmit -v
Running presubmit commit checks ...
Running /opt/node/tmp/v8/PRESUBMIT.py
[I2019-02-05 09:34:35,937 30658 140701642540864 driver.py] Generating grammar tables from /usr/lib/python2.7/lib2to3/Grammar.txt
[I2019-02-05 09:34:35,958 30658 140701642540864 driver.py] Generating grammar tables from /usr/lib/python2.7/lib2to3/PatternGrammar.txt
No changes in C/C++ files detected. Skipping check
No changes in Torque files detected. Skipping check
Total violating files: 0
[I2019-02-05 09:34:36,083 30658 140701642540864 presubmit_canned_checks.py] Valid authors are *@google.com, *@chromium.org, *@sdesigns.com, *@arm.com, *@palm.com, *@igalia.com, *@joyent.com, *@bloomberg.net, *@nvidia.com, *@blackberry.com, *@opera.com, *@intel.com, *@microsoft.com, *@mips.com, *@imgtec.com, *@wavecomp.com, *@loongson.cn, *@codeaurora.org, *@homejinni.com, *@*ibm.com, *@*.samsung.com, *@joyent.com, *@rt-rk.com, *@amazon.com, *@st.com, *@yandex-team.ru, *@strongloop.com, *@fb.com, *@oculus.com, *@vewd.com, *@groupon.com, *@meteor.com, *@cloudflare.com, [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Running /opt/node/tmp/v8/infra/testing/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/clusterfuzz/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/mb/PRESUBMIT.py
[I2019-02-05 09:34:37,780 30658 140701642540864 presubmit_canned_checks.py] Running pylint on 3 files
Running /opt/node/tmp/v8/tools/release/PRESUBMIT.py
Running /opt/node/tmp/v8/tools/testrunner/PRESUBMIT.py

** Presubmit Messages **
unittests/predictable_wrapper_test.py (0.33s)

./v8_foozzie_test.py (0.27s)

Pylint (3 files using ['--disable=all', '--enable=cyclic-import']) (0.63s)

./test_scripts.py (0.27s)

testproc/variant_unittest.py (0.17s)

local/pool_unittest.py (0.53s)

** Presubmit ERRORS **
OWNERS check failed: this CL has no Gerrit change number, so we can't check it for approvals.

[email protected] is not in AUTHORS file. If you are a new contributor, please visit
https://www.chromium.org/developers/contributing-code and read the "Legal" section
If you are a chromite, verify the contributor signed the CLA.

unittests/run_perf_test.py (0.30s) failed
ERROR

======================================================================
ERROR: setUpClass (__main__.PerfTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "unittests/run_perf_test.py", line 101, in setUpClass
    import run_perf
  File "/opt/node/tmp/v8/tools/run_perf.py", line 105, in <module>
    from past.builtins import basestring
ImportError: No module named past.builtins

----------------------------------------------------------------------
Ran 0 tests in 0.003s

FAILED (errors=1)


unittests/run_tests_test.py (0.22s) failed
ERROR

======================================================================
ERROR: setUpClass (__main__.SystemTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "unittests/run_tests_test.py", line 151, in setUpClass
    from testrunner import standard_runner
  File "/opt/node/tmp/v8/tools/testrunner/standard_runner.py", line 17, in <module>
    from . import base_runner
  File "/opt/node/tmp/v8/tools/testrunner/base_runner.py", line 28, in <module>
    from testrunner.local import testsuite
  File "/opt/node/tmp/v8/tools/testrunner/local/testsuite.py", line 37, in <module>
    from ..objects.testcase import TestCase
  File "/opt/node/tmp/v8/tools/testrunner/objects/testcase.py", line 30, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins

----------------------------------------------------------------------
Ran 0 tests in 0.050s

FAILED (errors=1)


unittests/v8_presubmit_test.py (0.18s) failed
Traceback (most recent call last):
  File "unittests/v8_presubmit_test.py", line 15, in <module>
    from v8_presubmit import FileContentsCache, CacheableSourceFileProcessor
  File "/opt/node/tmp/v8/tools/v8_presubmit.py", line 55, in <module>
    from testrunner.local import testsuite
  File "/opt/node/tmp/v8/tools/testrunner/local/testsuite.py", line 37, in <module>
    from ..objects.testcase import TestCase
  File "/opt/node/tmp/v8/tools/testrunner/objects/testcase.py", line 30, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins


Pylint (3 files using ['--disable=cyclic-import'] on 4 cores) (1.98s) failed
************* Module mb
E:1170, 0: invalid syntax (syntax-error)
************* Module mb_unittest
W: 67,32: Unused argument 'buffer_output' (unused-argument)
W: 67,22: Unused argument 'env' (unused-argument)
W: 82,21: Unused argument 'mode' (unused-argument)
W:545, 4: Attribute 'Run' defined outside __init__ (attribute-defined-outside-init)


./mb_unittest.py (0.16s) failed
Traceback (most recent call last):
  File "./mb_unittest.py", line 15, in <module>
    import mb
  File "/opt/node/tmp/v8/tools/mb/mb.py", line 1170
    print(*args, **kwargs)
          ^
SyntaxError: invalid syntax


mb_validate (0.15s) failed
  File "mb.py", line 1170
    print(*args, **kwargs)
          ^
SyntaxError: invalid syntax


local/testsuite_unittest.py (0.18s) failed
Traceback (most recent call last):
  File "local/testsuite_unittest.py", line 16, in <module>
    from testrunner.local.testsuite import TestSuite
  File "/opt/node/tmp/v8/tools/testrunner/local/testsuite.py", line 37, in <module>
    from ..objects.testcase import TestCase
  File "/opt/node/tmp/v8/tools/testrunner/objects/testcase.py", line 30, in <module>
    from past.builtins import cmp
ImportError: No module named past.builtins


local/statusfile_unittest.py (0.15s) failed
Traceback (most recent call last):
  File "local/statusfile_unittest.py", line 11, in <module>
    import statusfile
  File "/opt/node/tmp/v8/tools/testrunner/local/statusfile.py", line 35, in <module>
    from .variants import ALL_VARIANTS
ValueError: Attempted relative import in non-package


Presubmit checks took 6.4s to calculate.

Was the presubmit check useful? If not, run "git cl presubmit -v"
to figure out which PRESUBMIT.py was run, then run git blame
on the file to figure out who to ask for help.

queue was fixing with
https://github.com/bmsdave/v8/commit/f01480af6d3e98f17472c7a9e2b00c1400be2d7b

bmsdave on 5 Feb 2019

I create request to review -> https://chromium-review.googlesource.com/c/v8/v8/+/1454477

I propose to transfer the discussion there.

bmsdave on 5 Feb 2019

🚀2 ❤1 🎉1 👍1

The request 1454477 was splitted into these parts:
https://chromium-review.googlesource.com/c/v8/v8/+/1470121
https://chromium-review.googlesource.com/c/v8/v8/+/1470101
https://chromium-review.googlesource.com/c/v8/v8/+/1470122
https://chromium-review.googlesource.com/c/v8/v8/+/1470123

Is is merged

bmsdave on 19 Feb 2019

How do we close out this issue? The work has been done upstream.

cclauss on 15 Mar 2019

We'll open the PR to update V8 to version 7.4 (which contains the fixes) today

targos on 15 Mar 2019

👍3

Just to clarify: all v8 build scripts can run and are tested in CI on py3 as well as py2? So it's not just syntax that's updated but code now can be successfully executed in either py2 or py3?

If so, fantastic work!

hayd on 15 Mar 2019

I believe that most Node build machines are currently Python 2-only. See: nodejs/build#1674

cclauss on 15 Mar 2019

Is it the case in v8 CI tooling is still py2 only?
If not, have all v8 scripts actually been run in py3, or is it expected there are still many issues to uncover and overcome?

hayd on 15 Mar 2019

@hayd Unfortunately, there is still a lot of work. So far, we have only fixed almost all the problems pointed to by the static analyzer. But v8 CI tooling is still py2 only and before actually running on python3, it seems a long way off.

But we're getting closer)

bmsdave on 18 Mar 2019

👍4

On the static analyzer side, this is getting tantalizingly close... Update at https://github.com/v8/v8/pull/29#issuecomment-478024054

cclauss on 29 Mar 2019

__print()__ is a function in Python 3.
__execfile()__ was removed in Python 3.

__flake8__ testing of https://github.com/v8/v8 on Python 3.7.1

$ __flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics__

./tools/wasm-compilation-hints/wasm-objdump-compilation-hints.py:37:54: E999 SyntaxError: invalid syntax
          print "Custom section compilationHints with", num_hints, "hints:"
                                                     ^
./test/preparser/testcfg.py:69:5: F821 undefined name 'execfile'
    execfile(pathname, {"Test": Test, "Template": Template})
    ^
1     E999 SyntaxError: invalid syntax
1     F821 undefined name 'execfile'
2

@rryk @bmsdave @targos

cclauss on 13 Aug 2019

@bnoordhuis More upstreaming required to cover the basics of Python 3 compatibility.

cclauss on 2 Sep 2019

We are backsliding: https://github.com/v8/v8/pull/29#issuecomment-537490614

cclauss on 2 Oct 2019

Will those changes be backported to v10 LTS?

12101111 on 13 Oct 2019

I apologize for the long absence. I'm ready to continue.
I send PR from @AgentJ08 to https://chromium-review.googlesource.com/c/v8/v8/+/1864942

bmsdave on 17 Oct 2019

👍1

It looks like all the requirements are done:
https://travis-ci.com/bmsdave/v8/builds/132539826

bmsdave on 18 Oct 2019

Passing flake is not passing tests. Is this part of v8's CI? Premature to close IMO.

hayd on 18 Oct 2019

@hayd Do you have a set of test cases that fail?

cclauss on 18 Oct 2019

Reopening. We need to vendor in a new copy of deps/v8

https://travis-ci.com/nodejs/node/jobs/249904779#L252-L257

cclauss on 27 Oct 2019

How can we solve this problem?

Will updating v8 to node.js solve it?
Or can I make changes to this part https://github.com/nodejs/node/tree/master/deps/v8?

bmsdave on 28 Oct 2019

I think we need @targos and/or @Trott to weigh in on that.

cclauss on 28 Oct 2019

I don't really consider this a problem, because we don't use the scripts that are not compatible with Python 3.
If it is fixed at the V8 level (https://github.com/v8/v8), we will inherit that when we upgrade V8.
The next update will be to version 7.9: https://github.com/nodejs/node/pull/30020

targos on 28 Oct 2019

https://github.com/v8/v8 passes these tests.

cclauss on 28 Oct 2019

It's good that V8 itself is now compatible but other V8-related tools are not.

See https://ci.nodejs.org/job/node-test-commit-v8-linux/2590/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/console

08:09:01 Failed to fetch file gs://chromium-clang-format/942fc8b1789144b8071d3fc03ff0fcbe1cf81ac8 for v8/buildtools/linux64/clang-format. [Err: gsutil requires python 2.6 or 2.7.
08:09:01 ]
08:09:01 0> Failed to fetch file gs://chromium-clang-format/942fc8b1789144b8071d3fc03ff0fcbe1cf81ac8 for v8/buildtools/linux64/clang-format, skipping. [Err: gsutil requires python 2.6 or 2.7.

By looking at https://github.com/GoogleCloudPlatform/gsutil, gsutil seems to support Python 3 so maybe the version used by depot_tools is too old?

targos on 28 Oct 2019

Should this issue stay open? I believe this issue is outdated as Node.js build chain is compatible with Python 3.

aduh95 on 28 Apr 2020

@nodejs/v8 @nodejs/build ... what do you think? There doesn't seem to be reason to keep this open?

jasnell on 28 Apr 2020

This test would crash on Python 3 but that seem like a simple fix that is not on the critical path.

./test/debugging/wasm/gdb-server/gdb_rsp.py:35:14: F821 undefined name 'xrange'
    for i in xrange(int(timeout_in_seconds / poll_time_in_seconds)):
             ^
./test/debugging/wasm/gdb-server/gdb_rsp.py:67:8: F821 undefined name 'sys'
    if sys.platform == 'win32':
       ^

cclauss on 28 Apr 2020

The issue is still valid. V8 build tools are not yet all compatible with Python 3:

$ gclient sync
Warning: Running gclient on Python 3. 
If you encounter any issues, please file a bug on crbug.com under the Infra>SDK component.
Syncing projects: 100% (28/28), done.                                                

________ running 'vpython v8/third_party/depot_tools/update_depot_tools_toggle.py --disable' in '/home/mzasso/git/chromium/v8'
[E2020-04-28T19:19:27.955163+02:00 229437 0 annotate.go:241] goroutine 1:
[E2020-04-28T19:19:27.955189+02:00 229437 0 annotate.go:241] #0 go.chromium.org/luci/vpython/python/find.go:93 - python.Find()
[E2020-04-28T19:19:27.955199+02:00 229437 0 annotate.go:241]   annotation #0:
[E2020-04-28T19:19:27.955205+02:00 229437 0 annotate.go:241]     reason: no Python found
[E2020-04-28T19:19:27.955210+02:00 229437 0 annotate.go:241]   annotation #1:
[E2020-04-28T19:19:27.955215+02:00 229437 0 annotate.go:241]     reason: could not find appropriate executable for: "python2.7"

I don't know if we should keep this open though, there's nothing we can do about it in Node.js.

targos on 28 Apr 2020

IMO that should be tracked upstream, unless Node.js folks are actively working with V8 and Chromium to make the tools Python 3-compatible. Otherwise, as @targos mentioned, there's nothing we can do on our side.

It's also not a huge deal since it only affects folks building V8 through make-v8, which is limited to a subset of collaborators working on V8 upgrades on Node.js.

mmarchini on 28 Apr 2020

@targos, What happens when you add the 3??

vpython3 v8/third_party/depot_tools/update_depot_tools_toggle.py --disable' in '/home/mzasso/git/chromium/v8'
       ^

cclauss on 28 Apr 2020

@cclauss nothing is printed, exits with code 0

targos on 28 Apr 2020

😕1

Agreed with @targos and @mmarchini that this issue should be tracked upstream.

ryzokuken on 29 Apr 2020

Node: Dependency v8 is not Python 3 compatible

Most helpful comment

All 64 comments

Related issues