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: update npm to 2.11.1 #1899

Closed
wants to merge 3 commits into from
Closed

deps: update npm to 2.11.1 #1899

wants to merge 3 commits into from

Conversation

zkat
Copy link
Contributor

@zkat zkat commented Jun 5, 2015

https://github.com/npm/npm/releases/tag/v2.11.1

This includes a tweak to use GIT_SSH_COMMAND (available as of Git 2.3). See the changelog linked above for deets.

Also hi! :)

r: @Fishrock123
r: @chrisdickinson

@brendanashworth brendanashworth added the npm Issues and PRs related to the npm client dependency or the npm registry. label Jun 5, 2015
@Fishrock123
Copy link
Contributor

Hey there! I'll test/land this mostly tomorrow. :)

@piscisaureus ICYMI the delay-load hook additions have made it into node-gyp as for this version, other than enabled by default.

(Nice to see that ya'll are able to start taking a bit off of Forrest's plate. :P)

@Fishrock123 Fishrock123 self-assigned this Jun 5, 2015
@Fishrock123
Copy link
Contributor

Hmmm, getting this from make test-npm (which runs tools/test-npm.sh) and thus (in an npm directory):

../$NODE_EXE cli.js install --ignore-scripts
../$NODE_EXE cli.js run-script test-all

Error log.. looks like debug@* didn't install?: cc @othiym23

test/tap/00-verify-ls-ok.js .npm ERR! missing: debug@*, required by array-index@0.1.1
test/tap/00-verify-ls-ok.js ........................... 3/4 1s
  npm ls in npm
  not ok npm ls exited with code
    +++ found                                                          
    --- wanted                                                         
    -0                                                                 
    +1                                                                 
    compare: ===
    at:
      file: test/tap/00-verify-ls-ok.js
      line: 15
      column: 7
    source: |
      t.equal(code, 0, "npm ls exited with code")
    stack: |
      test/tap/00-verify-ls-ok.js:15:7
      f (node_modules/once/once.js:17:25)
      ChildProcess.<anonymous> (test/common-tap.js:56:5)
      maybeClose (internal/child_process.js:763:16)
      Process.ChildProcess._handle.onexit (internal/child_process.js:210:5)

@zkat
Copy link
Contributor Author

zkat commented Jun 5, 2015

Hi, @Fishrock123!

Both @othiym23 and I are seeing a different failure when running make test-npm:

zkat@Kats-MacBook-Pro:~/Documents/code/release-iojs(npm-2.11.1⚡) » make test-npm
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C out BUILDTYPE=Release V=1
make[1]: Nothing to be done for `all'.
ln -fs out/Release/iojs iojs
NODE_EXE=iojs tools/test-npm.sh
module.js:334
    throw err;
          ^
Error: Cannot find module 'internal/smalloc'
    at Function.Module._resolveFilename (module.js:332:15)
    at Function.Module._load (module.js:282:25)
    at Module.require (module.js:361:17)
    at require (module.js:380:17)
    at evalmachine.<anonymous>:22:20
    at Object.<anonymous> (/Users/zkat/Documents/code/release-iojs/test-npm/node_modules/graceful-fs/fs.js:11:1)
    at Module._compile (module.js:426:26)
    at Object.Module._extensions..js (module.js:444:10)
    at Module.load (module.js:351:32)
    at Function.Module._load (module.js:306:12)
make: *** [test-npm] Error 1

As far as the debug@* thing you're seeing goes -- are you rebasing against a different SHA? If so, can you let me know against which one? I can look further once I get past the internal/smalloc thing.

Thanks a lot for reviewing! :)

@Fishrock123
Copy link
Contributor

@zkat could you rebase against master? That error is caused by an issue related to graceful-fs that has been temporarily fixed by 2dcef83.

@zkat
Copy link
Contributor Author

zkat commented Jun 5, 2015

@Fishrock123 I did a rebase and ran the tests -- 2526 passing (4m).

See if it works for you now :)

@Fishrock123
Copy link
Contributor

If I run npm list from deps/npm I get this:

├─┬ node-gyp@2.0.1
│ ├─┬ glob@4.5.3
│ │ └─┬ minimatch@2.0.8
│ │   └─┬ brace-expansion@1.1.0
│ │     ├── balanced-match@0.2.0
│ │     └── concat-map@0.0.1
│ ├─┬ minimatch@1.0.0
│ │ └── sigmund@1.0.1
│ ├─┬ path-array@1.0.0
│ │ └─┬ array-index@0.1.1
│ │   └── UNMET DEPENDENCY debug@*

zkat added a commit to npm/node that referenced this pull request Jun 5, 2015
On case insensitive platforms, the rule was catching the node module
under npm and eslint.

This should fix the issue, and make nodejs#1899 mergeable.
@othiym23 othiym23 changed the title Npm 2.11.1 deps: update npm to 2.11.1 Jun 5, 2015
@othiym23
Copy link
Contributor

othiym23 commented Jun 5, 2015

I've confirmed that the issue mentioned by @Fishrock123 is due to the relevant module not being included in @zkat's push, because of https://github.com/nodejs/io.js/blob/a5bd466440f49bc6ba74c99714112a333ab71bd9/.gitignore#L24. This is addressed by #1908, which should be landed before we fix / land this PR.

Fishrock123 pushed a commit that referenced this pull request Jun 5, 2015
On case insensitive platforms, the rule was catching the debug module
under npm and eslint.

See: #1899 (comment)
PR-URL: #1908
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
zkat and others added 3 commits June 5, 2015 17: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 <info@bnoordhuis.nl>
The delay-load hook allows node.exe/iojs.exe to be renamed. See efadffe
for more background.

PR-URL: nodejs#1433
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@Fishrock123
Copy link
Contributor

@zkat so what I understand is that the patch to gitignore still isn't enough and needs to be improved before this / this week's npm can be updated + landed, is that correct?

@zkat
Copy link
Contributor Author

zkat commented Jun 9, 2015

We spent a while digging around git docs and the internets. .gitignore is pretty confusing and corner casey when it comes to ignoring directories. The immediate solution we came up with was to simply set core.ignorecase to false on OSX.

We still need a better solution going forward to prevent issues with case insensitivity, since we can't just commit this configuration. For example, in this case, we should look into whether these Visual Studio files actually do show up in various subdirectories, and try to be more specific with ignores.

That said, I fixed this PR so it does include the missing dependencies properly, and the tests should pass again.

Fishrock123 pushed a commit that referenced this pull request Jun 10, 2015
PR-URL: #1899
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Forrest L Norvell <forrest@npmjs.com>
@Fishrock123
Copy link
Contributor

Thanks, landed in f500e18...91d0a8b 🎉

@rvagg rvagg mentioned this pull request Jun 11, 2015
rvagg added a commit that referenced this pull request Jun 13, 2015
Notable Changes:

* libuv: Upgraded to 1.6.0 and 1.6.1, see full ChangeLog for details.
  (Saúl Ibarra Corretgé) #1905 #1889. Highlights include:
  - Fix TTY becoming blocked on OS X
  - Fix UDP send callbacks to not to be synchronous
  - Add uv_os_homedir() (exposed as os.homedir(), see below)
* npm: See full release notes for details. (Kat Marchán) #1899. Highlight:
  - Use GIT_SSH_COMMAND (available as of Git 2.3)
* openssl:
  - Upgrade to 1.0.2b and 1.0.2c, introduces DHE man-in-the-middle protection
    (Logjam) and fixes malformed ECParameters causing infinite loop
    (CVE-2015-1788). See the security advisory for full details.
    (Shigeki Ohtsu) #1950 #1958
  - Support FIPS mode of OpenSSL, see README for instructions.
    (Fedor Indutny) #1890
* os: Add os.homedir() method. (Colin Ihrig) #1791
* smalloc: Deprecate whole module. (Vladimir Kurchatkin) #1822
* Add new collaborators:
  - Alex Kocharin (@rlidwka)
  - Christopher Monsanto (@monsanto)
  - Ali Ijaz Sheikh (@ofrobots)
  - Oleg Elifantiev (@Olegas)
  - Domenic Denicola (@domenic)
  - Rich Trott (@Trott)
Trott added a commit to Trott/io.js that referenced this pull request Feb 8, 2017
* Remove unneeded temp dir cleanup
* Add check for error in `.close()` callback
* Improve error reporting

On that last bullet point, the previous version of the test reported
errors like this:

```
AssertionError: [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen deepStrictEqual [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen
```

Now, they look like this:

```
AssertionError: 2a is hex value for * and not catch-stdout-error.js
```
Trott added a commit to Trott/io.js that referenced this pull request Feb 10, 2017
* Remove unneeded temp dir cleanup
* Add check for error in `.close()` callback
* Improve error reporting

On that last bullet point, the previous version of the test reported
errors like this:

```
AssertionError: [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen deepStrictEqual [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen
```

Now, they look like this:

```
AssertionError: expected *, got ! by hex decoding 2a
```

PR-URL: nodejs#11232
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit that referenced this pull request Feb 13, 2017
* Remove unneeded temp dir cleanup
* Add check for error in `.close()` callback
* Improve error reporting

On that last bullet point, the previous version of the test reported
errors like this:

```
AssertionError: [ '.empty-repl-history-file',
  '.node_repl_history',
  'GH-1899-output.js',
  'GH-892-request.js',
  'a.js',
  'a1.js',
  'agen deepStrictEqual [ '.empty-repl-history-file',
  '.node_repl_history',
  'GH-1899-output.js',
  'GH-892-request.js',
  'a.js',
  'a1.js',
  'agen
```

Now, they look like this:

```
AssertionError: expected *, got ! by hex decoding 2a
```

PR-URL: #11232
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 14, 2017
* Remove unneeded temp dir cleanup
* Add check for error in `.close()` callback
* Improve error reporting

On that last bullet point, the previous version of the test reported
errors like this:

```
AssertionError: [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen deepStrictEqual [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen
```

Now, they look like this:

```
AssertionError: expected *, got ! by hex decoding 2a
```

PR-URL: nodejs#11232
Reviewed-By: James M Snell <jasnell@gmail.com>
krydos pushed a commit to krydos/node that referenced this pull request Feb 25, 2017
* Remove unneeded temp dir cleanup
* Add check for error in `.close()` callback
* Improve error reporting

On that last bullet point, the previous version of the test reported
errors like this:

```
AssertionError: [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen deepStrictEqual [ '.empty-repl-history-file',
  '.node_repl_history',
  'nodejsGH-1899-output.js',
  'nodejsGH-892-request.js',
  'a.js',
  'a1.js',
  'agen
```

Now, they look like this:

```
AssertionError: expected *, got ! by hex decoding 2a
```

PR-URL: nodejs#11232
Reviewed-By: James M Snell <jasnell@gmail.com>
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.

6 participants