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

Make output should filter through "info" and "warn" loglevels. #532

Closed
ghost opened this issue Nov 11, 2014 · 6 comments
Closed

Make output should filter through "info" and "warn" loglevels. #532

ghost opened this issue Nov 11, 2014 · 6 comments

Comments

@ghost
Copy link

ghost commented Nov 11, 2014

Output from Make should be filtered into loglevels. That is, Make's stdout should be pushed to log.info and stderr should be pushed to log.warn. For example, for a small library I created:

jason@ginny:~/src/rasterize$ node-gyp rebuild --loglevel warn
make: Entering directory `/home/jason/src/rasterize/build'
  CXX(target) Release/obj.target/rasterize/rasterize.o
  SOLINK_MODULE(target) Release/obj.target/rasterize.node
  SOLINK_MODULE(target) Release/obj.target/rasterize.node: Finished
  COPY Release/rasterize.node
make: Leaving directory `/home/jason/src/rasterize/build'

I would expect no output whatsoever from a successful build when the loglevel is set to warn.

Thanks!

@ghost ghost changed the title Make output should go through "info" loglevel. Make output should filter through "info" and "warn" loglevels. Nov 11, 2014
@ghost
Copy link
Author

ghost commented Nov 11, 2014

I suppose I should note that this is desirable as the current situation is making it difficult to build tools that correctly diagnose and alert me to production deployment errors. In particular, npm install --production --quiet should only produce output if something bad happened with the building of any dependent modules in a project!

@rvagg
Copy link
Member

rvagg commented Jun 20, 2019

closing due to staleness, a pull request for this would be great though

@rvagg rvagg closed this as completed Jun 20, 2019
@cinderblock
Copy link

Is there no way to make make quiet? (This issue is the top hit for "node-gyp make make quiet" for me)

My inkling is that binding.gyp should have some option to make make quiet but I'm not finding it. Notably, I've tried setting MAKEFLAGS and MAKEOVERRIDES with gyp defines to no avail. Looking at the generated Makefile, the quiet mode "is the brief-output summary of the command".

Maybe the only option is to capture the output as this issue suggests and do different things with it.

@rvagg
Copy link
Member

rvagg commented Dec 2, 2019

I can't recall which addon it was, a popular websockets library I think, that used to have a "build", or "install" script in package.json that would send the build output to a file (bash redirect, I think it might have manually invoked something like node-gyp build >& build.out) and then on non-zero exit code would spit out the build output to stdout, otherwise it would be silent. I have only vague memories of this and it was annoying at the time because it was difficult to debug problems with it. But you could pursue something like this if this really bothered you.

Otherwise I don't think it bothers any of us enough to pursue it. But as I said above, pull requests are welcome, but it'd have to not be too invasive (i.e. it's not the kind of feature we're going to take on a ton of risky new code for).

@cinderblock
Copy link

cinderblock commented Dec 3, 2019

Yeah, I'm not a fan of that method at all.


Is there a reason this issue is closed? Would it make sense to leave it open? Ooops, missed the "staleness" reason for some reason.


Looking at the code, I only see a couple usages of spawn.

The first is a test, so we can ignore it.

The second is the implementation that I presume actually calls python/gyp. Interestingly, it looks like there was an attempt at adding a quiet option:

node-gyp/lib/node-gyp.js

Lines 169 to 171 in 3555ff4

if (!opts.silent && !opts.stdio) {
opts.stdio = [0, 1, 2]
}

The last two files with spawn occurrences are:

node-gyp/lib/build.js

Lines 188 to 189 in 5d76938

var proc = gyp.spawn(command, argv)
proc.on('exit', onExit)

node-gyp/lib/configure.js

Lines 344 to 345 in 3555ff4

var cp = gyp.spawn(python, argv)
cp.on('exit', onCpExit)

Looking at those usages, neither sets the optional 3rd argument opts which would set the quiet mode that prevents the spawned process from making any outputs.

It should be easy enough to add something to those calls, but what do we pass to them? Currently, node-gyp seems to pass all the logging (and related verboseness) to the npmlog package. So the right solution I think is to capture the output from the spawned commands (instead of always passing them through to stdin/stdout/stderr).

This does get a little bit more complicated when you think about changing the verboseness of gyp's output and possibly filtering those outputs to multiple npmlog log levels.

If we had to pick just one log level to capture make's output, what would we use?

https://github.com/npm/npmlog/blob/541407008c509755255a4819606e7916d26a77f5/log.js#L296-L304

I'm thinking notice makes sense. Or maybe we add one?

@rvagg
Copy link
Member

rvagg commented Dec 3, 2019

notice seems like a good choice to me, I have my npm set to http, so I'd get it, but the default is warn, so the majority of people wouldn't https://github.com/npm/npmconf/blob/22827e4038d6eebaafeb5c13ed2b92cf97b8fb82/config-defs.js#L180

Unless someone else speaks up here, I'd encourage you to pursue a pull request that pipes the output line by line through npmlog. I'm not sure what to do about stderr, maybe that needs to go to warn? Without looking in detail at how, off the top of my head I reckon piping the streams to split2 and then terminating in a transform stream that sends the chunks to npmlog. Off the top of my head, something like:

stdout.pipe(split2()).pipe(new Transform({
  transform: (chunk, enc, cb) => {
    log.notice(chunk.toString()); cb();
  }
})

Unless you have a better idea (or the reality of the codebase makes that too tricky).

Feel free to do a PR as a draft to start with to get feedback if you need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants