Re: [PATCH] Do check_repository_format() early

"Nguyen Thai Ngoc Duy" [off-list ref] writes:

On Dec 2, 2007 1:58 AM, Junio C Hamano [off-list ref] wrote:

quoted

 * 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?

I think I phrased the above (2) not brilliantly.  I meant your "third
choice" is the sane approach.  Treat t/trash/test as a found place that
we do not understand, perhaps issue a warning saying that we will
operate in there but the repository version is too new for us to
understand, but still go ahead and operate in there without doing any
repository operation (so the plain git-apply without --index will act as
if it is a GNU patch called to modify files in that directory).