V8 gyp scaffolding responsibility handoff #411

refack · 2017-11-06T22:56:13Z

Refs: nodejs/CTC#76 (comment)
Refs: nodejs/CTC#2 (comment)

... the legacy GYP config will be removed from V8’s repository, but stays in node/deps/v8. Keeping it in sync with GN build config becomes Node.js’ responsibility ...

AFAICT V8 has already stopped CI testing the .gyp scaffolding, and are planning to remove the .gyp files by the end of the year.

Should we designate an "owner" for those files?
Should we keep upstreaming fixes meanwhile (deps: update V8 to 6.2.414.44 node#16848 (comment))?

/CC @nodejs/v8 @nodejs/gyp @nodejs/build @targos @hashseed

The text was updated successfully, but these errors were encountered:

fhinkel · 2017-11-07T06:55:58Z

Our bot is still running the regular Node make with gyp just like official Node.

hashseed · 2017-11-07T07:05:23Z

Precisely. We still test Node with gyp. We have plans to change this, and will notify once we have something usable.

refack · 2017-11-07T18:33:14Z

Our bot is still running the regular Node make with gyp just like official Node.

Obviously any CI coverage provided by V8 is greatly appreciated!
AFAICT there seems to be coverage missing - ARM was broken for a while, and Windows wasn't covered (maybe just building with MSBuild vs. ninja? While we use MSBuild and both VS2015 and VS2017).

IMHO the deprecation of the gyp scaffolding, and the pending removal is starting to be felt by node, and I'm just saying we should be ready. This way we won't need to scramble last minute to smooth over build problems when we want to backport fixes (nodejs/node#16848 (comment)).

Another point is that ATM reviewing these patches is cumbersome, since they can only land together with V8 backports or version bumps, see nodejs/node#16848 (comment)

refack · 2017-12-06T23:08:16Z

To reiterate, we already have knowledge and time investment in maintaining GYP (the tool), so taking ownership of the tool and the .gyp files is not too far fetched.

seishun · 2018-01-05T13:30:50Z

@hashseed It's 2018, any news?

hashseed · 2018-01-05T14:37:03Z

In the meantime I created a wrapper script as planned to create a libv8_monolith.a static library that can be used to link node instead of libv8_base.a. I haven't created a PR on the Node.js side yet to make things work smoothly.

To build node without using V8's gyp configs, perform following steps:

check out branch vee-eight-lkgr from https://github.com/v8/node
apply following patch

diff --git a/common.gypi b/common.gypi
index ba44178169..2e16bacb2b 100644
--- a/common.gypi
+++ b/common.gypi
@@ -48,7 +48,7 @@
         'V8_BASE': '<(PRODUCT_DIR)/obj/deps/v8/src/libv8_base.a',
        }, {
          'OBJ_DIR%': '<(PRODUCT_DIR)/obj.target',
-         'V8_BASE%': '<(PRODUCT_DIR)/obj.target/deps/v8/src/libv8_base.a',
+         'V8_BASE%': '<(PRODUCT_DIR)/obj.target/v8_monolith/geni/gn/obj/libv8_monolith.a',
       }],
       ['OS == "win"', {
         'os_posix': 0,
diff --git a/node.gypi b/node.gypi
index 718ef0c9e7..e88bc0520b 100644
--- a/node.gypi
+++ b/node.gypi
@@ -17,7 +17,7 @@
     }],
     [ 'node_use_bundled_v8=="true"', {
       'dependencies': [
-        'deps/v8/src/v8.gyp:v8',
+        'deps/v8/src/v8.gyp:v8_monolith',
         'deps/v8/src/v8.gyp:v8_libplatform'
       ],
     }],

chdir into the node checkout
run deps/v8/tools/node/fetch_deps.py deps/v8 force to fetch the necessary dependencies.
run ./configure && make -j 4 node. This uses deps/v8/tools/node/build_gn.py to build out/Release/obj.target/deps/v8/src/libv8_monolith.a to link into the final executable.

I only tested this with a static build for x64 linux. Once that's usable and streamlined I'll work on a few more build configurations.

seishun · 2018-01-05T17:13:00Z

Is there any difference between v8 and v8_monolith other than the way it's built?

hashseed · 2018-01-05T17:51:34Z

Not really. Except that v8 is a collection of files and v8_monolith just one.

