From 15ae82d5d6ccdcad00aeb456ff512d3031f03039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 14 Feb 2021 11:04:34 +0100 Subject: [PATCH 1/5] pretty: add %(describe) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a format placeholder for describe output. Implement it by actually calling git describe, which is simple and guarantees correctness. It's intended to be used with $Format:...$ in files with the attribute export-subst and git archive. It can also be used with git log etc., even though that's going to be slow due to the fork for each commit. Suggested-by: Eli Schwartz Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/pretty-formats.txt | 2 ++ pretty.c | 17 +++++++++++++++++ t/t4205-log-pretty-formats.sh | 10 ++++++++++ 3 files changed, 29 insertions(+) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 6b59e28d44..bb8c05bc21 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -208,6 +208,8 @@ The placeholders are: '%cs':: committer date, short format (`YYYY-MM-DD`) '%d':: ref names, like the --decorate option of linkgit:git-log[1] '%D':: ref names without the " (", ")" wrapping. +'%(describe)':: human-readable name, like linkgit:git-describe[1]; + empty string for undescribable commits '%S':: ref name given on the command line by which the commit was reached (like `git log --source`), only works with `git log` '%e':: encoding diff --git a/pretty.c b/pretty.c index b4ff3f602f..a0c427fb61 100644 --- a/pretty.c +++ b/pretty.c @@ -12,6 +12,7 @@ #include "reflog-walk.h" #include "gpg-interface.h" #include "trailer.h" +#include "run-command.h" static char *user_format; static struct cmt_fmt_map { @@ -1214,6 +1215,22 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return parse_padding_placeholder(placeholder, c); } + if (skip_prefix(placeholder, "(describe)", &arg)) { + struct child_process cmd = CHILD_PROCESS_INIT; + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + + cmd.git_cmd = 1; + strvec_push(&cmd.args, "describe"); + strvec_push(&cmd.args, oid_to_hex(&commit->object.oid)); + pipe_command(&cmd, NULL, 0, &out, 0, &err, 0); + strbuf_rtrim(&out); + strbuf_addbuf(sb, &out); + strbuf_release(&out); + strbuf_release(&err); + return arg - placeholder; + } + /* these depend on the commit */ if (!commit->object.parsed) parse_object(the_repository, &commit->object.oid); diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 749bc1431a..5a44fa447d 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -962,4 +962,14 @@ test_expect_success 'log --pretty=reference is colored appropriately' ' test_cmp expect actual ' +test_expect_success '%(describe) vs git describe' ' + git log --format="%H" | while read hash + do + echo "$hash $(git describe $hash)" + done >expect && + git log --format="%H %(describe)" >actual 2>err && + test_cmp expect actual && + test_must_be_empty err +' + test_done From b081547ec1de57162f2c1c7748aa09c861413d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 14 Feb 2021 11:10:57 +0100 Subject: [PATCH 2/5] pretty: add merge and exclude options to %(describe) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow restricting the tags used by the placeholder %(describe) with the options match and exclude. E.g. the following command describes the current commit using official version tags, without those for release candidates: $ git log -1 --format='%(describe:match=v[0-9]*,exclude=*rc*)' Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/pretty-formats.txt | 13 ++++++++-- pretty.c | 43 ++++++++++++++++++++++++++++++-- t/t4205-log-pretty-formats.sh | 16 ++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index bb8c05bc21..231010e6ef 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -208,8 +208,17 @@ The placeholders are: '%cs':: committer date, short format (`YYYY-MM-DD`) '%d':: ref names, like the --decorate option of linkgit:git-log[1] '%D':: ref names without the " (", ")" wrapping. -'%(describe)':: human-readable name, like linkgit:git-describe[1]; - empty string for undescribable commits +'%(describe[:options])':: human-readable name, like + linkgit:git-describe[1]; empty string for + undescribable commits. The `describe` string + may be followed by a colon and zero or more + comma-separated options. ++ +** 'match=': Only consider tags matching the given + `glob(7)` pattern, excluding the "refs/tags/" prefix. +** 'exclude=': Do not consider tags matching the given + `glob(7)` pattern, excluding the "refs/tags/" prefix. + '%S':: ref name given on the command line by which the commit was reached (like `git log --source`), only works with `git log` '%e':: encoding diff --git a/pretty.c b/pretty.c index a0c427fb61..c612d2ac9b 100644 --- a/pretty.c +++ b/pretty.c @@ -1150,6 +1150,34 @@ static int format_trailer_match_cb(const struct strbuf *key, void *ud) return 0; } +static size_t parse_describe_args(const char *start, struct strvec *args) +{ + const char *options[] = { "match", "exclude" }; + const char *arg = start; + + for (;;) { + const char *matched = NULL; + const char *argval; + size_t arglen = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(options); i++) { + if (match_placeholder_arg_value(arg, options[i], &arg, + &argval, &arglen)) { + matched = options[i]; + break; + } + } + if (!matched) + break; + + if (!arglen) + return 0; + strvec_pushf(args, "--%s=%.*s", matched, (int)arglen, argval); + } + return arg - start; +} + static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ const char *placeholder, void *context) @@ -1215,20 +1243,31 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return parse_padding_placeholder(placeholder, c); } - if (skip_prefix(placeholder, "(describe)", &arg)) { + if (skip_prefix(placeholder, "(describe", &arg)) { struct child_process cmd = CHILD_PROCESS_INIT; struct strbuf out = STRBUF_INIT; struct strbuf err = STRBUF_INIT; cmd.git_cmd = 1; strvec_push(&cmd.args, "describe"); + + if (*arg == ':') { + arg++; + arg += parse_describe_args(arg, &cmd.args); + } + + if (*arg != ')') { + child_process_clear(&cmd); + return 0; + } + strvec_push(&cmd.args, oid_to_hex(&commit->object.oid)); pipe_command(&cmd, NULL, 0, &out, 0, &err, 0); strbuf_rtrim(&out); strbuf_addbuf(sb, &out); strbuf_release(&out); strbuf_release(&err); - return arg - placeholder; + return arg - placeholder + 1; } /* these depend on the commit */ diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 5a44fa447d..7e36706212 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -972,4 +972,20 @@ test_expect_success '%(describe) vs git describe' ' test_must_be_empty err ' +test_expect_success '%(describe:match=...) vs git describe --match ...' ' + test_when_finished "git tag -d tag-match" && + git tag -a -m tagged tag-match&& + git describe --match "*-match" >expect && + git log -1 --format="%(describe:match=*-match)" >actual && + test_cmp expect actual +' + +test_expect_success '%(describe:exclude=...) vs git describe --exclude ...' ' + test_when_finished "git tag -d tag-exclude" && + git tag -a -m tagged tag-exclude && + git describe --exclude "*-exclude" >expect && + git log -1 --format="%(describe:exclude=*-exclude)" >actual && + test_cmp expect actual +' + test_done From 09fe8ca92e4f0269bd7914b5010731a09948abf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 28 Feb 2021 12:22:26 +0100 Subject: [PATCH 3/5] t4205: assert %(describe) test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document that the test is covering both describable and undescribable commits. Suggested-by: Ævar Arnfjörð Bjarmason Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- t/t4205-log-pretty-formats.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 7e36706212..3b098913ed 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -965,8 +965,17 @@ test_expect_success 'log --pretty=reference is colored appropriately' ' test_expect_success '%(describe) vs git describe' ' git log --format="%H" | while read hash do - echo "$hash $(git describe $hash)" + if desc=$(git describe $hash) + then + : >expect-contains-good + else + : >expect-contains-bad + fi && + echo "$hash $desc" done >expect && + test_path_exists expect-contains-good && + test_path_exists expect-contains-bad && + git log --format="%H %(describe)" >actual 2>err && test_cmp expect actual && test_must_be_empty err From 273c9901c2ebfb2dae0c52de912bf3f57be19aac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 28 Feb 2021 12:22:34 +0100 Subject: [PATCH 4/5] pretty: document multiple %(describe) being inconsistent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each %(describe) placeholder is expanded using a separate git describe call. Their outputs depend on the tags present at the time, so there's no consistency guarantee. Document that fact. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/pretty-formats.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 231010e6ef..45133066e4 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -212,7 +212,9 @@ The placeholders are: linkgit:git-describe[1]; empty string for undescribable commits. The `describe` string may be followed by a colon and zero or more - comma-separated options. + comma-separated options. Descriptions can be + inconsistent when tags are added or removed at + the same time. + ** 'match=': Only consider tags matching the given `glob(7)` pattern, excluding the "refs/tags/" prefix. From 96099726ddb00b45135964220ce56468ba9fe184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 28 Feb 2021 12:22:47 +0100 Subject: [PATCH 5/5] archive: expand only a single %(describe) per archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every %(describe) placeholder in $Format:...$ strings in files with the attribute export-subst is expanded by calling git describe. This can potentially result in a lot of such calls per archive. That's OK for local repositories under control of the user of git archive, but could be a problem for hosted repositories. Expand only a single %(describe) placeholder per archive for now to avoid denial-of-service attacks. We can make this limit configurable later if needed, but let's start out simple. Reported-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/gitattributes.txt | 3 ++- archive.c | 16 ++++++++++------ archive.h | 2 ++ pretty.c | 8 ++++++++ pretty.h | 5 +++++ t/t5001-archive-attr.sh | 14 ++++++++++++++ 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index e84e104f93..0a60472bb5 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -1174,7 +1174,8 @@ tag then no replacement will be done. The placeholders are the same as those for the option `--pretty=format:` of linkgit:git-log[1], except that they need to be wrapped like this: `$Format:PLACEHOLDERS$` in the file. E.g. the string `$Format:%H$` will be replaced by the -commit hash. +commit hash. However, only one `%(describe)` placeholder is expanded +per archive to avoid denial-of-service attacks. Packing objects diff --git a/archive.c b/archive.c index 5919d9e505..2dd2236ae0 100644 --- a/archive.c +++ b/archive.c @@ -37,13 +37,10 @@ void init_archivers(void) static void format_subst(const struct commit *commit, const char *src, size_t len, - struct strbuf *buf) + struct strbuf *buf, struct pretty_print_context *ctx) { char *to_free = NULL; struct strbuf fmt = STRBUF_INIT; - struct pretty_print_context ctx = {0}; - ctx.date_mode.type = DATE_NORMAL; - ctx.abbrev = DEFAULT_ABBREV; if (src == buf->buf) to_free = strbuf_detach(buf, NULL); @@ -61,7 +58,7 @@ static void format_subst(const struct commit *commit, strbuf_add(&fmt, b + 8, c - b - 8); strbuf_add(buf, src, b - src); - format_commit_message(commit, fmt.buf, buf, &ctx); + format_commit_message(commit, fmt.buf, buf, ctx); len -= c + 1 - src; src = c + 1; } @@ -94,7 +91,7 @@ static void *object_file_to_archive(const struct archiver_args *args, strbuf_attach(&buf, buffer, *sizep, *sizep + 1); convert_to_working_tree(args->repo->index, path, buf.buf, buf.len, &buf, &meta); if (commit) - format_subst(commit, buf.buf, buf.len, &buf); + format_subst(commit, buf.buf, buf.len, &buf, args->pretty_ctx); buffer = strbuf_detach(&buf, &size); *sizep = size; } @@ -633,12 +630,19 @@ int write_archive(int argc, const char **argv, const char *prefix, const char *name_hint, int remote) { const struct archiver *ar = NULL; + struct pretty_print_describe_status describe_status = {0}; + struct pretty_print_context ctx = {0}; struct archiver_args args; int rc; git_config_get_bool("uploadarchive.allowunreachable", &remote_allow_unreachable); git_config(git_default_config, NULL); + describe_status.max_invocations = 1; + ctx.date_mode.type = DATE_NORMAL; + ctx.abbrev = DEFAULT_ABBREV; + ctx.describe_status = &describe_status; + args.pretty_ctx = &ctx; args.repo = repo; args.prefix = prefix; string_list_init(&args.extra_files, 1); diff --git a/archive.h b/archive.h index 33551b7ee1..49fab71aaf 100644 --- a/archive.h +++ b/archive.h @@ -5,6 +5,7 @@ #include "pathspec.h" struct repository; +struct pretty_print_context; struct archiver_args { struct repository *repo; @@ -22,6 +23,7 @@ struct archiver_args { unsigned int convert : 1; int compression_level; struct string_list extra_files; + struct pretty_print_context *pretty_ctx; }; /* main api */ diff --git a/pretty.c b/pretty.c index c612d2ac9b..032e89cd4e 100644 --- a/pretty.c +++ b/pretty.c @@ -1247,6 +1247,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ struct child_process cmd = CHILD_PROCESS_INIT; struct strbuf out = STRBUF_INIT; struct strbuf err = STRBUF_INIT; + struct pretty_print_describe_status *describe_status; + + describe_status = c->pretty_ctx->describe_status; + if (describe_status) { + if (!describe_status->max_invocations) + return 0; + describe_status->max_invocations--; + } cmd.git_cmd = 1; strvec_push(&cmd.args, "describe"); diff --git a/pretty.h b/pretty.h index 7ce6c0b437..27b15947ff 100644 --- a/pretty.h +++ b/pretty.h @@ -23,6 +23,10 @@ enum cmit_fmt { CMIT_FMT_UNSPECIFIED }; +struct pretty_print_describe_status { + unsigned int max_invocations; +}; + struct pretty_print_context { /* * Callers should tweak these to change the behavior of pp_* functions. @@ -44,6 +48,7 @@ struct pretty_print_context { int color; struct ident_split *from_ident; unsigned encode_email_headers:1; + struct pretty_print_describe_status *describe_status; /* * Fields below here are manipulated internally by pp_* functions and diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh index e9aa97117a..712ae52299 100755 --- a/t/t5001-archive-attr.sh +++ b/t/t5001-archive-attr.sh @@ -128,4 +128,18 @@ test_expect_success 'export-subst' ' test_cmp substfile2 archive/substfile2 ' +test_expect_success 'export-subst expands %(describe) once' ' + echo "\$Format:%(describe)\$" >substfile3 && + echo "\$Format:%(describe)\$" >>substfile3 && + echo "\$Format:%(describe)${LF}%(describe)\$" >substfile4 && + git add substfile[34] && + git commit -m export-subst-describe && + git tag -m export-subst-describe export-subst-describe && + git archive HEAD >archive-describe.tar && + extract_tar_to_dir archive-describe && + desc=$(git describe) && + grep -F "$desc" archive-describe/substfile[34] >substituted && + test_line_count = 1 substituted +' + test_done