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

Logging: Swap println/eprintln for a logging library #924

Open
antheas opened this issue Dec 1, 2024 · 2 comments
Open

Logging: Swap println/eprintln for a logging library #924

antheas opened this issue Dec 1, 2024 · 2 comments

Comments

@antheas
Copy link
Contributor

antheas commented Dec 1, 2024

Currently, bootc uses the println and eprinln macros to log command progress. This makes it so that it is difficult to output structured output for programs to consume, as both 0 and 1 fds are used interchangeably for output, and to control the output logging level.

This is especially true for #921 , as in its current stage it requires the --json-fd=<fd> param, which introduces an unsafe block and it will be especially difficult to consume from bash (requires opening a new descriptor and makes difficult to pipe).

Therefore, bootc should switch to a standard logging framework that pipes to stderr exclusively, and better yet through indicatif, so that both progress bars and structured output over stdout work correctly and universally.

If such change happens before #921 is included in a release, --json-fd=<fd> can be dropped in favor of --json without introducing incompatibilities.

@cgwalters
Copy link
Collaborator

Therefore, bootc should switch to a standard logging framework that pipes to stderr exclusively, and better yet through indicatif, so that both progress bars and structured output over stdout work correctly and universally.

This is possible, but as it's a process-global thing it may be a bit tough to pull off quickly.

This is especially true for #921 , as in its current stage it requires the --json-fd= param, which introduces an unsafe block

I'm not concerned about the unsafe block.

and it will be especially difficult to consume from bash (requires opening a new descriptor and makes difficult to pipe).

I agree it should be possible to consume from bash but I also think bash is also a pretty sad "lowest common denominator".

I'll play a bit with what it looks like to handle output from fd3 from bash as a reference.

@antheas
Copy link
Contributor Author

antheas commented Dec 3, 2024

Therefore, bootc should switch to a standard logging framework that pipes to stderr exclusively, and better yet through indicatif, so that both progress bars and structured output over stdout work correctly and universally.

This is possible, but as it's a process-global thing it may be a bit tough to pull off quickly.

Agreed, I can play with the fd implementation for now. I will make sure dropping --json-fd will not cause issues as far as ublue is concerned even if there is a bootc release with it.

and it will be especially difficult to consume from bash (requires opening a new descriptor and makes difficult to pipe).

I agree it should be possible to consume from bash but I also think bash is also a pretty sad "lowest common denominator".

It is also harder to consume from python. Reading from stdout and writing to stdin is built-in in both POpen and run functions.

https://docs.python.org/3/library/subprocess.html#popen-constructor

For progress, POpen would be used with stdout=subprocess.PIPE, then Python automatically creates the pipe and it can be consumed with the file handle proc.stdout. It also cleans up when the process exits.

--json-fd would be custom. Around 5 lines of code with some cleanup. It is nothing crazy, but at the same time not ideal.

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

No branches or pull requests

2 participants