{"thread":{"id":"57012","subject":"Bug Report","startedAt":"2021-12-01T22:31:35Z","lastAt":"2021-12-02T05:37:56Z","messageCount":4,"participants":["Josh Rampersad","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"442821","messageId":"C29F5770-5092-40A6-9604-DA5F7166575D@voiceflow.com","threadId":"57012","inReplyTo":null,"subject":"Bug Report","fromName":"Josh Rampersad","fromEmail":"josh.rampersad@voiceflow.com","sentAt":"2021-12-01T22:31:03Z","receivedAt":"2021-12-01T22:31:35Z","isPatch":false,"sender":{"key":"josh.rampersad@voiceflow.com","avatar":null},"body":"\nThank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\nNavigated to a non-default branch of the repo.\nI wanted a list of tagged commits for a specific tag pattern relating to a\npackage in my repo.\nRan the command: `git log origin/master --no-walk --grep='my-package' --tags='*my-package*' --decorate-refs='*my-package*'  --format='format:%ct %H %D'`\nThe output was as expected with the tags not relating to my-package being filtered out from the output by the decorate-refs option.\nI then, wanted to pipe this output to a separate program.\n\n\nWhat did you expect to happen? (Expected behavior)\n\nI expected that the piped output would be the same as the output in my terminal.\n\nWhat happened instead? (Actual behavior)\n\nThe filtering I got from the decorate-refs flag was no longer being applied. Thus giving me a bunch of tags I did not want\n\nWhat's different between what you expected and what actually happened?\n\nWhether the decorate-refs option worked as expected.\n\nAnything else you want to add:\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.31.1\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Darwin 20.6.0 Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:27 PDT\n2021; root:xnu-7195.141.2~5/RELEASE_ARM64_T8101 x86_64\ncompiler info: clang: 12.0.0 (clang-1200.0.32.29)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/zsh\n\n\n[Enabled Hooks]"},{"id":"442849","messageId":"YahaA/ve7W5Y83Iu@coredump.intra.peff.net","threadId":"57012","inReplyTo":"C29F5770-5092-40A6-9604-DA5F7166575D@voiceflow.com","subject":"[PATCH 0/2] log: handle --decorate-ref without explicit --decorate","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-02T05:30:43Z","receivedAt":"2021-12-02T05:30:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 01, 2021 at 05:31:03PM -0500, Josh Rampersad wrote:\n\n> Ran the command: `git log origin/master --no-walk --grep='my-package' --tags='*my-package*' --decorate-refs='*my-package*'  --format='format:%ct %H %D'`\n> The output was as expected with the tags not relating to my-package being filtered out from the output by the decorate-refs option.\n> I then, wanted to pipe this output to a separate program.\n> [...]\n> The filtering I got from the decorate-refs flag was no longer being applied. Thus giving me a bunch of tags I did not want\n\nThanks for a clear description. As a quick reproduction in git.git, you\ncan see this with:\n\n  $ git log --no-walk --format='%s%d' --decorate-refs=*/v2.0.0 v2.1.0 v2.0.0\n  Git 2.1\n  Git 2.0 (tag: v2.0.0)\n\n  $ git log --no-walk --format='%s%d' --decorate-refs=*/v2.0.0 v2.1.0 v2.0.0 | cat\n  Git 2.1 (tag: v2.1.0)\n  Git 2.0 (tag: v2.0.0)\n\nThe problem is that without an explicit --decorate, git-log doesn't\nrealize it needs to initialize the decorations with the --decorate-refs\nargument. Later code realizes that \"%d\" requires decorations, so we load\nthem then, but fail to pay attention to the extra arguments.\n\nThe reason it's different with a pipe is that the default is\n--decorate=auto, so it behaves as if --decorate is given depending on\nwhether stdout is a terminal.\n\nHere are two patches. The first fixes the bug you saw, and the second is\na similar bug with --simplify-by-decoration. In the meantime, a\nworkaround is to specify --decorate along with --decorate-refs.\n\n  [1/2]: log: handle --decorate-refs with userformat \"%d\"\n  [2/2]: log: load decorations with --simplify-by-decoration\n\n builtin/log.c  | 23 +++++++++++++++++++----\n t/t4202-log.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 56 insertions(+), 4 deletions(-)\n\n-Peff\n"},{"id":"442850","messageId":"YahbLwYiBWUaCotS@coredump.intra.peff.net","threadId":"57012","inReplyTo":"YahaA/ve7W5Y83Iu@coredump.intra.peff.net","subject":"[PATCH 1/2] log: handle --decorate-refs with userformat \"%d\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-02T05:35:43Z","receivedAt":"2021-12-02T05:35:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In order to show ref decorations, we first have to load them. If you\nrun:\n\n  git log --decorate\n\nthen git-log will recognize the option and load them up front via\ncmd_log_init(). Likewise if log.decorate is set.\n\nIf you don't say --decorate explicitly, but do mention \"%d\" or \"%D\" in\nthe output format, like so:\n\n  git log --format=%d\n\nthen this also works, because we lazy-load the ref decorations. This has\nbeen true since 3b3d443feb (add '%d' pretty format specifier to show\ndecoration, 2008-09-04), though the lazy-load was later moved into\nlog-tree.c.\n\nBut there's one problem: that lazy-load just uses the defaults; it\ndoesn't take into account any --decorate-refs options (or its exclude\nvariant, or their config). So this does not work:\n\n  git log --decorate-refs=whatever --format=%d\n\nIt will decorate using all refs, not just the specified ones. This has\nbeen true since --decorate-refs was added in 65516f586b (log: add option\nto choose which refs to decorate, 2017-11-21). Adding further confusion\nis that it _may_ work because of the auto-decoration feature. If that's\nin use (and it often is, as it's the default), then if the output is\ngoing to stdout, we do enable decorations early (and so load them up\nfront, respecting the extra options). But otherwise we do not. So:\n\n  git log --decorate-refs=whatever --format=%d >some-file\n\nwould typically behave differently than it does when the output goes to\nthe pager or terminal!\n\nThe solution is simple: we should recognize in cmd_log_init() that we're\ngoing to show decorations, and make sure we load them there. We already\ncheck userformat_find_requirements(), so we can couple this with our\nexisting code there.\n\nThere are two new tests. The first shows off the actual fix. The second\nmakes sure that our fix doesn't cause us to stomp on an existing\n--decorate option (see the new comment in the code, as well).\n\nReported-by: Josh Rampersad <josh.rampersad@voiceflow.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/log.c  | 18 ++++++++++++++++--\n t/t4202-log.sh | 22 ++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f75d87e8d7..a924f56299 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -245,8 +245,22 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\trev->abbrev_commit = 0;\n \t}\n \n-\tif (rev->commit_format == CMIT_FMT_USERFORMAT && !w.decorate)\n-\t\tdecoration_style = 0;\n+\tif (rev->commit_format == CMIT_FMT_USERFORMAT) {\n+\t\tif (!w.decorate) {\n+\t\t\t/*\n+\t\t\t * Disable decoration loading if the format will not\n+\t\t\t * show them anyway.\n+\t\t\t */\n+\t\t\tdecoration_style = 0;\n+\t\t} else if (!decoration_style) {\n+\t\t\t/*\n+\t\t\t * If we are going to show them, make sure we do load\n+\t\t\t * them here, but taking care not to override a\n+\t\t\t * specific style set by config or --decorate.\n+\t\t\t */\n+\t\t\tdecoration_style = DECORATE_SHORT_REFS;\n+\t\t}\n+\t}\n \n \tif (decoration_style) {\n \t\tconst struct string_list *config_exclude =\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 7884e3d46b..5f0ae7a785 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -952,6 +952,28 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '\n \ttest_cmp expect.decorate actual\n '\n \n+test_expect_success 'decorate-refs with implied decorate from format' '\n+\tcat >expect <<-\\EOF &&\n+\tside-2 (tag: side-2)\n+\tside-1\n+\tEOF\n+\tgit log --no-walk --format=\"%s%d\" \\\n+\t\t--decorate-refs=\"*side-2\" side-1 side-2 \\\n+\t\t>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'implied decorate does not override option' '\n+\tcat >expect <<-\\EOF &&\n+\tside-2 (tag: refs/tags/side-2, refs/heads/side)\n+\tside-1 (tag: refs/tags/side-1)\n+\tEOF\n+\tgit log --no-walk --format=\"%s%d\" \\\n+\t\t--decorate=full side-1 side-2 \\\n+\t\t>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'log.decorate config parsing' '\n \tgit log --oneline --decorate=full >expect.full &&\n \tgit log --oneline --decorate=short >expect.short &&\n-- \n2.34.1.328.g8a543840e4\n\n"},{"id":"442851","messageId":"YahbseXEC66dK0k8@coredump.intra.peff.net","threadId":"57012","inReplyTo":"YahaA/ve7W5Y83Iu@coredump.intra.peff.net","subject":"[PATCH 2/2] log: load decorations with --simplify-by-decoration","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-02T05:37:53Z","receivedAt":"2021-12-02T05:37:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It's possible to specify --simplify-by-decoration but not --decorate. In\nthis case we do respect the simplification, but we don't actually show\nany decorations. However, it works by lazy-loading the decorations when\nneeded; this is discussed in more detail in 0cc7380d88 (log-tree: call\nload_ref_decorations() in get_name_decoration(), 2019-09-08).\n\nThis works for basic cases, but will fail to respect any --decorate-refs\noption (or its variants). Those are handled only when cmd_log_init()\nloads the ref decorations up front, which is only when --decorate is\nspecified explicitly (or as of the previous commit, when the userformat\nasks for %d or similar).\n\nWe can solve this by making sure to load the decorations if we're going\nto simplify using them but they're not otherwise going to be displayed.\n\nThe new test shows a simple case that fails without this patch. Note\nthat we expect two commits in the output: the one we asked for by\n--decorate-refs, and the initial commit. The latter is just a quirk of\nhow --simplify-by-decoration works. Arguably it may be a bug, but it's\nunrelated to this patch (which is just about the loading of the\ndecorations; you get the same behavior before this patch with an\nexplicit --decorate).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/log.c  |  5 +++--\n t/t4202-log.sh | 15 +++++++++++++++\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex a924f56299..93ace0dde7 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -262,7 +262,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t}\n \t}\n \n-\tif (decoration_style) {\n+\tif (decoration_style || rev->simplify_by_decoration) {\n \t\tconst struct string_list *config_exclude =\n \t\t\trepo_config_get_value_multi(the_repository,\n \t\t\t\t\t\t    \"log.excludeDecoration\");\n@@ -274,7 +274,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t\t\t\t   item->string);\n \t\t}\n \n-\t\trev->show_decorations = 1;\n+\t\tif (decoration_style)\n+\t\t\trev->show_decorations = 1;\n \n \t\tload_ref_decorations(&decoration_filter, decoration_style);\n \t}\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 5f0ae7a785..7a725b9a23 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -974,6 +974,21 @@ test_expect_success 'implied decorate does not override option' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'decorate-refs and simplify-by-decoration without output' '\n+\tcat >expect <<-\\EOF &&\n+\tside-2\n+\tinitial\n+\tEOF\n+\t# Do not just use a --format without %d here; we want to\n+\t# make sure that we did not accidentally turn on displaying\n+\t# the decorations, too. And that requires one of the regular\n+\t# formats.\n+\tgit log --decorate-refs=\"*side-2\" --oneline \\\n+\t\t--simplify-by-decoration >actual.raw &&\n+\tsed \"s/^[0-9a-f]* //\" <actual.raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'log.decorate config parsing' '\n \tgit log --oneline --decorate=full >expect.full &&\n \tgit log --oneline --decorate=short >expect.short &&\n-- \n2.34.1.328.g8a543840e4\n"}]}