diff mbox series

[v3,3/3] pretty: add abbrev option to %(describe)

Message ID 20211029184512.1568017-4-eschwartz@archlinux.org (mailing list archive)
State Superseded
Headers show
Series Add some more options to the pretty-formats | expand

Commit Message

Eli Schwartz Oct. 29, 2021, 6:45 p.m. UTC
The %(describe) placeholder by default, like `git describe`, uses a
seven-character abbreviated commit object name. This may not be
sufficient to fully describe all commits in a given repository,
resulting in a placeholder replacement 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.

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

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---
 Documentation/pretty-formats.txt |  4 ++++
 pretty.c                         | 16 +++++++++++++++-
 t/t4205-log-pretty-formats.sh    |  8 ++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

Comments

Eric Sunshine Oct. 29, 2021, 6:51 p.m. UTC | #1
On Fri, Oct 29, 2021 at 2:45 PM Eli Schwartz <eschwartz@archlinux.org> wrote:
> The %(describe) placeholder by default, like `git describe`, uses a
> seven-character abbreviated commit object name. This may not be
> sufficient to fully describe all commits in a given repository,
> resulting in a placeholder replacement 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.
> [...]
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> @@ -222,6 +222,10 @@ The placeholders are:
> +** 'abbrev=<N>': Instead of using the default number of hexadecimal digits
> +   (which will vary according to the number of objects in the repository with a
> +   default of 7) of the abbreviated object name, use <n> digits, or as many digits
> +   as needed to form a unique object name.

There's still an inconsistent mix of `<N>` and `<n>` here (mentioned
in my earlier review). Is that intentional or just a simple oversight?
Eli Schwartz Oct. 29, 2021, 7:04 p.m. UTC | #2
On 10/29/21 2:51 PM, Eric Sunshine wrote:
> On Fri, Oct 29, 2021 at 2:45 PM Eli Schwartz <eschwartz@archlinux.org> wrote:
>> The %(describe) placeholder by default, like `git describe`, uses a
>> seven-character abbreviated commit object name. This may not be
>> sufficient to fully describe all commits in a given repository,
>> resulting in a placeholder replacement 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.
>> [...]
>> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
>> ---
>> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
>> @@ -222,6 +222,10 @@ The placeholders are:
>> +** 'abbrev=<N>': Instead of using the default number of hexadecimal digits
>> +   (which will vary according to the number of objects in the repository with a
>> +   default of 7) of the abbreviated object name, use <n> digits, or as many digits
>> +   as needed to form a unique object name.
> 
> There's still an inconsistent mix of `<N>` and `<n>` here (mentioned
> in my earlier review). Is that intentional or just a simple oversight?


Ah, sorry... I overlooked that. It was originally copied from the
git-describe man page which uses lowercase and I overlooked that part of
your review.

It should be consistently uppercase here for consistency with
pretty-formats.
diff mbox series

Patch

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 86ed801aad..57fd84f579 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -222,6 +222,10 @@  The placeholders are:
 +
 ** 'tags[=<BOOL>]': Instead of only considering annotated tags,
    consider lightweight tags as well.
+** 'abbrev=<N>': Instead of using the default number of hexadecimal digits
+   (which will vary according to the number of objects in the repository with a
+   default of 7) of the abbreviated object name, use <n> digits, or as many digits
+   as needed to form a unique object name.
 ** 'match=<pattern>': Only consider tags matching the given
    `glob(7)` pattern, excluding the "refs/tags/" prefix.
 ** 'exclude=<pattern>': Do not consider tags matching the given
diff --git a/pretty.c b/pretty.c
index a105ef2a15..5662cb2943 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1218,9 +1218,10 @@  static size_t parse_describe_args(const char *start, struct strvec *args)
 {
 	struct {
 		char *name;
-		enum { OPT_BOOL, OPT_STRING, } type;
+		enum { OPT_BOOL, OPT_INTEGER, OPT_STRING, } type;
 	}  option[] = {
 		{ "tags", OPT_BOOL},
+		{ "abbrev", OPT_INTEGER },
 		{ "exclude", OPT_STRING },
 		{ "match", OPT_STRING },
 	};
@@ -1244,6 +1245,19 @@  static size_t parse_describe_args(const char *start, struct strvec *args)
 					found = 1;
 				}
 				break;
+			case OPT_INTEGER:
+				if (match_placeholder_arg_value(arg, option[i].name, &arg,
+								&argval, &arglen)) {
+					char *endptr;
+					if (!arglen)
+						return 0;
+					strtol(argval, &endptr, 10);
+					if (endptr - argval != arglen)
+						return 0;
+					strvec_pushf(args, "--%s=%.*s", option[i].name, (int)arglen, argval);
+					found = 1;
+				}
+				break;
 			case OPT_STRING:
 				if (match_placeholder_arg_value(arg, option[i].name, &arg,
 								&argval, &arglen)) {
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index d4acf8882f..35eef4c865 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1010,4 +1010,12 @@  test_expect_success '%(describe:tags) vs git describe --tags' '
 	test_cmp expect actual
 '
 
+test_expect_success '%(describe:abbrev=...) vs git describe --abbrev=...' '
+	test_when_finished "git tag -d tagname" &&
+	git tag -a -m tagged tagname &&
+	git describe --abbrev=15 >expect &&
+	git log -1 --format="%(describe:abbrev=15)" >actual &&
+	test_cmp expect actual
+'
+
 test_done