shejialuo <shejialuo@xxxxxxxxx> writes: > However, on a platform with O_NOFOLLOW, this situation will also happen. > So, I think we may just use "open_nofollow" now and don't talk about the > method one at all to avoid confusing readers. Exactly. That is what you see below ;-) >> As long as both approaches are equally portable, I do not think it >> matters which one we pick from correctness point of view, and we can >> pick the one that is easier to use to implement the feature. >> >> On a platform without O_NOFOLLOW, open_nofollow() falls back to the >> lstat and open, so your "open_nofollow() is better than lstat() and >> open()" argument does not portably work, though. >> ... >> OK. I notice that the previous step did not have any new test >> associated with it. Perhaps we can corrupt "HEAD" *and* replace >> packed-refs file with a symbolic link (or do some other damage >> to the refs) and make sure both breakages are reported? > > As I have said in the previous comment, we cannot detect the error if > "HEAD" itself is corrupted. However, we will check the referent in the > later. So, we don't need to do this. I still think you absolutely need to diagnose and tell the user about the broken HEAD. With your "don't check HEAD because a repository with a broken HEAD is not a repository", a check run in such a place may find everything else in the repository perfectly fine, but because the user wanted "git refs verify" to tell them about breakages, you would want to somehow tell them about it. Either it is missing, malformed, whatever.
