Add `no-print-progress` flag to `run` and `build`

[ohamill]

Implements #2299

• --no-print-progress flag has been added to the build and run commands to suppress progress output
• Add print_progress parameter to downloading_package, packages_downloaded, and compiling_package methods in Telemetry trait
• Update implementations of Telemetry trait to include new parameter

[lunagl]

~~Couldn't we let the tracing subscriber filter events/spans based on the level or target (or other metadata fields) instead of adding our own ad-hoc filtering on top?~~

Sorry misunderstood how the Telemetry system worked

[lpil]

Hello! We already have a system for this. Please pass in the Null reporter instead of the Cli reporter when this option is set. Thank you

Couldn't we let the tracing subscriber filter events/spans based on the level or target (or other metadata fields) instead of adding our own ad-hoc filtering on top?

@fschwalbe This change is unrelated to the tracing library used for logging etc.

[ohamill]

Ok understood! I'm just a little unsure how to proceed when creating the telemetry object in the main method of compiler-cli/src/build.rs. Instead of let telemetry = Box::new(cli::Reporter::new());, I think I want to do something like:

let no_print_progress = options.no_print_progress;
let telemetry: Box<dyn Telemetry> = if no_print_progress { Box::new(NullTelemetry{}) } else { Box::new(cli::Reporter::new()) };

When doing this, the compiler complains when calling let _guard = lock.lock(telemetry.as_ref()); that the size of telemetry cannot be determined at compile-time, and recommends that I consider relaxing the implicit Sized restriction in the lock method in compiler-cli/src/buiild_lock.rs. Is this an appropriate solution, or do I need to rethink the way I create the telemetry object in build.rs?

[lpil]

You could alternatively make the locker take a &dyn Telemetry, that might be easier.

[giacomocavalieri]

Hello @ohamill! Are you still working on this PR? If not I could help merge this, you did a great job here so it will probably need just some really minor adjustments 😊

[ohamill]

Hello @ohamill! Are you still working on this PR? If not I could help merge this, you did a great job here so it will probably need just some really minor adjustments 😊

Hello! Yes, sorry for the delay, I unfortunately haven't had any time lately to keep working on this. If you're able to help merge it, that would be a great help, thank you!

[giacomocavalieri]

Hello! Yes, sorry for the delay, I unfortunately haven't had any time lately to keep working on this. If you're able to help merge it, that would be a great help, thank you!

No, thank you! I'll try my best

@lpil I have a question about the Telemetry trait, could we change the code to use a Telemetry enum with different variants NullTelemetry | LogTelemetry | CliReporterTelemetry instead of the current design based on a trait? It would make passing around the telemetry object easier and remove the need for all the <Telem: Telemetry> type annotation that would pop up where it's added as an argument. I'm not sure using a trait is worth the extra complexity, especially since all implementations are empty structs but maybe I'm not seeing the problems with the enum-based approach

[lpil]

That cannot be done, no. gleam_core is pure, it does not have any ability to perform IO. There's lots of reasons why this is how it is built such as to enable more sophisticated testing and to permit the compiler to run in the browser.

[lpil]

Hello gang! Is anyone currently working on this?

[giacomocavalieri]

I was but got stuck on the refactoring, it wasn't as easy as I expected 🥲 feel free to pick this up if you want to!

[lpil]

OK! I'll close this as no one is working on it. Please feel free to reopen, thank you.