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

deps: upgrade to libuv 1.28.0 #27241

Merged
merged 3 commits into from Apr 22, 2019

Conversation

Projects
None yet
8 participants
@cjihrig
Copy link
Contributor

commented Apr 15, 2019

Notable changes:

  • uv_gettimeofday() has been added.
  • Streaming readdir() via the uv_fs_{open,read,close}dir() methods.
  • A macOS copyfile() permissions bug has been fixed.
  • A bug in uv_interface_addresses() on machines with multiple interfaces has been fixed.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
@nodejs-github-bot

This comment has been minimized.

@BridgeAR
Copy link
Member

left a comment

RSLGTM

@Trott

Trott approved these changes Apr 15, 2019

Copy link
Member

left a comment

Rubber-stamp LGTM

@Trott

This comment has been minimized.

Copy link
Member

commented Apr 15, 2019

The two commits will be squashed into one when landing? Otherwise, the first commit will have a failing test, possibly messing up a git bisect. Maybe that doesn't matter too much as you don't usually run make test with git bisect but instead something more specific....

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@richardlau

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

test-fs-copyfile-respect-permissions is failing on Windows:
e.g. https://ci.nodejs.org/job/node-test-binary-windows-2/190/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=0/testReport/junit/(root)/test/parallel_test_fs_copyfile_respect_permissions/

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'EPERM'
- 'EACCES'
    at Object.check (c:\workspace\node-test-binary-windows-2\test\parallel\test-fs-copyfile-respect-permissions.js:26:12)
    at expectedException (assert.js:586:19)
    at expectsError (assert.js:686:17)
    at Function.throws (assert.js:717:3)
    at Object.<anonymous> (c:\workspace\node-test-binary-windows-2\test\parallel\test-fs-copyfile-respect-permissions.js:37:10)
    at Module._compile (internal/modules/cjs/loader.js:766:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:777:10)
    at Module.load (internal/modules/cjs/loader.js:635:32)
    at Function.Module._load (internal/modules/cjs/loader.js:562:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:833:10)
@richardlau

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

@cjihrig the following should fix the test:

