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.6 #1390

Merged
merged 3 commits into from
Apr 10, 2015
Merged

deps: upgrade npm to 2.7.6 #1390

merged 3 commits into from
Apr 10, 2015

Conversation

othiym23
Copy link
Contributor

There are three fixes in npm@2.7.6 that are especially pertinent to the io.js massive:

  • b747593
    #7630 Don't automatically log all git failures as errors. maybeGithub needs to be able to fail without logging to support its fallback logic. (@othiym23)
  • 78005eb
    #7743 Always quote arguments passed
    to npm run-script. This allows build systems and the like to safely escape
    glob patterns passed as arguments to run-scripts with `npm run-script <script> -- `. This is a tricky change to test, and may be reverted or moved to `npm@3` if it turns out it breaks things for users. ([@mantoni](https://github.com/mantoni))
  • da015ee
    #7074 read-package-json@1.3.3:
    read-package-json no longer caches package.json files, which trades a
    very small performance loss for the elimination of a large class of really
    annoying race conditions. See #7074
    for the grisly details. (@othiym23)

There are the usual other array of smaller tweaks and fixes, which can be read about in the release notes.

I've applied the two roll-up patches for improved node-gyp support already. All I need are some reviewers!

r: @Fishrock123
r: @piscisaureus

@othiym23 othiym23 added the npm Issues and PRs related to the npm client dependency or the npm registry. label Apr 10, 2015
@Fishrock123
Copy link
Contributor

Holy moley you're up early :)

  • 78005eb
    #7743 Always quote arguments passed
    to npm run-script. This allows build systems and the like to safely escape
    glob patterns passed as arguments to run-scripts with `npm run-script <script> -- `. This is a tricky change to test, and may be reverted or moved to `npm@3` if it turns out it breaks things for users. ([@mantoni](https://github.com/mantoni))

Hmmm, I'm not sure how I feel like putting that into a release.

deps/npm/node_modules/semver/semver.min.js.gz

Again, not so fond of checking in more gzipped stuff since it doesn't compress in git history, but, I am assuming there is no way to make semver only do this for browsers? Neither of the min files matter here. (I understand it's small and pretty trivial, mostly curious.)

Code has had eyes. Running tests.

@Fishrock123
Copy link
Contributor

Tests are happier than last time, no failures.

@Fishrock123
Copy link
Contributor

Only other thing about this is, I don't really understand how the float.patch in core-util-is and readable-stream works.

Edit: Answered in #npm, probably just lint from testing.

@Fishrock123
Copy link
Contributor

LGTM other than that one change mentioned in the commit log. Edit: Discussed on #npm, seems fine.

@othiym23
Copy link
Contributor Author

Holy moley you're up early :)

I was up late. 🌅 The newest round of git changes ended up requiring a lot of testing, because I would like to stop working on the git code for a while. ;)

deps/npm/node_modules/semver/semver.min.js.gz

The thing about using bundled dependencies in npm (which is necessary because otherwise there's a bootstrapping problem) is that you kind of need to take them as-is and assume that what you install is what the publisher signed off on. It does use a tiny bit of disk space to leave all the packaged files in node_modules, but it saves the project's maintainers a huge amount of time to not have to do the additional QA necessary to confirm that removing files that look unnecessary are in fact unnecessary. A huge chunk of request is probably superfluous for npm's purposes, but that's not really how dependencies actually work. Files under node_modules should only be remarkable during review if they're egregious in some way – infeasibly large or look like malware, or something of similar urgency.

This is a tricky change to test, and may be reverted or moved to npm@3 if it turns out it breaks things for users. (@mantoni)

To capture our conversation from #npm here, I include these kinds of messages because the release notes are written for when the release is new and marked as npm@next, so that I can help people who help us test npm figure out what they should be looking for. Also, experience has taught me that sometimes it's tough to catch breaking changes. It's good to warn people when there are potential things that could trigger a semver-major change. I would love it if people would contribute more and better tests for potentially troublesome features like this, so we can prove one way or the other whether they're actually risky.

Only other thing about this is, I don't really understand how the float.patch in core-util-is and readable-stream works.

Publishing fewmets left behind by @isaacs's git workflow, I imagine. They could probably be stripped out, but see above for why I don't. The only files I habitually remove before putting together the PRs are *~ backup files.

othiym23 and others added 3 commits April 10, 2015 14:22
PR-URL: nodejs#1390
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
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 <info@bnoordhuis.nl>
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 causes many compiled addons to break when node.exe or iojs.exe are
renamed, because when the binary has been renamed the addon DLL can't
find the (right) .exe file to load its imports from.

This patch gives compiled addon developers an option to overcome this
restriction by compiling a delay-load hook into their binary. The
delay-load hook ensures that whenever a module tries to load imports
from node.exe/iojs.exe, it'll just look at the process image, thereby
making the addon work regardless of what name the node/iojs binary has.

To enable this feature, the addon developer must set the
'win_delay_load_hook' option to 'true' in their binding.gyp file, like
this:

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

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

PR-URL: nodejs#1251
Reviewed-By: Rod Vagg <rod@vagg.org>

PR-URL: nodejs#1266
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@othiym23 othiym23 merged commit 907aaf3 into nodejs:v1.x Apr 10, 2015
@othiym23 othiym23 deleted the npm-2.7.6 branch April 10, 2015 21:26
@rvagg rvagg mentioned this pull request Apr 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants