[v9.x] build: make gyp user defined variables lowercase #18899
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR-URL: nodejs#18270 Fixes: nodejs#8307 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
This helps to show the cause of errors in the CI. PR-URL: nodejs#18507 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
The method to implement is `_write` not `_transform`. PR-URL: nodejs#18604 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
The current output uses JSON.stringify to escape the config values. This switches to util.inspect to have a better readable output. PR-URL: nodejs#18597 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Andreas Madsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Unused since 34b535f. PR-URL: nodejs#18568 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Due to extensive reliance on timings and the fs module, this test is currently inherently flaky. Refactor it to simply use setImmediate and only one busy loop. PR-URL: nodejs#18567 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
For tests that use anonymous namespaces, some tagged the close of the namespace with 'namespace' while others used 'anonymous namespace'. It was suggested I should use 'anonymous namespace' in a recent PR review so make all of the tests consistent with this. PR-URL: nodejs#18583 Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#18651 Refs: nodejs#12223 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
PR-URL: nodejs#18651 Refs: nodejs#8281 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
PR-URL: nodejs#18670 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#18679 Fixes: nodejs#18544 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]>
Add optional Http2ServerRequest and Http2ServerResponse options to createServer and createSecureServer. Allows custom req & res classes that extend the default ones to be used without overriding the prototype. PR-URL: nodejs#15560 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs#18683 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
In ICU 61.x, icu4c will no longer put its declarations in the global namespace. Everything will be in the "icu::" namespace (or icu_60:: in the linker). Prepare for this. https://ssl.icu-project.org/trac/ticket/13460
Adds the remaining options from tls.createSecureContext() to the string generated by Agent#getName(). This allows https.request() to accept the options and generate unique sockets appropriately. PR-URL: nodejs#16402 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Replace var for let or const. PR-URL: nodejs#18649 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: James M Snell <[email protected]>
Previously, fs.readdirSync calls the function returned by env->push_values_to_array_function() in batch and check the returned Maybe right away in C++, which can lead to assertions if the call stack already reaches the maximum size. This patch fixes that by returning early the call fails so the stack overflow error will be properly thrown into JS land. PR-URL: nodejs#18647 Fixes: nodejs#18645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#18685 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Missing the length argument in napi_create_function. PR-URL: nodejs#18661 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#18665 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]>
Ref: nodejs#18643 PR-URL: nodejs#18677 Refs: nodejs#18643 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Previously, the err passed to the callback of fs.open() was not checked. PR-URL: nodejs#18681 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Don't disconnect the child until all exceptions are thrown. Fixes: nodejs#18659 PR-URL: nodejs#18692 Fixes: nodejs#18659 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#18697 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#18700 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Rename the `parentId` property on the PromiseWrap object to a `isChainedPromise` property. The former wasn't quite useful as it was always defined to be the same value as the trigger id available in the init hook. Instead rename the property to be closer to the information it communicates: whether the promise is a chained promise or not. PR-URL: nodejs#18633 Fixes: nodejs#18470 Reviewed-By: Andreas Madsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Currently the call can lead to a TypeError with the message: `Cannot read property 'value' of undefined`. This fixes it by first checking that the first argument is truthy. PR-URL: nodejs#18729 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#18609 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: nodejs#18591 Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Add useful info about process.domain to error meesages in the uncaughtException event listener and the beforeExit event listener. Refactor code such as using template literals, and also make sure uncaughtException listner is detached after firing once to avoid endless loop in case of exception throw in the beforeExit event listner. PR-URL: nodejs#18541 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Adds note that accessing the fd of the IPC channel in any other way than process.send, or using the IPC channel with child processes that is not Node.js is not supported. PR-URL: nodejs#17545 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Like man(7), mdoc(7) is a macro package for marking up computer manuals. The main difference is that mdoc is semantic rather than presentational, providing authors with a full DSL to abstract page markup from low-level Roff commands. By contrast, `man` is minimalist and leaves formatting to the author, who is expected to have a working amount of Roff knowledge. Therefore the use of `mdoc` for marking up Node's manpage is a decidedly better choice than bare `man`. Less room is left for error and mandoc(1) offers very robust error-checking and linting. PR-URL: nodejs#18559 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Use arrow functions and common.mustCall() and add a description. PR-URL: nodejs#18714 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>
This brings the behaviour in line with the documentation. PR-URL: nodejs#16944 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
1. Fixer for crypto-check.js 2. Extends tests PR-URL: nodejs#16647 Refs: nodejs#16636 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1. Adds fixer method 2. Extends test PR-URL: nodejs#16646 Refs: nodejs#16636 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#18739 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
Currenlty when configuring --debug-http2 /test/parallel/test-http2-getpackedsettings.js will segment fault: $ out/Debug/node test/parallel/test-http2-getpackedsettings.js Segmentation fault: 11 This is happening because the settings is created with the Environment in PackSettings: Http2Session::Http2Settings settings(env); This will cause the session to be set to nullptr. When the init function is later called the expanded DEBUG_HTTP2SESSION macro will cause the segment fault when the session is dereferenced. PR-URL: nodejs#18815 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
When using a debug build (on Windows specifically) the error case for tls_wrap causes an assert to fire because the index being passed is outside the bounds of the vector. The fix is to switch to iterators. PR-URL: nodejs#18830 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#18872 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#18873 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Refs: nodejs#11135 PR-URL: nodejs#18843 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
PR-URL: nodejs#18845 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#18849 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#18850 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins
force-pushed
the
v9.x-staging
branch
from
0e105ae
to
bc156da
Compare
This fixes a bug in the CPU profiler where some ticks were attributed to the wrong file. Original commit message: [cpu-profiler] Fix script name when recording inlining info Use the script name from the shared function info to create an inline entry. Otherwise functions are attributed to the wrong file in the CpuProfileNode. See googleapis/cloud-profiler-nodejs#89 Bug: v8:7203, v8:7241 Change-Id: I8ea31943741770e6611275a9c93375922b934547 Reviewed-on: https://chromium-review.googlesource.com/848093 Reviewed-by: Jaroslav Sevcik <[email protected]> Commit-Queue: Franziska Hinkelmann <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#50339} Refs: v8/v8@76c3ac5 PR-URL: nodejs#18298 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Note that the CI run for the exercise can be minimal. PR-URL: nodejs#18846 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#18847 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
* Sync format schemes with the current doc state. * Lowercase primitive types. * Fix typos and unify the style. * Remove tautological info. PR-URL: nodejs#18874 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
I mistakenly introduced user defined variables using uppercase characters, reading the gyp documentation they state: "Predefined variables. By convention, these are named with CAPITAL_LETTERS. Predefined variables are set automatically by GYP" and also "By convention, user-defined variables are named with lowercase_letters." This commit renames the user defined variables to lowercase to follow the above mentioned convention. PR-URL: nodejs#16238 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
addaleax
force-pushed
the
backport-16238
branch
from
f150b0d
to
6ece376
Compare
|
Could I get a quick LGTM on this? The only conflicts were neighboring lines in |
bnoordhuis
approved these changes
Feb 21, 2018
There was a problem hiding this comment.
Quick LGTM but note #16238 (review) - it's probably harmless to back-port this but a citgm run wouldn't hurt.
|
Thanks for the note on that! CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1299/ |
MylesBorins
force-pushed
the
v9.x-staging
branch
from
b9b5810
to
6edf952
Compare
|
Landed on v9.x-staging |
addaleax
pushed a commit
that referenced
this pull request
Feb 26, 2018
I mistakenly introduced user defined variables using uppercase characters, reading the gyp documentation they state: "Predefined variables. By convention, these are named with CAPITAL_LETTERS. Predefined variables are set automatically by GYP" and also "By convention, user-defined variables are named with lowercase_letters." This commit renames the user defined variables to lowercase to follow the above mentioned convention. Backport-PR-URL: #18899 PR-URL: #16238 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
addaleax
pushed a commit
that referenced
this pull request
Feb 26, 2018
I mistakenly introduced user defined variables using uppercase characters, reading the gyp documentation they state: "Predefined variables. By convention, these are named with CAPITAL_LETTERS. Predefined variables are set automatically by GYP" and also "By convention, user-defined variables are named with lowercase_letters." This commit renames the user defined variables to lowercase to follow the above mentioned convention. Backport-PR-URL: #18899 PR-URL: #16238 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #16238, with only trivial conflicts.