Re: [PATCH 3/3] pretty: add abbrev option to %(describe)

[Junio C Hamano <gitster@pobox.com>]

Eli Schwartz <eschwartz@archlinux.org> writes:

> The %(describe) placeholder by default, like `git describe`, uses a
> seven-character abbreviated commit hash. This may not be sufficient to

"hash" -> "object name".

> fully describe all git repos, resulting in a placeholder replacement

"all git repos" -> "all commits in a given repository" (there may be
other valid way to clarify, but the point is that 'describe' does
not describe 'git repos' in the sense that my repository gets
description X while your repository gets description Y).

> changing its length because the repository grew in size. This could
> cause the output of git-archive to change.
>
> Add the --abbrev option to `git describe` to the placeholder interface
> in order to provide tools to the user for fine-tuning project defaults
> and ensure reproducible archives.

Note that it is sad that --abbrev=<n> does not necessarily ensure
reproducibility.  To be more precise, I do not think it sacrifices
uniqueness to make the output reproducible.  You can get more than N
hex-digits in the output if N is too small to ensure uniquness.

So it indeed is that this line of thought ...

> One alternative would be to just always specify --abbrev=40 but this may
> be a bit too biased...

... to use --abbrev=999 (because 40 is not the length of a full
object name in the SHA-2 world) is the only reasonable way, if what
you care about is the reproducibility.

    Side note.  I think "git describe --no-abbrev" is buggy in that
    it does not give a full object name; I didn't check the code,
    but it appears to be behaving the same way as "git describe
    --abbrev=0" (show no hexdigits).  Fixing this bug may possibly
    be a low-hanging fruit.

But even if the feature cannot be used to guarantee a full
reproducibility, it is a good thing that we can now add this feature
with minimum effort thanks to the previous two steps.

The refactoring I suggested in my review for the previous step will
shine, if we want to do a good job parsing the --abbrev=<n> option,
since such a code organization would make it a fairly easy addition
to introduce "integer" type that calls match_placeholder_arg_value()
to read the option value (like "string" does) and validate that the
value is indeed an integer.

Would we want to support "--contains" as another boolean type?  How
about "--all" and "--long"?  All three sound plausible candidates.

Thanks.