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

Filter output #13

Closed
NOhs opened this issue Mar 9, 2018 · 7 comments
Closed

Filter output #13

NOhs opened this issue Mar 9, 2018 · 7 comments
Labels
enhancement New feature or request

Comments

@NOhs
Copy link
Collaborator

NOhs commented Mar 9, 2018

Currently you are using print statements everywhere. However, if you would use functions like the python logging stuff, you could set different output levels. So people who want to integrate your code into their toolchain could silence your output or select how much output they want.

@GPMueller
Copy link
Contributor

I was not able to figure out how to consistently get a default log level while allowing it to be set from the outside at the same time as having it work correctly for the global generateDepFile and compile functions.
Any log level set outside of these functions seems to not be used inside these functions, irrespective of using logging.getLogger(__name__) and logger.setLevel(...) or setting it directly via logging.getLogger.setLevel(...).

Since a logger cannot be serialized, it unfortunately cannot be added as a member of Buildable, either.

@NOhs
Copy link
Collaborator Author

NOhs commented Mar 26, 2018

Does this help: https://stackoverflow.com/a/43794480/2305545

@GPMueller
Copy link
Contributor

Probably at some point I am misunderstanding how Python works...
Possibly the solution is to use a Handler.

However, I am still not sure how logging should really be implemented in this code, the desired behaviour would probably be the following:

  • by default print everything info and above
  • if -V is passed, also print debug
  • if a parent script uses a different log level, that should be used
  • if a parent script wants to silence console output, but write to file, that should work fine as well

But should log messages from clang-build be formatted differently in different cases (console, file, parent script's format)?

@NOhs
Copy link
Collaborator Author

NOhs commented Mar 28, 2018

So using logging and multithreading seems to be indeed not trivial, looking at discussions on SO. We could have a look at this: https://github.com/jruere/multiprocessing-logging, but having output scrambled due to concurrently running tasks is not ideal. Maybe one should buffer and then output in serial after all subprocesses have finished. This way, errors are way easier to understand.

Having different settings for writing to file and terminal is easy with logging, if this is desired. I think the default log level should be warning. Especially for larger projects I think it is way better to not have lot's of output of all the parts that work. This drives me crazy everytime I use cmake and msbuild on Windows.

@NOhs
Copy link
Collaborator Author

NOhs commented Mar 28, 2018

On that note, we could also perform output analysis of clang and automatically detect if a warning was raised.

@GPMueller GPMueller added the enhancement New feature or request label Apr 2, 2018
@GPMueller
Copy link
Contributor

I think it's best if the output from clang is forwarded without being filtered, unless clang-build is explicitly silenced. Or was there another reason you would want to detect warnings?

If we are successful in implementing logging, we should also have it write a log file automatically (e.g. build/.clang-build-log/<timestamp>.log or several logs with different levels of filtering).

@GPMueller
Copy link
Contributor

With PR #23, merged with a7bbc88, logging has been implemented and is now used quite consistently. Remaining tasks will be tracked in follow-up issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants