Skip to content

Loading…

Output a better error message if Git is missing #3297

Closed
domenic opened this Issue · 8 comments

5 participants

@domenic
npm member

See #3291 for some confusion. "spawn ENOENT" is not so great.

@robertkowalski
npm member

Had a first look into this issue.

My assumption is that adding a general message for the type ENOENT in lib/utils/error-handler.js is not enough, as there could be other execs/spawns that could miss a program like git.

What would be the preferred way to solve this? Catching it somewhere and change it to a custom one like ENOGIT?

@guybrush

maybe we could use require('which'). the command is mostly used in cache.js i think, though git grep says also submodule.js and version.js

$ git grep -n 'npm.config.get("git")'
lib/cache.js:432:    var git = npm.config.get("git")
lib/cache.js:455:    exec( npm.config.get("git"), ["clone", u, p]
lib/cache.js:470:  var git = npm.config.get("git")
lib/submodule.js:58:  exec( npm.config.get("git"), [ "submodule", "update", "--init"
lib/submodule.js:64:  exec( npm.config.get("git"), [ "submodule", "add", url
lib/submodule.js:74:  exec( npm.config.get("git"), ["submodule", "status"], null, false
lib/version.js:75:  exec( npm.config.get("git"), ["status", "--porcelain"], process.env, false
lib/version.js:90:        ( [ [ exec, npm.config.get("git")
lib/version.js:92:          , [ exec, npm.config.get("git")
lib/version.js:94:          , [ exec, npm.config.get("git")
@robertkowalski robertkowalski added a commit to robertkowalski/npm that referenced this issue
@robertkowalski robertkowalski Show an error message if git is missing, fixes #3297 fea4515
@isaacs isaacs closed this in 82b9e7d
@robertkowalski
npm member

:D

@othiym23

Yeah, I think improving the messaging here falls under the bucket of "better error messages & reporting", which is on the roadmap, but could definitely be worked on piecemeal, if people want to pitch in (HINT HINT).

@domenic
npm member

Looking through those bug reports, it seems like the problem is that we don't bail after encountering ENOGIT, but instead go on to allow lots of other errors to pop up.

Perhaps this occurs because we are doing many installs in parallel and only check for the presence of Git once it becomes needed.

@othiym23

That is exactly what is going on, and also because frequently there are failures during rollback (an issue I am hoping we will address soon, by cleaning up how locking works). This can be addressed during the multi-stage install work, but it would still be useful to have a more explicit and actionable error message than ENOGIT.

@timoxley
npm member

I think a big part of people not reading the logs are the log prefixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.