seishun · 2018-01-05T19:22:38Z

run deps/v8/tools/node/fetch_deps.py deps/v8 to fetch the necessary dependencies.

The file deps/v8/tools/node/fetch_deps.py does not exist.

hashseed · 2018-01-05T19:28:02Z

Did you check out from github.com/v8/node? I landed it in the last few weeks so it's not yet in node master.

seishun · 2018-01-05T19:30:50Z

Yes, there's no node directory here: https://github.com/v8/node/tree/vee-eight-lkgr/deps/v8/tools

hashseed · 2018-01-05T20:04:08Z

You are right. Turns out anything called node is ignored by git due to Node's root .gitignore. So when I updated our fork's V8 via script, the whole folder was not added.

I fixed that in our node fork here. I'll upstream that soonish. Thanks for making me aware!

In the meantime I also push a new version of V8 to our fork, with the node folder.

seishun · 2018-01-05T20:23:23Z

fetch_deps.py doesn't do anything. It seems intentional: https://github.com/v8/node/blob/bc4ca529a007c9f23acd87534197086b2f10e65e/deps/v8/tools/node/fetch_deps.py#L89-L91

hashseed · 2018-01-05T21:08:02Z

Ugh. Yeah that's a detail I forgot. We wanted to get this landed first and set up the infra changes before enabling it. Just removing pass and uncommenting the function call should do the trick.

seishun · 2018-01-05T21:25:08Z

It doesn't seem to work. Here's the output:

seishun@ns396737:~/v8-node$ deps/v8/tools/node/fetch_deps.py deps/v8
Using depot tools in /home/seishun/v8-node/deps/v8/_depot_tools
Initializing temporary git repository in v8.
Initialized empty Git repository in /home/seishun/v8-node/deps/v8/.git/
[master (root-commit) ede811c] init
Fetching dependencies.
Syncing projects: 100% (23/23), done.                                   
Traceback (most recent call last):
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient.py", line 2679, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient.py", line 2665, in main
    return dispatcher.execute(OptionParser(), argv)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient.py", line 2420, in CMDsync
    ret = client.RunOnDeps('update', args)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient.py", line 1510, in RunOnDeps
    self.RunHooksRecursively(self._options, pm)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient.py", line 1033, in RunHooksRecursively
    hook.run(self.root.root_dir)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient.py", line 219, in run
    cmd, cwd=cwd, always=self._verbose)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient_utils.py", line 314, in CheckCallAndFilterAndHeader
    return CheckCallAndFilter(args, **kwargs)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/gclient_utils.py", line 509, in CheckCallAndFilter
    **kwargs)
  File "/home/seishun/v8-node/deps/v8/_depot_tools/subprocess2.py", line 262, in __init__
    % (str(e), kwargs.get('cwd'), args[0]))
OSError: Execution failed with error: [Errno 2] No such file or directory.
Check that . or download_from_google_storage exist and have execution permission.
Uninitializing temporary git repository
>> Cleaning up /home/seishun/v8-node/deps/v8/.git
Traceback (most recent call last):
  File "deps/v8/tools/node/fetch_deps.py", line 90, in <module>
    Main(sys.argv[1])
  File "deps/v8/tools/node/fetch_deps.py", line 86, in Main
    FetchDeps(v8_path, depot_tools)
  File "deps/v8/tools/node/fetch_deps.py", line 70, in FetchDeps
    cwd=os.path.join(v8_path, os.path.pardir))
  File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/home/seishun/v8-node/deps/v8/_depot_tools/gclient', 'sync', '--spec', "solutions = [{'managed': False, 'custom_vars': {'build_for_node': True}, 'url': 'https://chromium.googlesource.com/v8/v8.git', 'custom_deps': {'v8/third_party/android_tools': None, 'v8/test/wasm-js': None, 'v8/test/benchmarks/data': None, 'v8/test/mozilla/data': None, 'v8/third_party/colorama/src': None, 'v8/testing/gmock': None, 'v8/third_party/markupsafe': None, 'v8/test/test262/data': None, 'v8/testing/gtest': None, 'v8/third_party/catapult': None, 'v8/third_party/jinja2': None, 'v8/test/test262/harness': None, 'v8/tools/swarming_client': None, 'v8/tools/luci-go': None, 'v8/base/trace_event/common': None, 'v8/third_party/instrumented_libraries': None, 'v8/tools/gyp': None}, 'deps_file': 'DEPS', 'name': 'v8'}]"]' returned non-zero exit status 1

