Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V8 compilation error on PPC #119

Closed
targos opened this issue Oct 28, 2019 · 28 comments
Closed

V8 compilation error on PPC #119

targos opened this issue Oct 28, 2019 · 28 comments

Comments

@targos
Copy link
Member

targos commented Oct 28, 2019

See: https://ci.nodejs.org/job/node-test-commit-v8-linux/2590/nodes=ppcle-ubuntu1404,v8test=v8test/console

08:07:55 cc1plus: error: unrecognized command line option '-Wno-class-memaccess' [-Werror]
08:07:55 cc1plus: error: unrecognized command line option '-Wno-packed-not-aligned' [-Werror]
08:07:55 cc1plus: all warnings being treated as errors

@nodejs/platform-ppc

@miladfarca
Copy link

It seems like v8_enable_backtrace = true is missing from GN flags, could you try to add this flag and recompile V8.

@targos
Copy link
Member Author

targos commented Oct 28, 2019

where should I add the flag?

@miladfarca
Copy link

Flag should be added to the Jenkins job which runs gn. The above console output shows the current flag is set to the following:

gn gen out.gn/ppc64.release '--args=is_component_build=false is_debug=false use_goma=false goma_dir="None" use_custom_libcxx=false v8_target_cpu="ppc64" target_cpu="ppc64"'

v8_enable_backtrace = true can be be added to the args list.

targos added a commit to nodejs/node that referenced this issue Oct 29, 2019
@targos
Copy link
Member Author

targos commented Oct 29, 2019

@targos
Copy link
Member Author

targos commented Oct 29, 2019

It builds, but hits another error:

08:57:39 [1479/1479] LINK ./cctest
08:57:39 deps/v8/tools/run-tests.py --gn --arch=ppc64 \
08:57:39 				--mode=release --progress=dots --timeout=120 \
08:57:39 				mjsunit cctest debugger inspector message preparser \
08:57:39 				--junitout /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/v8-tap.xml
08:57:40 Traceback (most recent call last):
08:57:40   File "/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/tools/testrunner/base_runner.py", line 281, in execute
08:57:40     tests = self._load_testsuite_generators(args, options)
08:57:40   File "/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/tools/testrunner/base_runner.py", line 651, in _load_testsuite_generators
08:57:40     self.framework_name)
08:57:40   File "/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/tools/testrunner/local/testsuite.py", line 254, in Load
08:57:40     with _load_testsuite_module(name, root) as module:
08:57:40   File "/usr/lib/python2.7/contextlib.py", line 17, in __enter__
08:57:40     return self.gen.next()
08:57:40   File "/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/tools/testrunner/local/testsuite.py", line 245, in _load_testsuite_module
08:57:40     yield imp.load_module(name + "_testcfg", f, pathname, description)
08:57:40 SyntaxError: unqualified exec is not allowed in function '_ParsePythonTestTemplates' it contains a nested function with free variables (testcfg.py, line 71)

@miladfarca
Copy link

miladfarca commented Oct 30, 2019

I checked out node-v8 on a ppc, fetched dependencies according the the console output and tests start normally.

I did however get the above error on a V8 checkout with older dependencies, running "gclient sync" fixed it.

Could you try rerunning a fresh Jenkins job? I also don't have access to that machine, could you please add my key for any further investigations, I got permission here: nodejs/build#1706

ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDQZihjEXcY52UZo09CEb29HwOWwXcmwbwZFR4rsACQQyGUexL4fkVrFfwuG8eV1vg5KECsO8LiGY/MHkNIpABaJJoip0Qxgv0pAFtAukIDjLXXOV/VNJjfIto16vOAehRZkmI+BtQP8TjoT2CSyJgvVQcay8BhH52in1LQQsyCi2crHLYzDrrCgY/rAmuVb1MzMnT8mFOdJ8E5RBhjnmc1K4YBKmNTf6yefgbOJssI0lLzp7Q2uytzp3pipg7AO/VqmRn8953UTJS/cOQeBi3nCYGpz4I7kOKHgwbdW1IP/XFfm0KO5daulHQeToRGIE85ntxF314wsYE3ZyeJwKgH

