Re: [PATCH v3] describe: enable sparse index for describe

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 30, 2023, 15:13 UTC
Message-ID: <xmqqtty22ry9.fsf@gitster.g>
In-Reply-To: <xmqq1ql64796.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 16 quoted lines

> "Raghul Nanth A via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Raghul Nanth A <nanth.raghul@gmail.com>
>>
>> Add usage and performance tests for describe
>
> I think this sentence can easily go.
>
>> Describe uses the index when it is run with --dirty flag, which uses the
>> run_diff_index commmand. The command is sparse-index aware and hence we
>> can just set the requires-full-index to false
>
> End the sentence with full-stop.  More importantly, it would be
> nicer to have something to substantiate the "this is sparse aware"
> claim here, something like "since commit X" or "as shown in the test
> tXXXX.YY".

With a bit of digging into the history, I think the topic that was merged at 8d2c3732 (Merge branch 'ld/sparse-diff-blame', 2021-12-21) made various modes of "git diff" to be sparse-index aware. t1092 has tests for "git diff --cached", which uses the same machinery.

    Subject: [PATCH] describe: make it sparse-index aware

    "git describe" needs to compare the index and the working tree
    when (and only when) it is run with the "--dirty" flag.  This is
    done by calling the run_diff_index() function, which has been
    already made aware of the sparse-index in the series that led to
    8d2c3732 (Merge branch 'ld/sparse-diff-blame', 2021-12-21).
    t1092 has tests for "git diff --cached", which uses this exact
    machinery.

    Mark "git describe" that the command does not require the index
    to be expanded fully beforehand.

or something like that.

> Updated tests do look good, and the code change is of course minimal
> and to the point, which is also good.

Thanks.