mamba-org / mamba

The Fast Cross-Platform Package Manager
https://mamba.readthedocs.io
BSD 3-Clause "New" or "Revised" License
6.72k stars 346 forks source link

Libraries logging callbacks #2288

Open AntoinePrv opened 1 year ago

AntoinePrv commented 1 year ago

For libmamba to be free of side effects (such as printing), it is best to provide dependency-free callback functions for logging purposes. Users, including micromamba (or a verbose libmambacli), can then set these callbacks to forward the output to a log, a stdout/stderr...

Goals

Yes-goals

It would useful if the callback functions handle the following inputs:

No-goals

Re-writing a logging library is not a goal here. Everything that can be done by the user (eventually though a 3rd party) should be excluded, for example:

Maybe-goals

Implementation

Source

In C++20, we'de have std::source_location. Otherwise we can use macros with __FILE__ and the like (no portable function name though).

Enums vs mulitple callbacks

Should users provide one callback per log level

log_info(std::string_view msg, /* etc */);
log_debug(std::string_view msg, /* etc */);
...

Or a single function with a log level

log(mamba::LogLevel lvl, std::string_view msg)

The former is mostly useful for if there is no way to pass the message by parts (e.g. std::ostream) to the user so that temporary allocations can be deactivated as needed.

Klaim commented 1 year ago

Here is my current opinion summarized:

Some Notes:

No-Goal: Time and date (they can be computed in the user callback)

I agree, though this means that the callback must be called in-place as soon as the record is built - no other latency introduced.

In C++20, we'de have std::source_location

We could model a similar type (reuse the implementation from boost or other sources?) and replace it by std::source_location as soon as we have it available?

AntoinePrv commented 1 year ago

No formatting should happen in this system, if the callback wants to format they can, or not, that happens there.

I meant formatting in with fmt::format for utility (not layout). I feel a lot of message might look like "Error got {} but input should be smaller than {}", val, max.

Hind-M commented 1 year ago

I am writing a comment here because it's related to logging, and we should consider this when thinking about the different possible solutions: In micromamba python tests (maybe in mamba as well, I didn't check), we use json format to get the results of micromamba commands.

This leads to several issues:

Klaim commented 1 year ago

Good points @Hind-M and I would also add that the Powerloader integration PR is also "fixing" the behavior of --json and other visibility/log-level behavior because the current code is inconsistant. Basically, at the library level we will not need to handle any output- but at the executable cli level we have to handle: