Re: [PATCH] Do check_repository_format() early

> On Dec 2, 2007 1:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>  * Similarly, run a few commands in modes that do not require git
>>    repository.  For example, "git apply --stat" of an existing patch
>>    should be viewable no matter where you are (that is just a "better
>>    diffstat" mode), so ideally it should not barf only because you
>>    happen to be in a repository that is too new for you to understand.
>>    I do not know offhand how your patch would handle this situation.
>>
>> Note that making sure the latter works is tricky to do right, for a few
>> reasons.
>>
>>  (1) It is not absolutely clear what the right behaviour is.  It could
>>      be argued that we should just barf saying we found a repository we
>>      do not understand, refraining from doing any damange on it [*2*].
>>
>>  (2) If we choose not to barf on such a repository, it remains to be
>>      decided what "gently" should do --- if it should still treat
>>      t/trash/test (which has too new a version) as the found repository,
>>      or ignore it and use t/trash (which we can understand) as the found
>>      repository.  I think it should do the former.
>
> You might have forgotten the third choice: ignore t/trash/test and
> stop searching, instead pretend there is no repository at all (maybe
> with a big warning of unsupported repository).
>
> I agree t/trash should not be touched no matter what. I had enough
> "fun" with nested gitdir already. But if _gently() treats t/trash/test
> as a good repository, mysterious things may happen. Suppose gitdir v2
> supports some crazy refspec that current installed git cannot
> understand. Now you run git-remote on a v2 repository, it would end up
> barfing "invalid refspec" or something instead of "your repository
> version is not supported, upgrade git now". The latter error message
> is much clearer IMHO.
>
> If we are going "t/trash/test is good repo" route, we must make sure
> _gently() callers check repository version (and barf at proper places)
> before actually using it. Doing so makes repo version checking in
> _gently redundant, you need to check it from callers anyway as the
> callers will decide when to barf. Or return  *nongit_ok=-1 and let the
> callers check return value so they do not need to run
> check_repository_format_version() again.
>
> Comments?