deps: upgrade npm to 7.0.2 by MylesBorins · Pull Request #35667 · nodejs/node · GitHub
Skip to content

deps: upgrade npm to 7.0.2#35667

Closed
MylesBorins wants to merge 2 commits into
nodejs:masterfrom
MylesBorins:npm-7.0.1
Closed

deps: upgrade npm to 7.0.2#35667
MylesBorins wants to merge 2 commits into
nodejs:masterfrom
MylesBorins:npm-7.0.1

Conversation

@MylesBorins

@MylesBorins MylesBorins commented Oct 16, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

7.0.2 (2020-10-16)

DOCUMENTATION

BUG FIXES

DEPENDENCIES

@nodejs-github-bot nodejs-github-bot added the npm Issues and PRs related to the npm client dependency or the npm registry. label Oct 16, 2020
@MylesBorins MylesBorins added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2020
@nodejs-github-bot

nodejs-github-bot commented Oct 16, 2020

Copy link
Copy Markdown
Collaborator

@MylesBorins MylesBorins added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2020
@nodejs-github-bot

nodejs-github-bot commented Oct 16, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33656/

@MylesBorins

MylesBorins commented Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

/cc @nodejs/node-gyp the windows failures look gyp related.

https://ci.nodejs.org/job/node-test-binary-windows-native-suites/5549/nodes=win2012r2-vs2015-COMPILED_BY-vs2019-x86/console

@rvagg

rvagg commented Oct 16, 2020

Copy link
Copy Markdown
Member

It's pretty infuriating how little info we get from windows failures like this, it really needs local testing.

My guess is it's related to nodejs/gyp-next#71 but who really knows!

@targos @ryzokuken we might need your help on this.

@targos

targos commented Oct 16, 2020

Copy link
Copy Markdown
Member

The error is

error MSB4198: The expression "DirectoryName" cannot be evaluated on item "Release\\obj\\test_worker_terminate_finalization\\\\\\workspace\\node-test-binary-windows-native-suites\\node\\deps\\npm\\node_modules\\node-gyp\\src\\win_delay_load_hook.obj". The specified path, file name, or both are too long. The fully qualified file name must be less than 260 characters, and the directory name must be less than 248 characters.

The only idea I have is that we do avoid renaming absolute paths: nodejs/gyp-next#74

I applied it here to run CI

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Oct 16, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33669/

Trott
Trott approved these changes Oct 16, 2020
View reviewed changes

@Trott Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rubber-stamp LGTM if CI is green

@MylesBorins

MylesBorins commented Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

@targos thank you for finding the fix and floating the patch here. Do you have a timeline on a new release of node-gyp & gyp-next? We can float this patch on npm in the mean time and cut a 7.0.2 that includes some other fixes as well

@MylesBorins

MylesBorins commented Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

also, should we be floating this change on the internal version of gyp as well?

@targos

targos commented Oct 16, 2020

Copy link
Copy Markdown
Member

I'm waiting for a review on nodejs/gyp-next#75 to do the release.
The change is not necessary for the internal version.

MylesBorins and others added 2 commits October 16, 2020 17:55
@MylesBorins
deps: upgrade npm to 7.0.2
12929bf 
Refs: https://github.com/npm/cli/blob/latest/CHANGELOG.md#702-2020-10-16
@MylesBorins
test: fix addons/dlopen-ping-pong for npm 7.0.1
d803f4b 
This partially reverts c87641a as node-gyp no longer
puts shared objects in an inconsistent location.
@MylesBorins MylesBorins changed the title deps: upgrade npm to 7.0.1 deps: upgrade npm to 7.0.2 Oct 16, 2020
@MylesBorins

MylesBorins commented Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

updated to 7.0.2 which floats the gyp patch

@MylesBorins MylesBorins added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2020
@nodejs-github-bot

nodejs-github-bot commented Oct 16, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33680/

rvagg
rvagg approved these changes Oct 17, 2020
View reviewed changes

@rvagg rvagg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

confirmed same node-gyp code as is about to go out in a new release

@rvagg rvagg mentioned this pull request Oct 17, 2020
Merged
@MylesBorins MylesBorins added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 18, 2020
@github-actions github-actions Bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 18, 2020
@github-actions

github-actions Bot commented Oct 18, 2020

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot closed this Oct 18, 2020
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2020
@MylesBorins @nodejs-github-bot
deps: upgrade npm to 7.0.2
5b9593f 
Refs: https://github.com/npm/cli/blob/latest/CHANGELOG.md#702-2020-10-16

PR-URL: #35667
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Oct 18, 2020
@MylesBorins @nodejs-github-bot
test: fix addons/dlopen-ping-pong for npm 7.0.1
089d654 
This partially reverts c87641a as node-gyp no longer
puts shared objects in an inconsistent location.

PR-URL: #35667
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes
@watilde watilde watilde approved these changes
+1 more reviewer
@rvagg rvagg rvagg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

npm Issues and PRs related to the npm client dependency or the npm registry.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@MylesBorins @nodejs-github-bot @rvagg @targos @Trott @watilde