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 npm to 2.7.4 #1285

Closed
wants to merge 4 commits into from
Closed

deps: upgrade npm to 2.7.4 #1285

wants to merge 4 commits into from

Conversation

othiym23
Copy link
Contributor

This release is mostly bug fixes, including:

I've applied @cjihrig's node-gyp floating patch, as well as reapplying @piscisaureus's two patches for delayed loading of compiled modules. Are both of those commits necessary? Can they be combined into a single commit? I'm in the process of setting up a quilt to handle these floating patches (which I am still hopeful will only be needed for a little while), but the commit logs are going to start getting noisy if the list of commits that come along for the ride with each new version of npm continues to grow.

R: @piscisaureus
R: @rvagg
R: @Fishrock123

othiym23 and others added 4 commits March 27, 2015 03:57
Every npm version bump requires a few patches to be floated on
node-gyp for io.js compatibility. These patches are found in
03d1992,
5de334c, and
da730c7. This commit squashes
them into a single commit.

PR-URL: nodejs#990
Reviewed-By: Ben Noordhuis <[email protected]>
On Windows, when node or io.js attempts to dynamically load a compiled
addon, the compiled addon tries to load node.exe or iojs.exe again -
depending on which import library the module used when it was linked.
This makes it impossible to rename node.exe or iojs.exe, because when
that happens the module can't find its dependencies.

With this patch, a delay-load hook is added to all modules that are
compiled with node-gyp. The delay-load hook ensures that whenever a
module tries to load imports from node.exe/iojs.exe, it'll just refer
back to the process image, thus making it possible to rename the
iojs/node binary.

Bug: nodejs#751
Bug: nodejs#965
Upstream PR: nodejs/node-gyp#599

PR-URL: nodejs#1251
Reviewed-By: Rod Vagg <[email protected]>
The delay-load hook that was landed in 3d46fef to make compiled addons
work on Windows regardless of the iojs.exe/node.exe filename causes
issues with a small amount of compiled addons.

Therefore this patch makes it an opt-in feature. An addon may set the
'win_delay_load_hook' option to 'true' in its binding.gyp to enable this
feature.

Example:

```
{
  'targets': [
    {
      'target_name': 'ernie',
      'win_delay_load_hook': 'true',
      ...
```

Refs: nodejs#1251
PR-URL: nodejs#1266
Reviewed-By: Ben Noordhuis <[email protected]>
@@ -128,7 +128,7 @@ <h2 id="author">AUTHOR</h2>
<p><a href="http://blog.izs.me/">Isaac Z. Schlueter</a> ::
<a href="https://github.com/isaacs/">isaacs</a> ::
<a href="http://twitter.com/izs">@izs</a> ::
<a href="&#x6d;&#97;&#x69;&#x6c;&#116;&#x6f;&#x3a;&#x69;&#x40;&#105;&#122;&#x73;&#x2e;&#x6d;&#x65;">&#x69;&#x40;&#105;&#122;&#x73;&#x2e;&#x6d;&#x65;</a></p>
<a href="&#x6d;&#x61;&#105;&#108;&#x74;&#111;&#x3a;&#x69;&#x40;&#105;&#122;&#115;&#x2e;&#109;&#101;">&#x69;&#x40;&#105;&#122;&#115;&#x2e;&#109;&#101;</a></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why are the email links encoded like this, and why does it change each update? o.0

@YurySolovyov
Copy link

I always wonder why do you keep hardcoded version in all these .html files.
It looks like you have scripts on these pages, so maybe you can move version in some sort of config and then make a script that will insert version in a proper element?

@Fishrock123
Copy link
Contributor

Looked over the code. Can't wait for npm@3 to reduce some of those glob deps. :P

Also, yay no whitespace errors this time. :)

I'm pretty sure @piscisaureus's commits can be rolled into one second floating patch.

test-npm is not happy, but I'm pretty sure it's just github failing on clone. https://gist.github.com/Fishrock123/1877f47730d895ba5f1c

LGTM otherwise.

@othiym23
Copy link
Contributor Author

@YuriSolovyov the HTML documentation is primarily meant to be used by Windows users who don't have access to man. As such, it needs to be pretty much completely static.

@othiym23
Copy link
Contributor Author

@Fishrock123 The email links are randomly encoded to mitigate spam, and date from a quaint time when people thought that was possible.

@Fishrock123
Copy link
Contributor

Ah. Running the tests again now, hopefully github survives.

@Fishrock123
Copy link
Contributor

Tests seem fine, LGTM pending a potential squash on @piscisaureus's commits, but that isn't of too much concern.

@othiym23
Copy link
Contributor Author

I'd still like a thumbs up from @piscisaureus before merging. I'll probably combine his two commits, if he says that's OK.

@YurySolovyov
Copy link

@othiym23 still, that would make diffs a lot less noisy and short

@piscisaureus
Copy link
Contributor

LGTM. 42b8907 and cc4711a could be squashed, but I don't care a whole lot.

@piscisaureus
Copy link
Contributor

42b8907 and cc4711a could be squashed, but I don't care a whole lot.

I'll do that actually.

@piscisaureus
Copy link
Contributor

Landed in iojs:73de135...iojs:ba93c58

@Fishrock123
Copy link
Contributor

:shipit:

@othiym23
Copy link
Contributor Author

@piscisaureus f166cde is missing its commit message and associated metadata. @chrisdickinson made the call that it wasn't worth changing because there are commits after it, but we should probably avoid that sort of thing in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants