Re: Referring to commits in commit messages

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 20, 2018, 00:18 UTC
Message-ID: <8736qtf3u9.fsf@evledraar.gmail.com>
In-Reply-To: <20181219221401.GC228469@google.com>

On Wed, Dec 19 2018, Jonathan Nieder wrote:

Show 75 quoted lines

> Hi,
>
> Duy Nguyen wrote:
>> On Wed, Dec 19, 2018 at 6:04 PM Ævar Arnfjörð Bjarmason
>
>>> E.g. when composing
>>> https://public-inbox.org/git/878t0lfwrj.fsf@evledraar.gmail.com/ I
>>> remembered PERLLIB_EXTRA went back & forth between
>>> working/breaking/working with your/my/your patch, so:
>>>
>>>     git log --grep=0386dd37b1
>>>
>>> Just found the chain up to my breaking change, but not your 7a7bfc7adc,
>>> which refers to that commit as v1.9-rc0~88^2.
> [...]
>> To follow email model, this sounds like a good trailer for related
>> commits, like In-Reply-To for email. We could even have special
>> trailer "Fixes" to indicate what commit is the problem that this
>> commit fixes.
>
> In Linux kernel land, Documentation/process/submitting-patches.rst
> contains the following:
>
> -- >8 --
> If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> the SHA-1 ID, and the one line summary.  For example::
>
> 	Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()")
>
> The following ``git config`` settings can be used to add a pretty format for
> outputting the above style in the ``git log`` or ``git show`` commands::
>
> 	[core]
> 		abbrev = 12
> 	[pretty]
> 		fixes = Fixes: %h (\"%s\")
> -- 8< --
>
> I like it because (1) the semantics are clear, (2) it's very concrete
> (e.g. "first 12 characters", (3) it goes in a trailer, where other
> bits intended for machine consumption already go.
>
> Should we adopt it?  In other words, how about something like the
> following?
>
> If it seems like a good idea, I can add a commit message.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> diff --git i/Documentation/SubmittingPatches w/Documentation/SubmittingPatches
> index ec8b205145..36ce1ac5d8 100644
> --- i/Documentation/SubmittingPatches
> +++ w/Documentation/SubmittingPatches
> @@ -367,6 +367,20 @@ If you like, you can put extra tags at the end:
>  You can also create your own tag or use one that's in common usage
>  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
>
> +If your patch fixes a bug in a specific commit, e.g. you found an issue using
> +``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> +the SHA-1 ID, and the one line summary.  For example::
> +
> +	Fixes: 539047c19e ("revert: introduce --abort to cancel a failed cherry-pick")
> +
> +The following ``git config`` settings can be used to add a pretty format for
> +outputting the above style in the ``git log`` or ``git show`` commands::
> +
> +	[core]
> +		abbrev = 12
> +	[pretty]
> +		fixes = Fixes: %h (\"%s\")
> +
>  == Subsystems with dedicated maintainers
>
>  Some parts of the system have dedicated maintainers with their own

The core.abbrev=12 part of this I don't think would be a good idea, and have submitted a patch to the kernel to remove it: https://lore.kernel.org/lkml/20181220000112.24891-1-avarab@gmail.com/

If we find ourselves wanting to tweak core.abbrev for git.git, we should really take a step back and see if we can just fix the find_unique_abbrev_r() behavior so neither us nor anyone else should need to fiddle with it.

As noted on LKML I have upcoming patches to support core.abbrev relative values, e.g. "+2" will give you really future-proof SHAs. That should be Good Enough(TM) for most.

The only real improvement over the approximate_object_count() method I can think of is something where in gc / index-pack (for clone) we write out some statistics that allow us to later cheaply estimate the long-term growth curve of the repository, and e.g. say that a short SHA-1 should always be good for at least N years before it becomes ambiguous.

OTOH we could also just say that if you're a repo with >= 11 characters for abbreviation we might as well consider you in big boy territory, and just snap it to say 16 and be done with it. We'll have problems with 32 bit ints somewhere in git overflowing before we'd roll over to "17".