@targos
Copy link
Member Author

targos commented Oct 30, 2019

/cc @nodejs/build-infra ^

targos added a commit to nodejs/node that referenced this issue Oct 30, 2019
@sam-github
Copy link

Anyone with @nodejs/build access can add authorized keys to test machines (if an issue exists to say its OK!). I added @miladfarca.

@miladfarca
Copy link

miladfarca commented Oct 31, 2019

There seems to be a bug in Python versions prior to 2.7.9 which produces the above error:
https://bugs.python.org/issue21591
pyinstaller/pyinstaller#1408
django-extensions/django-extensions#1219

Python version on this machine is 2.7.6. My internal box has 2.7.15+ which has the bug fixed.

This is the V8 CL which has introduced this issue:
https://chromium-review.googlesource.com/c/v8/v8/+/1864942

I have asked the owner for a possible workaround (or python has to be upgraded/patched).

@sam-github
Copy link

I can schedule python updates if it ends up that is what's necessary. It'll require updates to all the ansilbe scripts that build python for those hosts (likely, unless some are getting it from package repos with updates, but thats not likely).

I'd need an explicit list of hosts that need the update - all *ppc64*? just ubuntu? just centos? Its not clear ATM.

Also, I'd need a minimum python2 version.

@miladfarca
Copy link

miladfarca commented Nov 1, 2019

Thanks Sam,
I have proposed a change for a workaround in this CL, waiting for a review:
https://chromium-review.googlesource.com/c/v8/v8/+/1893647

Let's see if it gets accepted considering Python 3 is going to replace it sometime very soon. If not then we might have to upgrade Python to at-least 2.7.9 (preferably 2.7.15+).

@miladfarca
Copy link

above CL is now landed. Issue should be solved on the next lkgr build.

@targos
Copy link
Member Author

targos commented Nov 4, 2019

Thanks.

Before closing this, can I ask why v8_enable_backtrace=true fixed the issue?
Should we also set it in our .gyp files to build Node.js?

@miladfarca
Copy link

miladfarca commented Nov 4, 2019

We always had this flag in our V8 internal builds, I feel this might be a gcc limitation where not having the backtrace symbols causes lambda related syntax errors. More info can be found here:
https://www.gnu.org/software/libc/manual/html_node/Backtraces.html
Might be a good idea to also add it to .gyp files.

targos added a commit to nodejs/node that referenced this issue Feb 20, 2020
mmarchini pushed a commit to mmarchini/node that referenced this issue Mar 4, 2020
mmarchini pushed a commit to mmarchini/node that referenced this issue Mar 6, 2020
mmarchini pushed a commit to mmarchini/node that referenced this issue Mar 6, 2020
mmarchini pushed a commit to nodejs/node that referenced this issue Mar 8, 2020
Refs: nodejs/node-v8#119

PR-URL: #32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit to nodejs/node that referenced this issue Mar 9, 2020
Refs: nodejs/node-v8#119

PR-URL: #32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
codebytere pushed a commit to nodejs/node that referenced this issue Mar 16, 2020
Refs: nodejs/node-v8#119

PR-URL: #32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
codebytere pushed a commit to nodejs/node that referenced this issue Mar 17, 2020
Refs: nodejs/node-v8#119

PR-URL: #32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
codebytere pushed a commit to nodejs/node that referenced this issue Mar 23, 2020
Refs: nodejs/node-v8#119

PR-URL: #32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
codebytere pushed a commit to nodejs/node that referenced this issue Mar 30, 2020
Refs: nodejs/node-v8#119

PR-URL: #32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
richardlau pushed a commit to richardlau/node-1 that referenced this issue Oct 5, 2020
Refs: nodejs/node-v8#119

PR-URL: nodejs#32113
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants