This repository has been archived by the owner on Oct 15, 2020. It is now read-only.
Merge nodejs/master into xplat #211
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
This change removes a need for using deprecated debug context for breaking at the start of the main module. PR-URL: nodejs/node#12076 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
* we use u_setDataDirectory() in "unicode/putil.h" * at present, this header is indirectly included, but this will change in ICU 59 * no impact on past ICUs. * this is an exact analog to nodejs/node#11753 PR-URL: nodejs/node#12078 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Currently argv_[1] and argv_[2] are getting truncated by one character because of an incorrect addition of one to account for the null character. I only noticed this when working on #12087, but that fix will probably not get included in favor of a JavaScript test so I'm adding this separate commit for it. Refs: nodejs/node#12087 PR-URL: nodejs/node#12110 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
This is a fix for test-child-process-exec-kill-throws which is currently flaky on Windows. A bug in the test was causing the child process to fail for reasons other than those intended by the test. Instead of failing for exceeding the `maxBuffer` setting, the test was failing because it was trying to load `internal/child_process` without being passed the `expose-internals` flag. Move that module to where only the parent process (which gets the flag) loads it. Additionally, improve an assertion message to help debug problems like this. PR-URL: nodejs/node#12111 Fixes: nodejs/node#12053 Reviewed-By: Richard Lau <riclau@uk.ibm.com>
.eslintrc was renamed in #7699 to .eslintrc.yaml. PR-URL: nodejs/node#12116 Refs: nodejs/node#7699 Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The --use-bundled-ca and --use-openssl-ca command line arguments are mutually exclusive but can both be used on the same command line. This commit adds a check if both options are used. Fixes: nodejs/node#12083 PR-URL: nodejs/node#12087 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Missed while reviewing 1fde98b ("v8: expose new V8 serialization API.") PR-URL: nodejs/node#12118 Refs: nodejs/node#11048 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Make sure trailing garbage is not treated as a valid base64 character. Fixes: nodejs/node#11987 PR-URL: nodejs/node#11995 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Nothing but trouble can ever come from it. PR-URL: nodejs/node#12057 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, when in strict mode, function declarations are copied on the sandbox by CopyProperties(), which is not necessary and will break when CP is removed. This change maintains current behavior, letting GlobalPropertySetterCallback copy functions on the sandbox instead of using CP to do the task. PR-URL: nodejs/node#12051 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Define `V8_ENABLE_CHECKS` in `common.gypi` for the debug mode. Without this, these checks would only be present in the object files generated from the V8 build, and so for inline functions in v8.h multiple different definitions could be generated, where one definition includes the check and the other does not. Refs: nodejs/node#11975 (comment) PR-URL: nodejs/node#12029 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Adds support for the PSS padding scheme. Until now, the sign/verify functions used the old EVP_Sign*/EVP_Verify* OpenSSL API, making it impossible to change the padding scheme. Fixed by first computing the message digest and then signing/verifying with a custom EVP_PKEY_CTX, allowing us to specify options such as the padding scheme and the PSS salt length. Fixes: nodejs/node#1127 PR-URL: nodejs/node#11705 Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net>
* Rename listen to listenInCluster * Rename _listen2 to _setupListenHandle * Remove _listen since it's a one-liner only used in one place * Correct comments in server.listen PR-URL: nodejs/node#11796 Reviewed-By: James M Snell <jasnell@gmail.com>
Adds a centered logo to the README to make it a little more festive. As centering is not possible in pure Markdown, a bit of HTML is used. PR-URL: nodejs/node#12148 Ref: nodejs/node#6920 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Also support Uint8Array as a `dictionary` option. PR-URL: nodejs/node#12001 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Display `v8::External` values as `[External]` rather than `{}`
which makes them look like objects.
PR-URL: nodejs/node#12151
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
assert.deepEqual and assert.deepStrictEqual currently return true for any pair of Maps and Sets regardless of content. This patch adds support in deepEqual and deepStrictEqual to verify the contents of Maps and Sets. Deeo equivalence checking is currently an O(n^2) operation, and worse, it gets slower exponentially if maps and sets were nested. Note that this change breaks compatibility with previous versions of deepEqual and deepStrictEqual if consumers were depending on all maps and sets to be seen as equivalent. The old behaviour was never documented, but nevertheless there are certainly some tests out there which depend on it. Support has stalled because the assert API was frozen, but was recently unfrozen in CTC#63. --- Later squashed in: This change updates the checks for deep equality checking on Map and Set to check all set values / all map keys to see if any of them match the expected result. This change is much slower, but based on the conversation in the pull request its probably the right approach. Fixes: nodejs/node#2309 Refs: tape-testing/tape#342 Refs: nodejs/node#2315 Refs: nodejs/CTC#63 PR-URL: nodejs/node#12142 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Add support for abi stable module API (N-API) as "Experimental feature". The goal of this API is to provide a stable Node API for native module developers. N-API aims to provide ABI compatibility guarantees across different Node versions and also across different Node VMs - allowing N-API enabled native modules to just work across different versions and flavors of Node.js without recompilation. A more detailed introduction is provided in: https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf. The feature, during its experimental state, will be guarded by a runtime flag "--napi-modules". Only when this flag is added to the command line will N-API modules along with regular non N-API modules be supported. The API is defined by the methods in "src/node_api.h" and "src/node_api_types.h". This is the best starting point to review the API surface. More documentation will follow. In addition to the implementation of the API using V8, which is included in this PR, the API has also been validated against chakracore and that port is available in https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x. The current plan is to provide N-API support in versions 8.X and 6.X directly. For older versions, such as 4.X or pre N-API versions of 6.X, we plan to create an external npm module to provide a migration path that will allow modules targeting older Node.js versions to use the API, albeit without getting the advantage of not having to recompile. In addition, we also plan an external npm package with C++ sugar to simplify the use of the API. The sugar will be in-line only and will only use the exported N-API methods but is not part of the N-API itself. The current version is in: https://github.com/nodejs/node-api. This PR is a result of work in the abi-stable-node repo: https://github.com/nodejs/abi-stable-node/tree/doc, with this PR being the cumulative work on the api-prototype-8.x branch with the following contributors in alphabetical order: Author: Arunesh Chandra <arunesh.chandra@microsoft.com> Author: Gabriel Schulhof <gabriel.schulhof@intel.com> Author: Hitesh Kanwathirtha <hiteshk@microsoft.com> Author: Ian Halliday <ianhall@microsoft.com> Author: Jason Ginchereau <jasongin@microsoft.com> Author: Michael Dawson <michael_dawson@ca.ibm.com> Author: Sampson Gao <sampsong@ca.ibm.com> Author: Taylor Woll <taylor.woll@microsoft.com> PR-URL: nodejs/node#11975 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix the offset calculation for deserializing TypedArrays that are not aligned in their original buffer. Since `byteOffset` refers to the offset into the source `Buffer` instance, not its underlying `ArrayBuffer`, that is what should be passed to `buffer.copy`. PR-URL: nodejs/node#12143 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
This changes to the way path parsing for non-special URLs. It allows paths to be empty for non-special URLs and also takes that into account when serializing. Fixes: nodejs/node#11962 Refs: whatwg/url#213 PR-URL: nodejs/node#12058 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Refs: web-platform-tests/wpt#4586 Refs: nodejs/node#11887 PR-URL: nodejs/node#12058 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Updates: + Bring tests url-setter-tests from WPT, and put it as JavaScript + Comment out unpassed tests Refs: web-platform-tests/wpt#5112 Refs: nodejs/node#11887 PR-URL: nodejs/node#12058 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Ref: nodejs/node#12143 (comment) PR-URL: nodejs/node#12186 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`Error[CODE]` becomes `Error [CODE]` PR-URL: nodejs/node#12099 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Evan Lucas <evanlucas@me.com>
PR-URL: nodejs/node#12141 Ref: nodejs/CTC#89 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
makeCallback and makeStatsCallback are both tested intedependently. PR-URL: nodejs/node#12140 Fixes: nodejs/node#12136 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Previously, the --check CLI flag had no effect when run on code piped from stdin. This commit updates the bootstrap logic to handle the --check flag the same way regardless of whether the code is piped from stdin. PR-URL: nodejs/node#11689 Fixes: nodejs/node#11680 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
The -c flag ("check script syntax") and -e flag ("evaluate given code")
have contradictory meanings. Make them mutually exclusive by throwing
when both of them are provided.
Fixes: nodejs/node#11680
PR-URL: nodejs/node#11689
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
As it is, the `killSignal` is just retrieved from an object and used.
If the signal passed is actually one of the inherited properties of
that object, Node.js will die. For example,
➜ node -e "child_process.spawnSync('ls', {killSignal: 'toString'})"
Assertion failed: (0), function uv_close, file ....core.c, line 166.
[1] 58938 abort node -e "child_process.spawnSync(...)"
1. This patch makes sure that the signal is actually a own property of
the constants object.
2. Extends the killSignal validation to all the other functions.
PR-URL: nodejs/node#10423
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
1. necessarily reliably => necessarily reliable 2. projects root directory => project's root directory 3. remove `console` highlighting, as `test` alone is highlighted 4. fix broken link for Android NDK 5. highlight the directory location `/usr/local/ssl/fips-2.0` 6. update expected output to an example for `process.versions.openssl` as the version displayed is not mentioned in the document PR-URL: nodejs/node#11963 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: nodejs/node#12247 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Initialize status to napi_generic_failure and only check it after having made an actual N-API call. This fixes up 8fbace1. PR-URL: nodejs/node#12283 Ref: nodejs/node#12279 Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: nodejs/node#12254 Refs: nodejs/node#11875 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
Use no-restricted-syntax to implement the requirement that `Error` objects must be thrown with the `new` keyword. PR-URL: nodejs/node#12249 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: nodejs/node#12256 Fixes: nodejs/node#12161 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Refactor benchmark/_http-benchmarkers.js and add a test double HTTP benchmarker for testing. PR-URL: nodejs/node#12121 Refs: nodejs/node#12068 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12121 Refs: nodejs/node#12068 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12121 Refs: nodejs/node#12068 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
A std::string manages its own memory, so using one removes the implicit assumption that the argv vector passed to node will never be deallocated. Also, the enabled_categories are used to construct a std::stringstream, so its simpler to use the standard library consistently. PR-URL: nodejs/node#12242 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
The addons-napi testsuite is already included in $(CI_NATIVE_SUITES), so we don't need to manually specify it in the test-ci target as well. PR-URL: nodejs/node#12201 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* Since nodejs/node#6559 known_issues does run on CI. * Add some notes to explain the expectations around tests in known_issues. PR-URL: nodejs/node#12262 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
Kicked-off a CI run: |
Updated the following tests: test-cli-syntax - conditionalized error message check test-repl-sigint-nested-eval - disabled on ChakraCore test-querystring-escape - conditionalized error message check test-url-parse-invalid-input - conditionalized error message check test-util-inspect - updated engine specific string addons-napi/test_buffer/test - skip GC check test-debugger-repeat-last - disabled since inspector support is missing The first two issues are because of unimplemented features in ChakraShim. We can fix this after the merge
kfarnung
reviewed
Apr 17, 2017
test/parallel/test-cli-syntax.js
Outdated
| @@ -53,7 +53,12 @@ const syntaxArgs = [ | |||
| assert.strictEqual(c.stdout, '', 'stdout produced'); | |||
|
|
|||
| // stderr should include the filename | |||
| assert(c.stderr.startsWith(file), "stderr doesn't start with the filename"); | |||
| // (v8 prints the filename to the error stream- chakracore does not) | |||
| console.log(c.stderr); | |||
There was a problem hiding this comment.
Used for debugging? Should we remove it?
There was a problem hiding this comment.
gah- good catch- pushed a commit to remove it.
kfarnung
approved these changes
Apr 17, 2017
Remove leftover console.log from test-cli-syntax
|
@kfarnung thanks for the review- do you have a link to the CI run? |
|
@digitalinfinity CI link was called out above: https://ci.nodejs.org/job/chakracore-test-windows/58/ Looks like there's one failure for 2012r2 only, most likely a flaky test. Will kick off one last CI run to see if we can get a clean test pass. |
digitalinfinity
pushed a commit
that referenced
this pull request
Apr 18, 2017
PR-URL: #211 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
digitalinfinity
pushed a commit
that referenced
this pull request
Apr 18, 2017
Add stubs for v8 API which are newly required to build node-chakracore PR-URL: #211 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
digitalinfinity
added a commit
that referenced
this pull request
Apr 18, 2017
Updated the following tests: test-cli-syntax - conditionalized error message check test-repl-sigint-nested-eval - disabled on ChakraCore test-querystring-escape - conditionalized error message check test-url-parse-invalid-input - conditionalized error message check test-util-inspect - updated engine specific string addons-napi/test_buffer/test - skip GC check test-debugger-repeat-last - disabled since inspector support is missing The first two issues are because of unimplemented features in ChakraShim. We can fix this after the merge PR-URL: #211 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
2 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
chakrashim