git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Đoàn Trần Công Danh <congdanhqx@gmail.com>
Date
Oct 26, 2021, 12:06 UTC
Message-ID
<YXfvY3n9wEwctjUR@danh.dev>
In-Reply-To
<20211026013452.1372122-4-eschwartz@archlinux.org>
On 2021-10-25 21:34:52-0400, Eli Schwartz <eschwartz@archlinux.org> wrote:
Show 75 quoted lines
> 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>
> ---
> 
> Notes:
>     With regard to validating that an integer is passed, I attempt to parse the
>     result using the same mechanism git-describe itself does in the abbrev
>     callback, just with slightly different validation of what we have at the end...
>     because of course here argval is the entire rest of the format string,
>     including the ")".
>     
>     While testing that this actually does what it's supposed to do, I noticed that
>     it doesn't validate junk like leading whitespace or plus signs... this is a
>     problem for `git describe --abbrev='    +15'` too so I guess it's not my
>     problem to fix...
> 
>  Documentation/pretty-formats.txt |  4 ++++
>  pretty.c                         | 16 +++++++++++++++-
>  t/t4205-log-pretty-formats.sh    |  8 ++++++++
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> 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 16b5366fed..44bfc49b38 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 },
>  	};
> @@ -1245,6 +1246,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) && arglen) {
> +					if (!arglen)
> +						return 0;
> +					char* endptr;
Other than the question pointed out by Eric,

with DEVELOPER=1, -Werror=declaration-after-statement We'll need this change squashed in:

------- 8< -----
diff --git a/pretty.c b/pretty.c
index 289b5456c8..85d4ab008b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1249,9 +1249,9 @@ static size_t parse_describe_args(const char *start, struct strvec *args)
 			case OPT_INTEGER:
 				if (match_placeholder_arg_value(arg, option[i].name, &arg,
 								&argval, &arglen) && arglen) {
+					char* endptr;
 					if (!arglen)
 						return 0;
-					char* endptr;
 					strtol(argval, &endptr, 10);
 					if (endptr - argval != arglen)
 						return 0;
------- >8 -----

> +					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) && 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
> -- 
> 2.33.1
> 
-- 
Danh
Previous: Eric SunshineNext: Eric Sunshine
Message 20 of 46 in “Add some more options to the pretty-formats”
  1. 0/3 Add some more options to the pretty-formatsEli Schwartz, Oct 24, 2021
  2. 1/3 pretty.c: rename describe options variable to more descriptive nameEli Schwartz, Oct 24, 2021
  3. Junio C HamanoOct 24, 2021
  4. Eli SchwartzOct 24, 2021
  5. 3/3 pretty: add abbrev option to %(describe)Eli Schwartz, Oct 24, 2021
  6. Junio C HamanoOct 24, 2021
  7. Eli SchwartzOct 24, 2021
  8. 2/3 pretty: add tag option to %(describe)Eli Schwartz, Oct 24, 2021
  9. Junio C HamanoOct 24, 2021
  10. Eli SchwartzOct 24, 2021
  11. 0/3 Add some more options to the pretty-formatsEli Schwartz, Oct 26, 2021
  12. 1/3 pretty.c: rework describe options parsing for better extensibilityEli Schwartz, Oct 26, 2021
  13. Eric SunshineOct 26, 2021
  14. Eli SchwartzOct 26, 2021
  15. 2/3 pretty: add tag option to %(describe)Eli Schwartz, Oct 26, 2021
  16. Eric SunshineOct 26, 2021
  17. Eli SchwartzOct 26, 2021
  18. 3/3 pretty: add abbrev option to %(describe)Eli Schwartz, Oct 26, 2021
  19. Eric SunshineOct 26, 2021
  20. Đoàn Trần Công DanhOct 26, 2021
  21. Eric SunshineOct 26, 2021
  22. Eli SchwartzOct 26, 2021
  23. Carlo ArenasOct 27, 2021
  24. Johannes SchindelinNov 3, 2021
  25. Johannes SchindelinNov 4, 2021
  26. Eli SchwartzNov 7, 2021
  27. 0/3 Add some more options to the pretty-formatsEli Schwartz, Oct 29, 2021
  28. 2/3 pretty: add tag option to %(describe)Eli Schwartz, Oct 29, 2021
  29. Junio C HamanoOct 29, 2021
  30. Eli SchwartzOct 29, 2021
  31. Junio C HamanoOct 29, 2021
  32. Junio C HamanoOct 29, 2021
  33. Eli SchwartzOct 29, 2021
  34. 3/3 pretty: add abbrev option to %(describe)Eli Schwartz, Oct 29, 2021
  35. Eric SunshineOct 29, 2021
  36. Eli SchwartzOct 29, 2021
  37. 1/3 pretty.c: rework describe options parsing for better extensibilityEli Schwartz, Oct 29, 2021
  38. Junio C HamanoOct 29, 2021
  39. Eli SchwartzOct 29, 2021
  40. Junio C HamanoOct 29, 2021
  41. 0/3 Add some more options to the pretty-formatsEli Schwartz, Oct 31, 2021
  42. 1/3 pretty.c: rework describe options parsing for better extensibilityEli Schwartz, Oct 31, 2021
  43. 2/3 pretty: add tag option to %(describe)Eli Schwartz, Oct 31, 2021
  44. Junio C HamanoOct 31, 2021
  45. Eli SchwartzOct 31, 2021
  46. 3/3 pretty: add abbrev option to %(describe)Eli Schwartz, Oct 31, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.