Reorganize `Process` to include extra global states
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 7k
- Forks
- 1.1k
- Avg merge
- 22h 40m
- Merged PRs (30d)
- 46
Description
This was part of the original plan after #3803, but we probably have put it aside for a while:
In https://github.com/rust-lang/rustup/pull/4182#issuecomment-2663126604, I've noticed that the current Process abstraction should probably be extended to allow the existence of a tracing_subscriber global state, so that we can handle graceful shutdown more easily.
Having noticed the clear pattern here:
... it should probably be modeled as an RAII guard.
I have also noticed that this should resolve another problem, i.e. the corresponding construct in tests is simply no longer used, as these two functions no longer have a caller:
This is most probably caused by the removal of rustup_macros (https://github.com/rust-lang/rustup/pull/3877/commits/d60f0058503954f57786d16e15a2d42f6798e141): indeed, the tests are passing on the CI, but sometimes one might want to run tests with otel on as well.
@djc What do you think?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read src/bin/rustup-init.rs around the existing global-state setup and src/test.rs around the unused test functions. Review the discussion in #3803 and pull request #4182 before deciding how Process should represent the tracing_subscriber state. Done means the RAII-based organization supports graceful shutdown and tests can still run with otel enabled.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cli, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100