Junio C Hamano <gitster@xxxxxxxxx> writes：
> Sorry, but the above makes it sound as if 246deeac (environment:
> make `get_git_dir()` accept a repository, 2024-09-12) that retired
> get_git_dir() and introduced repo_get_git_dir() was the culprit that
> made their semantics change, but is that really true? It appears
> that in the version immediately before that commit, get_git_dir()
> was also a reference to a variable, without any lazy initialization
> the above message says that the code tries to avoid, so I am even
> more confused after reading the above.
I was reading the code in master and noticed that repo_get_git_dir()
no longer sets up the environment. I’ve learned that I should use git blame
to identify which commit changed the code, so I can make my message clearer.
> Perhaps you have 73f192c9 (setup: don't perform lazy initialization
> of repository state, 2017-06-20) in mind? That one did stop calling
> setup_git_env() and instead force a hard BUG("") when git_dir is not
> set up yet. And that BUG("") still survives in repo_get_git_dir()
> we have today.
Exactly
> So the call to repo_get_git_dir() may still not be made from this
> code path. It may not attempt to set up, but instead it would die
> if we haven't successfully set up the repository before. The
> relevance of the comment was not changed by 246deeac that moved this
> code from get_git_dir() to repo_get_git_dir(), and more importantly,
> it was not changed by this patch we are reviewing here.
>
> But stepping back a bit, is it what a9ca8a85 originally wanted to
> achieve with this comment to "avoid calling get_git_dir()" in the
> first place? Once the guarding condition is satisfied, it calls
> trace_repo_setup(), which in turn calls get_git_dir() anyway.
> Perhaps it wanted to explain why startup_info->have_repository is
> checked here?
Yes, I think so. Maybe updating the comment to say
“call repo_get_git_dir() after setting up the_repository”
would be more appropriate.
Thank you for your review,
Lidong
