Re: [PATCH] read-cache: use index state repository for trace2 logging
- From
jayesh0104 <jayeshdaga99@gmail.com>
- Date
- Mar 28, 2026, 05:25 UTC
- Message-ID
- <20260328052505.76445-1-jayeshdaga99@gmail.com>
- In-Reply-To
- <xmqqqzp5kzj3.fsf@gitster.g>
Hi Junio, Derrick,
Thanks for the detailed review and suggestions.
On the fallback to `the_repository`: I agree with your observation that `istate->repo` being NULL would indicate a bug rather than a scenario to defensively handle. My initial intent was to be conservative in case there were edge paths where `istate->repo` might not be initialized, but given that INDEX_STATE_INIT(r) sets this unconditionally, it makes sense to rely on that invariant instead of masking potential issues. I will drop the fallback and use `istate->repo` directly (and verify via the test suite).
Regarding scope, Derrick’s suggestion to split this into separate commits makes sense. I’ll proceed as follows:
1. A focused patch that replaces `the_repository` with `istate->repo` in the trace2 calls within this file and removes the associated TODO comments.
2. A follow-up patch that replaces other uses of `the_repository` in places where an `istate` is already available (e.g., `refresh_index()`, `tweak_untracked_cache()`, `do_write_index()`, etc.), keeping changes logically grouped for easier review.
I’ll also run the full test suite after removing the fallback to confirm there are no hidden assumptions.
Junio, thanks also for pointing out the author identity. I’ll update it to use my real name in the next version.
Thanks again for the guidance.
Best, Jayesh