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.

Sign up for GitHub

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

Jump to bottom

deps,v8: link with atomic for platforms lacking CAS #23286

Closed
wants to merge 9 commits into from
Closed

deps,v8: link with atomic for platforms lacking CAS #23286

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c679d80
deps: update V8 to 7.1.0
targos Oct 5, 2018
7adef39
build: reset embedder string to "-node.0"
targos Oct 5, 2018
37bb409
src: update NODE_MODULE_VERSION to 68
targos Oct 5, 2018
193ee73
deps: update v8.gyp
targos Aug 29, 2018
34671d2
deps: update v8.gyp
targos Oct 5, 2018
d172625
deps: V8 sync gypfiles with 7.1
refack Oct 4, 2018
f84d485
deps,v8: link with `atomic` for platforms lacking CAS
refack Oct 5, 2018
6e24dcc
temprary fix for CI
refack Oct 2, 2018
24ffb72
fixup! 1
refack Oct 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ build-ci:
# - node-test-commit-linux-coverage: where the build and the tests need
# to be instrumented, see `coverage`.
run-ci: build-ci
$(MAKE) test-ci
$(MAKE) test-ci -j1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this WIP?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and no. for the canary branch I float this as 6e24dcc
It's either this or #23257. One of them will need to land upstream, since now we see #22006 on a lot of platforms (make test-ci rebuilds a few files and relinks the node binary, in parallel to the script that builds the addons, so it's a race to the binary)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's either this or #23257.

I’m not sure I follow … that PR landed, right?

I don’t think -j1 is something we want in CI…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure I follow … that PR landed, right?

#23257 was a revert of 5d8373a, because there was a bug on macOS.

IMHO running just the make part of test-ci with -j1 is not that bad. Most of the steps have parallelism built in

node/Makefile

Lines 457 to 467 in 2ba19ff

test-ci: | clear-stalled build-addons build-addons-napi doc-only
out/Release/cctest --gtest_output=tap:cctest.tap
$(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \
--mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \
$(TEST_CI_ARGS) $(CI_JS_SUITES) $(CI_NATIVE_SUITES) $(CI_DOC)
@echo "Clean up any leftover processes, error if found."
ps awwx | grep Release/node | grep -v grep | cat
@PS_OUT=`ps awwx | grep Release/node | grep -v grep | awk '{print $$1}'`; \
if [ "$${PS_OUT}" ]; then \
echo $${PS_OUT} | xargs kill -9; exit 1; \
fi

For build-addons && build-addons-napi you added the script that does multi-proc. doc-only had it for a long time. and the test uses $JOBS for parallelism level...


test-release: test-build
$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER)
Expand Down
45 changes: 36 additions & 9 deletions common.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,56 @@

'openssl_fips%': '',

# Default to -O0 for debug builds.
'v8_optimized_debug%': 0,
# Some STL containers (e.g. std::vector) do not preserve ABI compatibility
# between debug and non-debug mode.
'disable_glibcxx_debug': 1,

# Don't use ICU data file (icudtl.dat) from V8, we use our own.
'icu_use_data_file_flag%': 0,

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.3',
'v8_embedder_string': '-node.0',

##### V8 defaults for Node.js #####

# Old time default, now explicitly stated.
'v8_use_snapshot': 'true',

# Refs: https://github.com/nodejs/node/issues/23122
# Refs: https://github.com/nodejs/node/issues/23167
# Enable compiler warnings when using V8_DEPRECATED apis.
'v8_deprecation_warnings': 1,
# Enable compiler warnings when using V8_DEPRECATE_SOON apis.
'v8_imminent_deprecation_warnings': 1,

# Default to -O0 for debug builds.
'v8_optimized_debug': 0,

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,

# Don't bake anything extra into the snapshot.
'v8_use_external_startup_data%': 0,
'v8_use_external_startup_data': 0,

# https://github.com/nodejs/node/pull/22920/files#r222779926
'v8_enable_handle_zapping': 0,

# Disable V8 untrusted code mitigations.
# See https://github.com/v8/v8/wiki/Untrusted-code-mitigations
'v8_untrusted_code_mitigations': 'false',

# Some STL containers (e.g. std::vector) do not preserve ABI compatibility
# between debug and non-debug mode.
'disable_glibcxx_debug': 1,
# Still WIP in V8 7.1
'v8_enable_pointer_compression': 'false',

# Don't use ICU data file (icudtl.dat) from V8, we use our own.
'icu_use_data_file_flag%': 0,
# New in V8 7.1
'v8_enable_embedded_builtins': 'true',

# This is more of a V8 dev setting
# https://github.com/nodejs/node/pull/22920/files#r222779926
'v8_enable_fast_mksnapshot': 0,

##### end V8 defaults #####

'conditions': [
['GENERATOR=="ninja"', {
Expand Down
20 changes: 20 additions & 0 deletions deps/v8/.clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
---
Checks: '-*,
modernize-redundant-void-arg,
modernize-replace-random-shuffle,
modernize-shrink-to-fit,
modernize-use-auto,
modernize-use-bool-literals,
modernize-use-equals-default,
modernize-use-equals-delete,
modernize-use-nullptr,
modernize-use-override,
google-build-explicit-make-pair,
google-explicit-constructor,
google-readability-casting'
WarningsAsErrors: ''
HeaderFilterRegex: ''
AnalyzeTemporaryDtors: false
...

2 changes: 2 additions & 0 deletions deps/v8/.gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
# Do not modify line endings for binary files (which are sometimes auto
# detected as text files by git).
*.png binary
# Don't include minified JS in git grep/diff output
test/mjsunit/asm/sqlite3/*.js -diff
1 change: 0 additions & 1 deletion deps/v8/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
/tools/clang
/tools/gcmole/gcmole-tools
/tools/gcmole/gcmole-tools.tar.gz
/tools/gyp
/tools/jsfunfuzz/jsfunfuzz
/tools/jsfunfuzz/jsfunfuzz.tar.gz
/tools/luci-go
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ Peter Rybin <[email protected]>
Peter Varga <[email protected]>
Peter Wong <[email protected]>
Paul Lind <[email protected]>
PhistucK <[email protected]>
Qingyan Li <[email protected]>
Qiuyi Zhang <[email protected]>
Rafal Krypa <[email protected]>
Expand Down Expand Up @@ -162,6 +163,7 @@ Vladimir Krivosheev <[email protected]>
Vladimir Shutoff <[email protected]>
Wiktor Garbacz <[email protected]>
Xiaoyin Liu <[email protected]>
Yannic Bonenberger <[email protected]>
Yong Wang <[email protected]>
Yu Yin <[email protected]>
Zac Hansen <[email protected]>
Expand Down
Loading