| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I'm getting some weirdness over here npm ERR! code MODULE_NOT_FOUND
npm ERR! Cannot find module 'internal/util'Eeek! It seems like the tests on v5.x work as expected as do the tests at 76cb81b when it was last upgraded. |
Sorry, something went wrong.
|
Looks like it was 1124de2 which landed about 4 hours ago and it is semver major ¯_(ツ)_/¯ PR for reference #4525 /cc @thefourtheye @jasnell @ChALkeR @bnoordhuis It looks like there was a search against the ecosystem, but perhaps not done against npm itself edit: I just have to note how crazy it is the a deprecation that was supposed to happen in 2010 finally lands an hour before a release that it breaks... I'm without words |
Sorry, something went wrong.
|
@thealphanerd, it should not fail, the API change probably had nothing to do with it. Most probably something is re-executing source code of the fs module, monkey-patching it. Will take a look later today. |
Sorry, something went wrong.
|
@ChALkeR my thoughts exactly. Seemed very odd that this would break it. npm is effectively broken on master right now |
Sorry, something went wrong.
|
@thealphanerd 1124de2 introduced require('internal/util'), which is not requirable from the external code (i.e. not from the Node.js core), so anything that wraps fs module code, monkey-patches it and re-executes it (as graceful-fs was doing a while ago) is broken by that. |
Sorry, something went wrong.
|
@thealphanerd Why? I think we decided against supporting monkey-patching core modules in that way. Some module required by npm should be fixed. Could you give me a trace, please? |
Sorry, something went wrong.
|
I was unaware of not supporting monkey-patching, but that is reasonable. That being said This version of npm has some pretty important performance improvements. Do you think it makes sense to temporarily revert the offending change until the next release of npm which can perhaps fix this? |
Sorry, something went wrong.
|
@thealphanerd See the issues and PRs I linked above in #5097 (comment) for history on this. |
Sorry, something went wrong.
|
Okay. Actual problem is one of the npm's dependencies are still using older version of cmd-shim, which uses older, monkey-patched version of graceful-fs Excerpt from npm-debug.log file generated when I ran make test-npm locally 258 verbose stack Error: Cannot find module 'internal/util' 258 verbose stack at Function.Module._resolveFilename (module.js:339:15) 258 verbose stack at Function.Module._load (module.js:290:25) 258 verbose stack at Module.require (module.js:367:17) 258 verbose stack at require (internal/module.js:16:19) 258 verbose stack at evalmachine.<anonymous>:38:26 258 verbose stack at Object.<anonymous> (/home/thefourtheye/git/io.js/test-npm/node_modules/cmd-shim/node_modules/graceful-fs/fs.js:11:1) 258 verbose stack at Module._compile (module.js:417:34) 258 verbose stack at Object.Module._extensions..js (module.js:426:10) 258 verbose stack at Module.load (module.js:357:32) 258 verbose stack at Function.Module._load (module.js:314:12) Update: On further investigation, npm itself is using cmd-shim@~2.0.1 and since there was no new release after merging npm/cmd-shim#14, npm still uses the monkey-patched graceful-fs (indirectly) and that is why this is failing. Because we don't allow userland modules to access modules in lib/internal and the older monkey-patched version of graceful-fs is trying to require internal/util. Probable fix is to have cmd-shim release a new version and then to have npm use the latest version of it. |
Sorry, something went wrong.
Sorry, something went wrong.
|
edit: |
Sorry, something went wrong.
|
Okay, updated the comment #5097 (comment) with some findings. |
Sorry, something went wrong.
|
@nodejs/ctc My opinion here is that we should in fact fix 1124de2 on the Node.js side for now, but add an explicit warning there, for at least one major version (v6.x), so that everyone who monkey-patches fs in that way (e.g uses an old version of the graceful-fs module) will be notified of the problem and will get off it, because sudden breakage with a bogus error message could be confusing. This does not cancel the fact that the problem should be fixed on cmd-shim/npm side, of course. I will make a PR for that when I'll get home. |
Sorry, something went wrong.
|
Can we just remove lower versions of graceful-fs from the dep tree? This was fixed ages ago in graceful-fs and I am not particularly sympathetic to it blatantly abusing the runtime code. |
Sorry, something went wrong.
|
@Fishrock123 That has to be done, of course, I'm not suggesting leaving an old version of graceful-fs hanging there. |
Sorry, something went wrong.
|
As soon as the updated cmd-shim is published I'll add a commit to this PR that updates it. (It'll be two weeks before it shows up in the version of npm being upstreamed directly.) @Fishrock123 We can't just remove the deeper one's from the dep tree, w/o patching cmd-shim or we'd be shipping a version of npm that's marked as invalid when you do an npm ls -g. |
Sorry, something went wrong.
|
Proposed solution:
|
Sorry, something went wrong.
|
I made a branch that reverts 1124de2 and applies b5362b5 I then used that build of node to run citgm. https://ci.nodejs.org/job/thealphanerd-smoker/59/ edit: all failures are unrelated |
Sorry, something went wrong.
This is needed to give users a grace period before actually breaking modules that re-evaluate fs sources from context where internal modules are not allowed, e.g. older version of graceful-fs module. To be reverted in Node.js 7.0. Fixes nodejs#5097, see also nodejs#1898, nodejs#2026, and nodejs#4525.
This is needed to give users a grace period before actually breaking modules that re-evaluate fs sources from context where internal modules are not allowed, e.g. older version of graceful-fs module. To be reverted in Node.js 7.0 Fixes: #5097, see also #1898, #2026, and #4525. PR-URL: #5102 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This is needed to give users a grace period before actually breaking modules that re-evaluate fs sources from context where internal modules are not allowed, e.g. older version of graceful-fs module. To be reverted in Node.js 7.0 Fixes: nodejs#5097, see also nodejs#1898, nodejs#2026, and nodejs#4525. PR-URL: nodejs#5102 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
| Back | FazBrowse Home | New Git URL |
This contains these releases:
https://github.com/npm/npm/releases/tag/v3.7.0
https://github.com/npm/npm/releases/tag/v3.7.1
Notable changes are:
r: @Fishrock123
r: @zkat