hashseed · 2018-01-05T21:30:27Z

Hm yeah something seems to be missing. I'll look into it next week. Thanks for the feedback!

hashseed · 2018-01-05T22:46:06Z

I figured it out. gclient needs depot_tools to be included in PATH to work. I fixed the script and am pushing the fix to v8/node/vee-eight-lkgr.

Sorry about all the trouble. I'm testing this again on a clean machine to make sure.

hashseed · 2018-01-05T22:59:40Z

Seems to work for me now. I also changed the fetch_deps.py script to accept a second argument to skip the patching.
deps/v8/tools/node/fetch_deps.py deps/v8 force should work.

seishun · 2018-01-06T10:55:36Z

There's a different error now:

seishun@ns396737:~/v8-node$ deps/v8/tools/node/fetch_deps.py deps/v8 force
Checking out depot_tools.
Cloning into '/home/seishun/v8-node/deps/v8/_depot_tools'...
remote: Sending approximately 29.77 MiB ...
remote: Total 25020 (delta 16153), reused 25020 (delta 16153)
Receiving objects: 100% (25020/25020), 29.77 MiB | 23.36 MiB/s, done.
Resolving deltas: 100% (16153/16153), done.
Checking connectivity... done.
Using depot tools in /home/seishun/v8-node/deps/v8/_depot_tools
Initializing temporary git repository in v8.
Initialized empty Git repository in /home/seishun/v8-node/deps/v8/.git/
[master (root-commit) 70c7515] init
Fetching dependencies.
Bootstrapping cipd client for linux-amd64 from https://chrome-infra-packages.appspot.com/client?platform=linux-amd64&version=git_revision:2288f0cb47eea8b5430882867099586afcee2368...
Syncing projects: 100% (23/23), done.                                   
Running hooks:  95% (22/23) regyp_if_needed     
________ running '/usr/bin/python v8/gypfiles/gyp_v8 --running-as-hook' in '.'
Traceback (most recent call last):
  File "v8/gypfiles/gyp_v8", line 46, in <module>
    import gyp
ImportError: No module named gyp
Error: Command '/usr/bin/python v8/gypfiles/gyp_v8 --running-as-hook' returned non-zero exit status 1 in .
Uninitializing temporary git repository
>> Cleaning up /home/seishun/v8-node/deps/v8/.git
Traceback (most recent call last):
  File "deps/v8/tools/node/fetch_deps.py", line 93, in <module>
    Main(sys.argv[1])
  File "deps/v8/tools/node/fetch_deps.py", line 89, in Main
    FetchDeps(v8_path, depot_tools)
  File "deps/v8/tools/node/fetch_deps.py", line 73, in FetchDeps
    env=env)
  File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['gclient', 'sync', '--spec', "solutions = [{'managed': False, 'custom_vars': {'build_for_node': True}, 'url': 'https://chromium.googlesource.com/v8/v8.git', 'custom_deps': {'v8/third_party/android_tools': None, 'v8/test/wasm-js': None, 'v8/test/benchmarks/data': None, 'v8/test/mozilla/data': None, 'v8/third_party/colorama/src': None, 'v8/testing/gmock': None, 'v8/third_party/markupsafe': None, 'v8/test/test262/data': None, 'v8/testing/gtest': None, 'v8/third_party/catapult': None, 'v8/third_party/jinja2': None, 'v8/test/test262/harness': None, 'v8/tools/swarming_client': None, 'v8/tools/luci-go': None, 'v8/base/trace_event/common': None, 'v8/third_party/instrumented_libraries': None, 'v8/tools/gyp': None}, 'deps_file': 'DEPS', 'name': 'v8'}]"]' returned non-zero exit status 2

hashseed · 2018-01-06T19:33:56Z

For some reason my python distribution has gyp as package included. Yours doesn't. But then again we should not need it at all.

Does this patch fix the issue?

[2018-01-06 20:32:59] yangguo@yangguo2:~/test/node3$ less patch

