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.

Sign up for GitHub

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

Jump to bottom

deps: upgrade npm to 7.0.2 #35667

Closed
wants to merge 2 commits into from
Closed

deps: upgrade npm to 7.0.2 #35667

wants to merge 2 commits into from

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

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

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

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

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

@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

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 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

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

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

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 <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
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 <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvagg rvagg rvagg approved these changes

@Trott Trott Trott approved these changes

@watilde watilde watilde approved these changes

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