Skip to content

deps: upgrade npm to 7.0.2#35667

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

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

Conversation

@MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented Oct 16, 2020 •
edited
Loading

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
Copy link
Collaborator

nodejs-github-bot commented Oct 16, 2020

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

@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
Copy link
Collaborator

nodejs-github-bot commented Oct 16, 2020

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

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Oct 16, 2020

/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
Copy link
Member

rvagg commented Oct 16, 2020

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
Copy link
Member

targos commented Oct 16, 2020

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
Copy link
Collaborator

nodejs-github-bot commented Oct 16, 2020

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

Trott
Trott approved these changes Oct 16, 2020
View reviewed changes
Copy link
Member

@Trott Trott left a comment

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
Copy link
Contributor Author

MylesBorins commented Oct 16, 2020

@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
Copy link
Contributor Author

MylesBorins commented Oct 16, 2020

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

@targos
Copy link
Member

targos commented Oct 16, 2020

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
Copy link
Contributor Author

MylesBorins commented Oct 16, 2020

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
Copy link
Collaborator

nodejs-github-bot commented Oct 16, 2020

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

rvagg
rvagg approved these changes Oct 17, 2020
View reviewed changes
Copy link
Member

@rvagg rvagg left a comment

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
Copy link
Contributor

github-actions bot commented Oct 18, 2020

Landed in fbe33aa...089d654

@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