Remove lib/utils/lifecycle.js (and add npm-exec) #3313

Closed
isaacs opened this Issue Apr 4, 2013 · 27 comments

7 participants

@isaacs
npm member

This should be replaced with a standalone package that executes a command in the context of a package.

The script runner thingie should emit warn/info/verbose events so that we can log the same kinds of things that we do now (or perhaps use an optionally-supplied npmlog object for logging?) but should not write to stdio by default.

Also, the module should be smart enough to not try to run two commands at the same time, since that usually just ends up causing headaches.

This should be used by a new npm exec command, which is called by npm run-script, which should be called instead of calling lifecycle(...) directly.

This would also let people do things like npm exec -- some arbitrary shell stuff which would be nice, rather than requiring that it be added to package.json ahead of time.

The run-script.js should be where the "note" gets logged to stdout.

cc: @ForbesLindesay

@ForbesLindesay
npm member

Where should the functionality currently in lifecycle.js makeEnv go? It seems to pull in all the settings for the entirety of npm and place them in the environment?

@ForbesLindesay
npm member

Do we still need npat as an option to set TAP = 1. This seems really odd and nothing to do with running commands.

@ForbesLindesay
npm member

Actually, scratch that last comment, it's part of makeEnv anyway

@ForbesLindesay
npm member

What's the purpose of this line

@isaacs
npm member

I think the lifecycle script runner thingie should maybe take the same options as spawn(), but it should add all the npm_package_* environment variables from the package.json file. (It's ok to depend on read-package-json, imo.)

The npm config environs should be added by npm-exec.

The npat check and TAP=1 should be added by npm-test.

https://github.com/isaacs/npm/blob/master/lib/utils/lifecycle.js#L30 <-- legacy garbage

@ForbesLindesay
npm member

Shouldn't the lifecycle thingy also look up the script in package.scripts.

@isaacs
npm member

No, I think that should be done by npm-run-script.

@ForbesLindesay
npm member

so we have three modules:

npm-run-script => npm-lifecycle => npm-exec

@isaacs
npm member

Sorry, to clarify:

  • npm-run-script = npm/lib/run-script.js
  • npm-exec = npm/lib/exec.js
  • package-lifecycle (or whatever you decide to call it) = New module
@isaacs
npm member

package-exec maybe?

@ForbesLindesay
npm member

I created https://github.com/ForbesLindesay/pkgexec but started working out what went in it

I assume from that that it's npm/lib/run-script.js calls npm/lib/exec.js which calls pkgexec

@ForbesLindesay
npm member

And then we'll have to separate out npm/lib/exec into a separate module (perhaps npm-exec) which can then be used internally in place of lib/utils/lifecycle.js, to break the dependencies stopping things from being separated out.

@isaacs
npm member

Well, npm/lib/exec.js will be a new top-level command, so that you can run npm exec $command to run things as if they were a script.

@isaacs
npm member

I assume from that that it's npm/lib/run-script.js calls npm/lib/exec.js which calls pkgexec

This is correct ^

@ForbesLindesay
npm member

Yes, but my hope would be that everything in the core npm repository will ultimately become just command line arguments parsing. And since npm install depends on npm/lib/exec.js, npm/lib/exec.js will have to become a separate module before we can separate out the installation logic.

@isaacs
npm member

Ok, lovely, but that's like 50 steps away from where we're at now. Just want to make sure that you're not hoping to get there in one step :)

@grncdr

Perhaps I can interest you in this?

It's by no means complete, (in particular, it doesn't handle interactive commands very well right now) but it generally works and could be used to run the scripts in package.json in a much more cross-platform way.

@grncdr

Ok, so interactive commands work now 🍰

I also took a look at the comments on #3375, and feel like it's worth pointing out that my npm-exec actually is the code from lib/utils/lifecycle.js separated out into it's own module. I'm working on a patch to npm itself that will mostly replace lib/utils/lifecycle.js with npm-exec, but it's not ready yet.

On a related-but-different issue, what do you think of having npm <command> fall back to searching for an external npm-<command> script if no internally defined <command> is found? (e.g. what git does). That would make it much easier for end-users to compose their npm experience out of smaller tools having their own maintainers and release life-cycle.

@aseemk

On a related-but-different issue, what do you think of having npm <command> fall back to searching for an external npm-<command> script if no internally defined <command> is found?

Assuming you mean e.g. npm foobar would be equivalent to npm run foobar, I've wanted this for a long time. Big +1!

@grncdr

No, I mean npm foobar would be equivalent to npm-foobar if-and-only-if there is no internal foobar command and the npm-foobar script exists and is executable.

To see a demonstration of what I mean, put an executable script named git-mything somewhere on your PATH and run git mything, pretty cool eh?

@aseemk

Oh interesting. Where would this npm-foobar script live? Anywhere in your PATH? So it would be more of a thing for you, the individual user, rather than something associated with modules?

@grncdr

Anywhere in your PATH?

Yes, including the path modifications that npm run-script (and one day npm exec) would normally do for you. (So you could easily install npm sub-commands as modules inside your project).

@mhart

I guess this would allow us to be able to suppress the > output from run-script? At the moment it looks like there's a hardcoded console.log in lifecycle.js that doesn't obey --silent and the like - I was going to file an issue, but it looks like this covers it (although much larger in scope).

Is it worth filing a separate issue to suppress the > lines in a smaller change? Otherwise it's kinda hard to use the output of npm run as is.

@Raynos

This issue blocks passing arguments to npm run.

is there anything that can be done to move this forward ?

@grncdr

Honestly I'm not sure why @can3p's PR #4058 wasn't merged months ago. It's a pragmatic approach and I don't see any way of improving it dramatically (short of distributing a posix compliant shell with npm).

@othiym23

cc @bcoe This is relevant to npm-exec / improving npm run.

@othiym23 othiym23 added npm-exec and removed split-apart labels Jul 24, 2014
@othiym23 othiym23 added this to the 2.0.0 milestone Aug 28, 2014
@othiym23

Closing this in favor of #6053; This is an important tool to have available, and maybe we can adopt grncdr/npm-exec, but this discussion is obsoleted by the incorporation of #5518.

@othiym23 othiym23 closed this Aug 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment