Re: [PATCH 0/3] git-describe deals gracefully with broken submodules

> Stefan Beller <sbeller@google.com> writes:
>
>> Our own version generation in GIT-VERSION-GEN is somewhat sane by testing
>> if we have a .git dir, and use that as a signal whether the obtained
>> copy of git was obtained using git (clone/fetch) or if it is just a
>> downloaded tar ball.
>>
>> Other scripts to generate a version are not as cautious and just run
>> "git describe". An error from git-describe is treated as a sufficient
>> signal to assume it is not a git repository.
>>
>> When submodules come into play, this is not true, as a submodule
>> may be damaged instead, such that we're still in a git repository
>> but error out for the sake of reporting a severly broken submodule.
>>
>> Add a flag to git-describe that instructs it to treat severe submodule
>> errors as "dirty" instead of erroring out.
>
> I do not have a strong preference for or against the "treat a broken
> repository as if nothing is wrong with the revision, but just mark
> it as dirty" idea.  I would be more receptive if it substituted the
> "-dirty" marker with something else, e.g. "-broken", though.

>
> My knee-jerk reaction to the code change is that treating submodule
> as something very special is probably not a good idea.  Even if you
> do not use submodules, if some of the objects referenced from your
> index and/or HEAD are damaged or otherwise causes some error while
> accessing, the diff machinery would die, wouldn't it?

> I saw that some new symbolic constants in the code to tell the
> machinery to "gracefully die" (or "hide the breakage under the rug")
> are named with SUBMODULE in them, which is probably a bad sign that
> the design is being too centric to submodules.  The implementation
> that covers only breakages in submodule as its first step may be OK
> (you have to start somewhere, after all), but I think the aspiration
> should be to cover all kinds of breakages in the end and turn them
> to be "graceful", and if you had that goal in mind, you wouldn't be
> naming these constants with SUBMODULE in them.

>
> If "treat a broken repository as just being 'dirty'" were a good
> idea, I'd suspect that we would want to see all breakages, not just
> ones related to submodules, to be treated the same way.
>
> But it is possible there may be a reason why submodules are special.
> I do not think the third paragraph quoted above is a good
> justification.  A repository with broken submodule is just as broken
> and untrustworthy as a broken repository without a submodule, and if
> you want to allow such a checkout with broken submodule to call
> itself v2.0-dirty, you would also want to allow a broken checkout
> without any submodule to do so, too.

> I probably would
>
> - introduce a new "git describe --possibly-broken" option;
>
> - instead of running "diff-index" internally to decide the "-dirty"
>   state, spawn "diff-index" as a separate process;
>
> - observe the exit status from "diff-index" and add "-dirty" suffix
>   when it is _known_ to be dirty, add "-broken" suffix when it
>   died, and leave out the suffix when we know that the checkout is
>   clean.
>
> That way, we wouldn't have to contaminate the generic codepath with
> a "treat broken and modified states as if they are the same" logic,
> only to support such a niche feature that we wouldn't want to use
> anywhere else.