build: avoid building V8 snapshot if --without-snapshot#28467
build: avoid building V8 snapshot if --without-snapshot#28467targos wants to merge 1 commit intonodejs:masterfrom
Conversation
|
cc @joyeecheung |
joyeecheung
left a comment
There was a problem hiding this comment.
nit for the commit message: “avoid building the default v8 snapshot...” for clarity (or maybe not, but when I saw the title I thought this was to make —without-snapshot imply —without-node-snapshot)
|
Refs: #27838 (comment) |
common.gypi
Outdated
There was a problem hiding this comment.
Can we de-dup this a little bit?
['v8_use_snapshot==1', {
'v8_lib_name': 'snapshot',
}, {
'v8_lib_name': 'nosnapshot',
}],
['GENERATOR == "ninja"', {
'obj_dir': '<(PRODUCT_DIR)/obj',
'v8_base': '<(PRODUCT_DIR)/obj/tools/v8_gypfiles/libv8_<(v8_lib_name).a',
}, {
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base': '<(PRODUCT_DIR)/obj.target/tools/v8_gypfiles/libv8_<(v8_lib_name).a',
}],
['OS == "win"', {
'obj_dir': '<(PRODUCT_DIR)/obj',
'v8_base': '<(PRODUCT_DIR)/lib/libv8_<(v8_lib_name).a',
}],
['OS == "mac"', {
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base': '<(PRODUCT_DIR)/libv8_<(v8_lib_name).a',
}],
There was a problem hiding this comment.
gyp: Undefined variable v8_lib_name in /home/mzasso/git/nodejs/node/node.gyp while trying to load /home/mzasso/git/nodejs/node/node.gyp
|
Fwiw we want to remove no snapshot builds from V8 by the end of this year. We are working on the blockers of that right now. Reason is that it now takes unreasonable amount of time to run without snapshot, and also doesn't match what we ship. |
|
That's good to know. We should probably remove or at least deprecate nosnapshot support in v13 then. |
|
The same tests failed three times in a row. It seems unlikely to be a flake, but they didn't fail in previous runs. I don't know what to do now... |
|
It should be skipped if Node.js is built with the --without-snapshot configure flag.
b3dbe01 to
995ecd3
Compare
|
I rebased on master and reworded the commit message. Maybe CI will pass now. |
|
Landed in a8ed416 |
It should be skipped if Node.js is built with the --without-snapshot configure flag. PR-URL: #28467 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
It should be skipped if Node.js is built with the --without-snapshot configure flag. PR-URL: #28467 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
/cc @bnoordhuis @refack @ryzokuken