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

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
Member

@MylesBorins MylesBorins commented Oct 16, 2020 •
edited

7.0.2 (2020-10-16)

DOCUMENTATION

BUG FIXES

DEPENDENCIES

@nodejs-github-bot nodejs-github-bot added the npm label Oct 16, 2020
@MylesBorins MylesBorins added the request-ci label Oct 16, 2020
@github-actions github-actions bot removed the request-ci label Oct 16, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Oct 16, 2020

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

@MylesBorins MylesBorins added the request-ci label Oct 16, 2020
@github-actions github-actions bot removed the request-ci label Oct 16, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Oct 16, 2020

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

@MylesBorins
Copy link
Member Author

@MylesBorins 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 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 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 hidden.

Sign in to view
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot 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 changes
Copy link
Member

@Trott Trott left a comment

Rubber-stamp LGTM if CI is green

@MylesBorins
Copy link
Member Author

@MylesBorins 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
Member Author

@MylesBorins 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 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 Oct 16, 2020
@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
Member Author

@MylesBorins MylesBorins commented Oct 16, 2020

updated to 7.0.2 which floats the gyp patch

@MylesBorins MylesBorins added the request-ci label Oct 16, 2020
@github-actions github-actions bot removed the request-ci label Oct 16, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot 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 changes
Copy link
Member

@rvagg rvagg left a comment

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 label Oct 18, 2020
@github-actions github-actions bot removed the commit-queue label Oct 18, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Oct 18, 2020

Landed in fbe33aa...089d654

@github-actions github-actions bot closed this Oct 18, 2020
nodejs-github-bot added a commit that referenced this issue Oct 18, 2020
@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 added a commit that referenced this issue 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]>
@MylesBorins MylesBorins added dont-land-on-v10.x dont-land-on-v12.x dont-land-on-v14.x labels Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvagg rvagg

@Trott Trott

@watilde watilde

Assignees
No one assigned
Labels
dont-land-on-v12.x dont-land-on-v14.x npm
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
6 participants
@MylesBorins @nodejs-github-bot @rvagg @targos @Trott @watilde