diff --git a/test/parallel/test-fs-copyfile-respect-permissions.js b/test/parallel/test-fs-
index 34697ee..3f8d025 100644
--- a/test/parallel/test-fs-copyfile-respect-permissions.js
+++ b/test/parallel/test-fs-copyfile-respect-permissions.js
@@ -23,7 +23,9 @@ function beforeEach() {
   fs.chmodSync(dest, '444');

   const check = (err) => {
-    assert.strictEqual(err.code, 'EACCES');
+    const expected = ['EACCES', 'EPERM'];
+    assert(expected.includes(err.code),
+           `err.code '${err.code}' is neither '${expected.join('\' nor \'')}'`);
     assert.strictEqual(fs.readFileSync(dest, 'utf8'), 'dest');
     return true;
   };

Also please remove the entries in test/known_issues/known_issues.status:

diff --git a/test/known_issues/known_issues.status b/test/known_issues/known_issues.status
index d7e0b54..3463f0a 100644
--- a/test/known_issues/known_issues.status
+++ b/test/known_issues/known_issues.status
@@ -7,24 +7,18 @@ prefix known_issues
 [true] # This section applies to all platforms

 [$system==win32]
-test-fs-copyfile-respect-permissions: SKIP

 [$system==linux]
 test-vm-timeout-escape-promise: PASS,FLAKY
-test-fs-copyfile-respect-permissions: SKIP

 [$system==macos]

 [$system==solaris]
-test-fs-copyfile-respect-permissions: SKIP

 [$system==freebsd]
-test-fs-copyfile-respect-permissions: SKIP

 [$system==aix]
-test-fs-copyfile-respect-permissions: SKIP

 [$arch==arm]
 # https://github.com/nodejs/node/issues/24120
 test-vm-timeout-escape-nexttick: PASS,FLAKY
-test-fs-copyfile-respect-permissions: SKIP
@richardlau

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Fixes: #27273

@cjihrig cjihrig force-pushed the cjihrig:libuv branch from 5a9eead to afe3bb7 Apr 22, 2019

@nodejs-github-bot

This comment has been minimized.

Copy link

commented Apr 22, 2019

@cjihrig cjihrig force-pushed the cjihrig:libuv branch from afe3bb7 to bd1a675 Apr 22, 2019

cjihrig added a commit to cjihrig/node-1 that referenced this pull request Apr 22, 2019

deps: upgrade to libuv 1.28.0
Notable changes:

- uv_gettimeofday() has been added.
- Streaming readdir() via the uv_fs_{open,read,close}dir() methods.
- A macOS copyfile() permissions bug has been fixed.
- A bug in uv_interface_addresses() on machines with multiple
  interfaces has been fixed.

Fixes: nodejs#27273
PR-URL: nodejs#27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cjihrig added a commit to cjihrig/node-1 that referenced this pull request Apr 22, 2019

test: move known issue test to parallel
As of libuv 1.28.0, this bug is fixed, and the test can be
moved to parallel. This commit also updates an error code
check to work on Windows.

PR-URL: nodejs#27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cjihrig added a commit to cjihrig/node-1 that referenced this pull request Apr 22, 2019

test: unskip copyfile permission test
This is no longer a flakey test, and should run everywhere.

PR-URL: nodejs#27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cjihrig added some commits Apr 15, 2019

deps: upgrade to libuv 1.28.0
Notable changes:

- uv_gettimeofday() has been added.
- Streaming readdir() via the uv_fs_{open,read,close}dir() methods.
- A macOS copyfile() permissions bug has been fixed.
- A bug in uv_interface_addresses() on machines with multiple
  interfaces has been fixed.

Fixes: #27273
PR-URL: #27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
test: move known issue test to parallel
As of libuv 1.28.0, this bug is fixed, and the test can be
moved to parallel. This commit also updates an error code
check to work on Windows.

PR-URL: #27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
test: unskip copyfile permission test
This is no longer a flakey test, and should run everywhere.

PR-URL: #27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@cjihrig cjihrig force-pushed the cjihrig:libuv branch from bd1a675 to a3d1922 Apr 22, 2019

@cjihrig

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Landed in 2161690...a3d1922. Thanks for the reviews, especially @richardlau.

@cjihrig cjihrig closed this Apr 22, 2019

@cjihrig cjihrig deleted the cjihrig:libuv branch Apr 22, 2019

@cjihrig cjihrig merged commit a3d1922 into nodejs:master Apr 22, 2019

1 of 2 checks passed

Travis CI - Pull Request Build Errored
Details
Travis CI - Branch Build Passed
Details

MylesBorins added a commit to MylesBorins/node that referenced this pull request May 20, 2019

deps: upgrade to libuv 1.28.0
Notable changes:

- uv_gettimeofday() has been added.
- Streaming readdir() via the uv_fs_{open,read,close}dir() methods.
- A macOS copyfile() permissions bug has been fixed.
- A bug in uv_interface_addresses() on machines with multiple
  interfaces has been fixed.

Fixes: nodejs#27273
PR-URL: nodejs#27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit to MylesBorins/node that referenced this pull request May 20, 2019

test: move known issue test to parallel
As of libuv 1.28.0, this bug is fixed, and the test can be
moved to parallel. This commit also updates an error code
check to work on Windows.

PR-URL: nodejs#27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit to MylesBorins/node that referenced this pull request May 20, 2019

test: unskip copyfile permission test
This is no longer a flakey test, and should run everywhere.

PR-URL: nodejs#27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request May 21, 2019

deps: upgrade to libuv 1.28.0
Notable changes:

- uv_gettimeofday() has been added.
- Streaming readdir() via the uv_fs_{open,read,close}dir() methods.
- A macOS copyfile() permissions bug has been fixed.
- A bug in uv_interface_addresses() on machines with multiple
  interfaces has been fixed.

Fixes: #27273
Backport-PR-URL: #27776
PR-URL: #27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request May 21, 2019

test: move known issue test to parallel
As of libuv 1.28.0, this bug is fixed, and the test can be
moved to parallel. This commit also updates an error code
check to work on Windows.

Backport-PR-URL: #27776
PR-URL: #27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request May 21, 2019

test: unskip copyfile permission test
This is no longer a flakey test, and should run everywhere.

Backport-PR-URL: #27776
PR-URL: #27241
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@BethGriggs BethGriggs referenced this pull request May 22, 2019

Merged

v10.16.0 proposal #27514

BethGriggs added a commit that referenced this pull request May 28, 2019

2019-05-28, Version 10.16.0 'Dubnium' (LTS)
Notable changes:

- **deps**:
  - update ICU to 64.2 (Ujjwal Sharma)
    [#27361](#27361)
  - upgrade npm to 6.9.0 (Kat Marchán)
    [#26244](#26244)
  - upgrade openssl sources to 1.1.1b (Sam Roberts)
    [#26327](#26327)
  - upgrade to libuv 1.28.0 (cjihrig)
    [#27241](#27241)
- **events**:
  - add once method to use promises with EventEmitter (Matteo Collina)
   [#26078](#26078)
- **n-api**:
  - mark thread-safe function as stable (Gabriel Schulhof)
    [#25556](#25556)
- **repl**:
  - support top-level for-await-of (Shelley Vohr)
    [#23841](#23841)
- **zlib**:
  - add brotli support (Anna Henningsen)
    [#24938](#24938)

PR-URL: #27514

BethGriggs added a commit that referenced this pull request May 28, 2019

2019-05-28, Version 10.16.0 'Dubnium' (LTS)
Notable changes:

- **deps**:
  - update ICU to 64.2 (Ujjwal Sharma)
    [#27361](#27361)
  - upgrade npm to 6.9.0 (Kat Marchán)
    [#26244](#26244)
  - upgrade openssl sources to 1.1.1b (Sam Roberts)
    [#26327](#26327)
  - upgrade to libuv 1.28.0 (cjihrig)
    [#27241](#27241)
- **events**:
  - add once method to use promises with EventEmitter (Matteo Collina)
   [#26078](#26078)
- **n-api**:
  - mark thread-safe function as stable (Gabriel Schulhof)
    [#25556](#25556)
- **repl**:
  - support top-level for-await-of (Shelley Vohr)
    [#23841](#23841)
- **zlib**:
  - add brotli support (Anna Henningsen)
    [#24938](#24938)

PR-URL: #27514
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.