fix(cli): initialize and flush logging for every subcommand (#194)#195
Merged
Conversation
Previously only `run` and `patches` initialized the tracing subscriber, because the call to `init_logging_full` lived inside `CliRunner::with_options` and only those two commands constructed a `CliRunner`. Every other CLI subcommand (`packages`, `config`, `diagnostics`, `cache`, `scenery-index`, `publish`, `setup`, `init`, `download`) ran with `tracing` events going nowhere — `RUST_LOG=trace` produced no log file. This bit us during the #186 / #187 diagnostic round-trip with dee-sea. Lift logging initialization out of `CliRunner` and into `main.rs` so it runs once before subcommand dispatch. `CliRunner` is now config-only; the logging guard is held for the lifetime of `main` via a local binding. Promote `--debug` and `--profile` to top-level `Cli` flags with `global = true`. They now apply to every subcommand and are visible to `init_cli_logging` before dispatch. `xearthlayer run --debug` keeps working unchanged thanks to clap's global-flag positioning, and `xearthlayer --debug packages install eu` is now possible — exactly the diagnostic flow that #186 needed. New module `logging_init` with `derive_log_paths` (pure helper) and `init_cli_logging` (config-aware wrapper around `init_logging_full`). Four unit tests cover path derivation edge cases (normal, bare filename, root path, deeply nested). Robustness: if `ConfigFile::load()` fails (parse error or first-run without a config file), main falls back to `ConfigFile::default()` so logging still initializes. If `init_cli_logging` itself fails (e.g., log directory unwritable), main prints a stderr warning and continues without a log file rather than aborting. Manual verification: - `RUST_LOG=trace xearthlayer config get general.update_check` → `~/.xearthlayer/xearthlayer.log` is created (was missing before) - `xearthlayer --debug diagnostics` → log file populated with INFO events from wgpu detection - `xearthlayer run --debug --help` and `xearthlayer --help` both surface the global flags correctly Fixes #194 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… error exit Two related changes in service of #194's underlying goal — making the log file actually useful for diagnosing package-install failures like the one dee-sea reported in #186. Flush on error exit: Previously `CliError::exit` called `std::process::exit` directly, which skips local destructors. The `LoggingGuard` in main therefore never dropped on the error path, and `tracing-appender`'s background writer was killed mid-flight with its queue unflushed — every failed CLI invocation produced an empty log file. Replace `exit` with `report() -> u8`; have `main` return `ExitCode` so the guard drops normally and the appender flushes before the process terminates. Structured tracing across the install pipeline: Add INFO events at every stage transition in `installer.rs` — package install started, metadata fetched, download phase started / completed (with bytes + elapsed_secs), reassembly started / completed, extraction started / completed, package install completed. All emitted with structured fields (region, version, parts, bytes, etc.) for grep / awk analysis. In `download/orchestrator.rs`: DEBUG event when a part download starts (with url and filename), DEBUG on success (with bytes), DEBUG on abort (sibling failed first). WARN on each retry (with attempt number and last error). ERROR when retries exhaust (with attempts count and last error). Manual verification: - `xearthlayer --debug packages install eu` (errors AlreadyInstalled) now writes "package install started" to the log instead of leaving it empty - `xearthlayer --debug diagnostics` (success path) still writes its wgpu detection events - All 2,404 unit tests still pass via `make pre-commit` This unblocks the #186 diagnostic loop: when dee-sea retries on his build of `main`, his failed install will produce a structured log showing exactly which parts failed, how many retries each got, and what the underlying error message was. Refs #194 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three coordinated changes that take the CLI's log file from "exists but empty for the commands that need it most" to "useful for diagnosing real package-install failures." Filed as #194 after dee-sea's diagnostic round-trip on #186 went sideways because `RUST_LOG=trace xearthlayer packages install eu` produced no log file.
1. Initialize logging for every subcommand (`20b1e85`)
Previously only `run` and `patches` initialized the tracing subscriber, because the call to `init_logging_full` lived inside `CliRunner::with_options` and only those two commands constructed a `CliRunner`. Every other CLI subcommand ran with `tracing` events going nowhere.
Lift logging initialization out of `CliRunner` and into `main.rs` so it runs once before subcommand dispatch. `CliRunner` is now config-only.
Promote `--debug` and `--profile` to top-level `Cli` flags with `global = true`. They now apply to every subcommand. `xearthlayer run --debug` keeps working unchanged thanks to clap's global-flag positioning, and `xearthlayer --debug packages install eu` is now possible.
2. Flush log on error exit (`55b0e3b`, first half)
Discovered while verifying #1: `CliError::exit` called `std::process::exit` directly, which skips local destructors. The `LoggingGuard` therefore never dropped on the error path, and `tracing-appender`'s background writer was killed mid-flight with its queue unflushed — every failed CLI invocation produced an empty log file even after #1.
Replace `exit` with `report() -> u8`; have `main` return `ExitCode` so the guard drops normally before process termination.
3. Structured tracing across the install pipeline (`55b0e3b`, second half)
Added INFO events at every stage transition in `installer.rs` — install started, metadata fetched, download phase started/completed (with bytes + elapsed_secs), reassembly started/completed, extraction started/completed, install completed. All emitted with structured fields (region, version, parts, bytes, etc.) for grep / awk analysis.
In `download/orchestrator.rs`: DEBUG event when a part download starts, DEBUG on success, DEBUG on abort. WARN on each retry. ERROR when retries exhaust.
Test plan
What dee-sea will see when he retries on `main`
When he runs `RUST_LOG=trace xearthlayer packages install eu` from a fresh build, his failed install will now produce a log file showing exactly which parts failed, how many retries each got, and what the underlying error message was — exactly the diagnostic we asked for in #186 but couldn't capture before.
Fixes #194
🤖 Generated with Claude Code