diff --git a/deps/v8/DEPS b/deps/v8/DEPS
index d79a0d1fb4..38293dfb4c 100644
--- a/deps/v8/DEPS
+++ b/deps/v8/DEPS
@@ -319,6 +319,7 @@ hooks = [
     # A change to a .gyp, .gypi, or to GYP itself should run the generator.
     'name': 'regyp_if_needed',
     'pattern': '.',
+    'condition': 'build_for_node != True',
     'action': ['python', 'v8/gypfiles/gyp_v8', '--running-as-hook'],
   },
   # Download and initialize "vpython" VirtualEnv environment packages.

seishun · 2018-01-06T21:21:49Z

Seems to work now.

Is this the final solution? I'm curious why you went with a fetch_deps.py script. I understand that checking in binaries is not ideal, but relying on a remote resource makes it susceptible to link rot, consequently there's no guarantee that this commit will be buildable in the future.

hashseed · 2018-01-06T21:35:22Z

Thanks for verifying!

This is not the "final" solution in the sense of that's where Node.js' build system should end up. The problem this should solve is that the V8 team is committed to test against Node.js, but wants to end support for gyp. This is not designed as a full replacement for V8's gyp configs.

In the short term, Node.js inherits ownership over V8's gyp configurations to continue building with pure gyp configs (no GN wrapper). Ultimately Node.js will have to figure out whether to keep maintaining its own and V8's gyp configurations, or find something better altogether.

These remote resources are ones that V8 relies on and are tested on our infrastructure, so for the purpose of V8 testing against Node.js, link rot is not an issue. However, for building Node.js without having to rely on V8 and Chrome resources, this would indeed be a problem, so the gyp configs have to be maintained.

seishun · 2018-01-07T13:03:18Z

Ultimately Node.js will have to figure out whether to keep maintaining its own and V8's gyp configurations, or find something better altogether.

That's the issue - if Node.js were to consider adopting GN (at least for V8), the dependence on V8 and Chrome resources might become an obstacle, considering the Node.js repo is currently self-contained.

hashseed · 2018-01-07T13:20:03Z

Having talked to the GN team in the past, I don't think GN is a viable option for Node.js to completely follow through. This gyp-gn bridge is really just to cut the dependency to gyp for our integration testing.

seishun · 2018-01-07T13:37:39Z

Having talked to the GN team in the past, I don't think GN is a viable option for Node.js to completely follow through.

Any reason for that besides the dependence on remote resources? And could you clarify what PR you meant in #411 (comment)? I assumed it referred to the GN wrapper.

hashseed · 2018-01-07T13:46:01Z

The doc lays out a couple of reasons. Mostly because GN does not support the full range of platforms that Node.js does.

The PR I have in mind is to introduce a build flag to Node.js to be able to use the gn wrapper, currently the patch in step 2.

seishun · 2018-01-07T14:51:26Z

Mostly because GN does not support the full range of platforms that Node.js does.

According to @refack, the V8 support for those platforms that are not supported by GN is maintained by companies interested in such support (IRC log). So it seems reasonable to expect those companies to help with GN support on those platforms as well if Node.js were to follow this route. It seems to me that it would be more efficient in the long run than maintaining gyp configs.

hashseed · 2018-01-07T16:01:45Z

A couple of thoughts:

Platforms not backed by companies, such as FreeBSD, will not receive any support.
I have been wanting to asking @mhdawson for ever how the GN support for S390 and PPC looks. I heard rumors that there was something being worked on.
GN relies on a number of platform-dependent configs and binaries. You will not get around of fetching these resources from somewhere, unless you want to include them in Node altogether.
GN is not designed for broad public adoption from what I gathered.
The advantage of GN over something new, e.g. cmake, is that V8 already comes with GN files. Configs for cmake would have to be maintained, just like for gyp in the near future. Bazel has the same advantage as GN.
I'm not even sure Chromium/V8 will stick with GN in, say, 5 years :P

seishun · 2018-01-07T17:57:58Z

Platforms not backed by companies, such as FreeBSD, will not receive any support.

Are there any other such platforms? FreeBSD is free and works on x86-64, so pretty much any collaborator can get involved with its support.

GN relies on a number of platform-dependent configs and binaries. You will not get around of fetching these resources from somewhere, unless you want to include them in Node altogether.

I think including them in Node is an option.

Bazel has the same advantage as GN.

In that case, Bazel is definitely worth looking into.

I'm not even sure Chromium/V8 will stick with GN in, say, 5 years :P

I'd say if the effort to adopt GN remains useful for 5 years, it would be more than worth it.

mhdawson · 2018-01-08T23:01:17Z

@hashseed I'll ask the people who were working on the GN support to comment here.

hashseed · 2018-01-12T14:09:31Z

I streamlined things a bit.

git clone https://github.com/v8/node v8-node
cd v8-node 
git checkout vee-eight-lkgr
./configure --build-v8-with-gn
make -j4 test

jasnell · 2018-02-17T00:44:55Z

Is there a next step here? Or can this issue be closed?

hashseed · 2018-02-17T07:00:23Z

Guess next steps will have to happen when upgrading to V8 6.6.

fhinkel · 2018-03-08T09:31:36Z

Closing via nodejs/node#19201, feel free to re-open if needed.

refack mentioned this issue Nov 6, 2017

deps: update V8 to 6.2.414.44 nodejs/node#16848

Closed

MylesBorins added the tsc-agenda label Dec 5, 2017

mhdawson mentioned this issue Dec 5, 2017

Node.js Foundation Technical Steering Committee (TSC) Meeting 2017-12-06 #432

Closed

Trott removed the tsc-agenda label Dec 6, 2017

seishun mentioned this issue Jan 12, 2018

RFC: Bazel for build system #464

Closed

hashseed mentioned this issue Jan 26, 2018

Upcoming changes to GYP nodejs/node-v8#38

Closed

fhinkel closed this as completed Mar 8, 2018

hashseed mentioned this issue Apr 17, 2018

Support building with win-clang nodejs/node#19630

Closed

5 tasks

refack mentioned this issue Oct 17, 2018

Build node with GN nodejs/node#21410

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V8 gyp scaffolding responsibility handoff #411

V8 gyp scaffolding responsibility handoff #411

refack commented Nov 6, 2017

fhinkel commented Nov 7, 2017

hashseed commented Nov 7, 2017

refack commented Nov 7, 2017

refack commented Dec 6, 2017

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018 •

edited

Loading

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018 •

edited

Loading

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

hashseed commented Jan 5, 2018 •

edited

Loading

hashseed commented Jan 5, 2018

seishun commented Jan 6, 2018

hashseed commented Jan 6, 2018

seishun commented Jan 6, 2018

hashseed commented Jan 6, 2018

seishun commented Jan 7, 2018

hashseed commented Jan 7, 2018

seishun commented Jan 7, 2018

hashseed commented Jan 7, 2018

seishun commented Jan 7, 2018

hashseed commented Jan 7, 2018 •

edited

Loading

seishun commented Jan 7, 2018

mhdawson commented Jan 8, 2018

hashseed commented Jan 12, 2018

jasnell commented Feb 17, 2018

hashseed commented Feb 17, 2018

fhinkel commented Mar 8, 2018

V8 gyp scaffolding responsibility handoff #411

V8 gyp scaffolding responsibility handoff #411

Comments

refack commented Nov 6, 2017

fhinkel commented Nov 7, 2017

hashseed commented Nov 7, 2017

refack commented Nov 7, 2017

refack commented Dec 6, 2017

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018 • edited Loading

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018 • edited Loading

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

seishun commented Jan 5, 2018

hashseed commented Jan 5, 2018

hashseed commented Jan 5, 2018 • edited Loading

hashseed commented Jan 5, 2018

seishun commented Jan 6, 2018

hashseed commented Jan 6, 2018

seishun commented Jan 6, 2018

hashseed commented Jan 6, 2018

seishun commented Jan 7, 2018

hashseed commented Jan 7, 2018

seishun commented Jan 7, 2018

hashseed commented Jan 7, 2018

seishun commented Jan 7, 2018

hashseed commented Jan 7, 2018 • edited Loading

seishun commented Jan 7, 2018

mhdawson commented Jan 8, 2018

hashseed commented Jan 12, 2018

jasnell commented Feb 17, 2018

hashseed commented Feb 17, 2018

fhinkel commented Mar 8, 2018

hashseed commented Jan 5, 2018 •

edited

Loading

hashseed commented Jan 5, 2018 •

edited

Loading

hashseed commented Jan 5, 2018 •

edited

Loading

hashseed commented Jan 7, 2018 •

edited

Loading