Skip to content

Make diagnostic logging safe and bounded#157

Open
camjac251 wants to merge 3 commits into
andy-5:developfrom
camjac251:fix/diagnostic-logging
Open

Make diagnostic logging safe and bounded#157
camjac251 wants to merge 3 commits into
andy-5:developfrom
camjac251:fix/diagnostic-logging

Conversation

@camjac251

Copy link
Copy Markdown

Summary

  • replace argument and path logging with metadata-only diagnostic events
  • record dispatch, completion, duration, exit status, and process error kind
  • cap the active log at 1 MiB and retain one rotated backup
  • serialize rotation and writes with a non-blocking Windows named mutex
  • make every logging, rotation, and contention failure non-fatal to Git
  • document how wrapper diagnostics differ from Fork's verbose Git output

Problem

The existing opt-in log can contain command arguments, paths, environment-derived
values, and operating-system error text. It is unbounded, and an I/O failure can
interrupt the Git operation that logging is meant to diagnose. Concurrent wrapper
processes can also race during rotation and append.

Implementation

Diagnostic records now contain only controlled event names, counts, routing
targets, timestamps, process IDs, exit status, durations, and io::ErrorKind.
They do not serialize arguments, repository paths, environment values, OS error
messages, or credentials.

A non-blocking named mutex covers the complete size-check, rotation, and append
sequence. Contended records are dropped instead of delaying Git. Each line is
assembled before one write_all call, and all logging failures are ignored.

Verification

  • focused logging suite: 6 unit tests and 3 integration tests passed
  • concurrent rotation stress: 8 Windows wrapper processes passed with bounded,
    complete logs and preserved rotated history
  • process-error privacy contract test passed
  • Windows-target suite: 25 unit tests and 16 integration tests passed with one
    unrelated host-shell fixture excluded
  • cargo build --release --target x86_64-pc-windows-gnu
  • cargo fmt --all -- --check
  • cargo clippy --target x86_64-pc-windows-gnu --bin wslgit --tests
  • git diff --check

Diagnostic output previously recorded arguments and paths, could fail Git
operations on I/O errors, and grew without a limit. Emit metadata-only
records, cap and rotate the log under a non-blocking process mutex, and
treat logging failures or contention as best-effort drops.
Clarify that wrapper diagnostics are opt-in and independent from Fork's
verbose Git output. Document the metadata contract, size bounds, dropped
records under contention, and the intended troubleshooting workflow.
Use the direct string length and pass path and argument values in the
forms expected by the command APIs. This keeps the new diagnostics code
from adding warnings to repositories that enforce a clean Clippy run.
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

Successfully merging this pull request may close these issues.

1 participant