-
Notifications
You must be signed in to change notification settings - Fork 167
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
Ninja not deployed to s390x and ppcle machines #1205
Comments
Ping @nodejs/build @rvagg @mhdawson This is blocking nodejs/node#19201 |
We are looking into this, we will probably need a set of ansible scripts to pull and build from ninja source on these platforms. |
Would it be possible for someone to test locally and chime in on the thread if the test suite passes or not?
… On Apr 3, 2018, at 12:59 PM, Muntasir Mallick ***@***.***> wrote:
We are looking into this, we will probably need a set of ansible scripts to pull and build from ninja source on these platforms.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#1205 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecVxtsFxlwEGdFmvZ4RVXpXeGyvRWaks5tk6qAgaJpZM4TDq2U>.
|
We've been working to get GN for PPC and s390x included in the google dev_tools but I see we've not made it in time to avoid problems. I think for now we can install temporarily on the machines themselves. Depending on how long we think getting them into dev_tools is going to take we may want to update the ansible scripts to build/install them, but I'd prefer they are pulled from the dev_tools like other platforms as that validates that v8 can be built more generally. |
/cc @nodejs/v8-update |
If it's just P.S. AFAICT it's also missing for |
P.S. AFAICT |
Its more than that, we believe we need GN as well and we are working on compiling that now. |
ppc64le is NOT deprecated, it was only the be version which is not part of this job as I've already removed it. |
We had pre-built binaries that we had used for earlier testing, but they have glibc dependencies that prevent them from running on the community CI machines. We are currently working on building on the CI machines as a temporary work around. |
In that case I'm assuming appending title |
Yes working on it for both PPC and s390 |
So we added gn and ninja binaries for PPC and there are still some issues as it is trying to use the x86 version of gn. @mmallick-ca will have a deeper look so that we can figure out how long we think it will take to fix it up. |
AFAICT this is still broken, any eta @mhdawson ? |
We have built a set of gn/ninja binaries and used gn's command line options to generate the ninja files to build v8 on our test machines. Just need to try it out on the community machines and submit a pull-request for the changes that will be required in make-v8.sh. @mhdawson and I will be getting together upcoming Monday to try out the binaries and install some of the dependencies. |
The s390x is now passing with medium term fix (adding pieces needed on build machine) until we get changes upstreamed. The same formula should would for PPC but we need to get some files and due to internal infra issue that may take a few days. |
Is the goal to build node for s390/ppc with ninja, or also v8 with gn via For the latter we have a minor issue where ICU is built twice, once with the v8-monolith target, and once as part of node. If the ICU versions mismatch there are some funny crashes. |
@hashseed, we are not doing any work on building node differently from the rest of the project. All we want to get working here is that |
We have a medium term fix where @mhdawson has installed the appropriate gn/ninja binaries and other dependencies on the community machine. We are using a patch to make-v8.sh to invoke them appropriately. In the long term we have to teach v8-gen.py to generate the appropriate gn command. But for now this should enable v8 tests to run. |
Latest CI runs are green :) https://ci.nodejs.org/job/node-test-commit-v8-linux/ |
Last thing I should cover before closing this out is adding to the doc how to restore the files if necessary. We could add to ansible but not 100% sure its worth doing that since it is temporary. @gdams can you add considering that to your todo list? |
If gn now works on all supported platforms, is there a reason to continue building V8 with Gyp? |
@seishun, we still have a long way to go before gn is supported on our platforms. As @mmallick-ca I've manually copied files/tools onto the our ci machines and we dynamically patch make-v8 to make it run. We will work to contribute changes back to the google infra but that's likely to take quite a while. |
Found this on index 4365412..0092d6c 100755
--- a/tools/make-v8.sh
+++ b/tools/make-v8.sh
@@ -5,5 +5,17 @@ V8_BUILD_OPTIONS=$2
cd deps/v8
tools/node/fetch_deps.py .
-PATH=~/_depot_tools:$PATH tools/dev/v8gen.py $BUILD_ARCH_TYPE --no-goma $V8_BUILD_OPTIONS
-PATH=~/_depot_tools:$PATH ninja -C out.gn/$BUILD_ARCH_TYPE/ d8 cctest inspector-test
+
+# set paths manually for now to use locally installed gn
+export BUILD_TOOLS=/home/iojs/build-tools
+export LD_LIBRARY_PATH=$BUILD_TOOLS:$LD_LIBRARY_PATH
+export PATH=$BUILD_TOOLS:$PATH
+CXX_PATH=`which $CXX |grep g++`
+rm -f "$BUILD_TOOLS/g++"
+rm -f "$BUILD_TOOLS/gcc"
+ln -s /usr/bin/$CXX "$BUILD_TOOLS/g++"
+ln -s /usr/bin/$CC "$BUILD_TOOLS/gcc"
+g++ --version
+export PKG_CONFIG_PATH=$BUILD_TOOLS/pkg-config-files
+gn gen out.gn/$BUILD_ARCH_TYPE --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"'
+ninja -C out.gn/$BUILD_ARCH_TYPE d8 cctest inspector-test Also seems to fails the linux s390x - https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel72-s390x,v8test=v8test/1442/console
|
If fixed the job to avoid trying to apply the patch on versions earlier that 10. Its required to build with gn. It is in the package -> build-tools-ppc.tar.gz in www/ppc in the downloads served by the ci server. Same for s390. As metioned in #1205 (comment) we still have to doc that or add to ansible before closing this issue. I've asked @gdams to add to his todo list in the above comment. |
is this fixed? |
I believe this is resolved in the context of the original problem. Closing. Please re-open if that is not the right thing to do. |
nodejs/node#19201 (comment)
Can we get that fixed?
The text was updated successfully, but these errors were encountered: