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

obuild: update node.gyp to reference gypfiles/v8.gyp #37

Closed
wants to merge 12 commits into from

Conversation

joyeecheung
Copy link
Member

Refs: v8/v8@f9934aa
Fixes: #36

Also noticed that parallel/test-inspector-esm currently fails because the scope chain and the format of the responses from the inspector have changed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

nodejs-ci and others added 12 commits January 24, 2018 06:55
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 6.6.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
It is necessary to enable more C++ features in order to build V8 6.4.
This commit renames V8DBG_CLASS_MAP__INSTANCE_ATTRIBUTES__INT
to V8DBG_CLASS_MAP__INSTANCE_TYPE__UINT16_T following upstream changes.
This commit updates the following postmortem metadata constants:

- v8dbg_class_Map__inobject_properties_or_constructor_function_index__int
  - This is now
    v8dbg_class_Map__inobject_properties_start_or_constructor_function_index__char
    as of
    v8/v8@61bf2cc
- v8dbg_class_Map__instance_attributes__int
  - This is now v8dbg_class_Map__instance_type__uint16_t as of
    v8/v8@c00bb6d
    and
    v8/v8@cb46310
- v8dbg_class_Map__instance_size__int
  - This is now v8dbg_class_Map__instance_size_in_words__char as of
    v8/v8@61bf2cc

Refs: nodejs#34

Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
This commit updates the following postmortem metadata constant:

- v8dbg_bit_field3_dictionary_map_shift
  - This is now v8dbg_bit_field3_is_dictionary_map_shift as of
    v8/v8@7a159da

Refs: nodejs#34

Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@hashseed
Copy link
Member

hashseed commented Jan 26, 2018

LGTM.

Also note that I already fixed parallel/test-inspector-esm on node/master. It was not due to the format of the inspector response, but because the scoping for modules changed in V8.

@targos targos changed the title build: update node.gyp to reference gypfiles/v8.gyp obuild: update node.gyp to reference gypfiles/v8.gyp Jan 26, 2018
@targos
Copy link
Member

targos commented Jan 26, 2018

@joyeecheung thanks, I was working on it yesterday but couldn't finish. I am unable to compile though, here is the error I get (with V8 lkgr):

... member /home/mzasso/git/nodejs/node-canary/out/Release/obj.target/v8_base/deps/v8/src/api.o in archive is not an object
collect2: error: ld returned 1 exit status

@targos
Copy link
Member

targos commented Jan 26, 2018

diff --git a/Makefile b/Makefile
index f2ceb6e029..462fd283c9 100644
--- a/Makefile
+++ b/Makefile
@@ -96,7 +96,7 @@ $(NODE_G_EXE): config.gypi out/Makefile
 
 out/Makefile: common.gypi deps/uv/uv.gyp deps/http_parser/http_parser.gyp \
               deps/zlib/zlib.gyp deps/v8/gypfiles/toolchain.gypi \
-              deps/v8/gypfiles/features.gypi deps/v8/src/v8.gyp node.gyp \
+              deps/v8/gypfiles/features.gypi deps/v8/gypfiles/v8.gyp node.gyp \
               config.gypi
        $(PYTHON) tools/gyp_node.py -f make
 
diff --git a/node.gypi b/node.gypi
index bc9db9b814..f1029f2571 100644
--- a/node.gypi
+++ b/node.gypi
@@ -86,7 +86,7 @@
        target_arch=="ia32" or target_arch=="x32")', {
       'defines': [ 'NODE_ENABLE_VTUNE_PROFILING' ],
       'dependencies': [
-        'deps/v8/src/third_party/vtune/v8vtune.gyp:v8_vtune'
+        'deps/v8/gypfiles/v8vtune.gyp:v8_vtune'
       ],
     }],
     [ 'node_no_browser_globals=="true"', {

@joyeecheung I'm adding this diff to your commit and trying to run CI

@targos
Copy link
Member

targos commented Jan 26, 2018

@targos
Copy link
Member

targos commented Jan 26, 2018

Error on CI: g++: error: /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/out/Release/obj.target/deps/v8/src/libv8_base.a: No such file or directory

@hashseed
Copy link
Member

hashseed commented Jan 26, 2018

I think you also need to change the paths in common.gypi since libv8_base.a is no longer in obj.target/deps/v8/src/, but instead in obj.target/deps/v8/gypfiles.

diff --git a/common.gypi b/common.gypi
index cdc11bd909..3a328129d2 100644
--- a/common.gypi
+++ b/common.gypi
@@ -45,14 +45,14 @@
     'conditions': [
       ['GENERATOR=="ninja"', {
         'OBJ_DIR': '<(PRODUCT_DIR)/obj',
-        'V8_BASE': '<(PRODUCT_DIR)/obj/deps/v8/src/libv8_base.a',
+        'V8_BASE': '<(PRODUCT_DIR)/obj/deps/v8/gypfiles/libv8_base.a',
        }, {
          'OBJ_DIR%': '<(PRODUCT_DIR)/obj.target',
          'conditions': [
            [ 'build_v8_with_gn=="true"', {
              'V8_BASE': '<(PRODUCT_DIR)/obj.target/v8_monolith/geni/gn/obj/libv8_monolith.a',
            }, {
-             'V8_BASE': '<(PRODUCT_DIR)/obj.target/deps/v8/src/libv8_base.a',
+             'V8_BASE': '<(PRODUCT_DIR)/obj.target/deps/v8/gypfiles/libv8_base.a',
            }],
          ],
       }],

@hashseed
Copy link
Member

Err... that patch doesn't apply on your branch since it's for our vee-eight-lkgr.

@targos
Copy link
Member

targos commented Jan 26, 2018

@targos
Copy link
Member

targos commented Jan 26, 2018

This is resolved!

@targos targos closed this Jan 26, 2018
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

Successfully merging this pull request may close these issues.

Cannot compile canary
6 participants