{"thread":{"id":"42650","subject":"[PATCH v2 0/2] Introduce log.showSignature config variable","startedAt":"2016-06-18T12:25:31Z","lastAt":"2016-06-19T11:31:01Z","messageCount":5,"participants":["Mehul Jain","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"289571","messageId":"20160618122510.5105-1-mehul.jain2029@gmail.com","threadId":"42650","inReplyTo":null,"subject":"[PATCH v2 0/2] Introduce log.showSignature config variable","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-06-18T12:25:08Z","receivedAt":"2016-06-18T12:25:31Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Add a new configuratation variable \"log.showSignature\" for git-log and\nrelated commands. \"log.showSignature=true\" will enable user to see GPG signature\nby default for git-log and related commands.\n\nChanges:\n\t* Order of patches is reversed as suggested by Junio[1].\n\t* A new test has been introduced for \"--no-show-signature\"\n\t  option.\n\nPrevious patch: http://thread.gmane.org/gmane.comp.version-control.git/296474\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/296474/focus=296476\n\nMehul Jain (2):\n  log: add \"--no-show-signature\" command line option\n  log: add log.showSignature configuration variable\n\n Documentation/git-log.txt |  4 ++++\n builtin/log.c             |  6 ++++++\n revision.c                |  2 ++\n t/t4202-log.sh            | 35 +++++++++++++++++++++++++++++++++++\n t/t7510-signed-commit.sh  |  7 +++++++\n 5 files changed, 54 insertions(+)\n\n-- \n2.9.0.rc0.dirty\n\n"},{"id":"289572","messageId":"20160618122510.5105-3-mehul.jain2029@gmail.com","threadId":"42650","inReplyTo":"20160618122510.5105-1-mehul.jain2029@gmail.com","subject":"[PATCH v2 2/2] log: add log.showSignature configuration variable","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-06-18T12:25:10Z","receivedAt":"2016-06-18T12:25:45Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Users may want to always use \"--show-signature\" while using git-log and\nrelated commands.\n\nWhen log.showSignature is set to true, git-log and related commands will\nbehave as if \"--show-signature\" was given to them.\n\nNote that this config variable is meant to affect git-log, git-show,\ngit-whatchanged and git-reflog. Other commands like git-format-patch,\ngit-rev-list are not to be affected by this config variable.\n\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n Documentation/git-log.txt |  4 ++++\n builtin/log.c             |  6 ++++++\n t/t4202-log.sh            | 25 +++++++++++++++++++++++++\n t/t7510-signed-commit.sh  |  7 +++++++\n 4 files changed, 42 insertions(+)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 03f9580..bbb5adc 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -196,6 +196,10 @@ log.showRoot::\n \t`git log -p` output would be shown without a diff attached.\n \tThe default is `true`.\n \n+log.showSignature::\n+\tIf `true`, `git log` and related commands will act as if the\n+\t`--show-signature` option was passed to them.\n+\n mailmap.*::\n \tSee linkgit:git-shortlog[1].\n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex 099f4f7..7103217 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -33,6 +33,7 @@ static const char *default_date_mode = NULL;\n static int default_abbrev_commit;\n static int default_show_root = 1;\n static int default_follow;\n+static int default_show_signature;\n static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config;\n@@ -119,6 +120,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n+\trev->show_signature = default_show_signature;\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n \n \tif (default_date_mode)\n@@ -409,6 +411,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tuse_mailmap_config = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"log.showsignature\")) {\n+\t\tdefault_show_signature = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 02384a3..63ed863 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -900,6 +900,31 @@ test_expect_success GPG '--no-show-signature overrides --show-signature' '\n \t! grep \"^gpg:\" actual\n '\n \n+test_expect_success GPG 'log.showsignature=true behaves like --show-signature' '\n+\tgit checkout -b test_sign master &&\n+\techo foo >foo &&\n+\tgit add foo &&\n+\tgit commit -S -m signed_commit &&\n+\ttest_config log.showsignature true &&\n+\tgit log -1 signed >actual &&\n+\tgrep \"gpg: Signature made\" actual &&\n+\tgrep \"gpg: Good signature\" actual\n+'\n+\n+test_expect_success GPG '--no-show-signature overrides log.showsignature=true' '\n+\ttest_config log.showsignature true &&\n+\tgit log -1 --no-show-signature signed >actual &&\n+\t! grep \"^gpg:\" actual\n+'\n+\n+test_expect_success GPG '--show-signature overrides log.showsignature=false' '\n+\ttest_when_finished \"git reset --hard && git checkout master\" &&\n+\ttest_config log.showsignature false &&\n+\tgit log -1 --show-signature signed >actual &&\n+\tgrep \"gpg: Signature made\" actual &&\n+\tgrep \"gpg: Good signature\" actual\n+'\n+\n test_expect_success 'log --graph --no-walk is forbidden' '\n \ttest_must_fail git log --graph --no-walk\n '\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 4177a86..6e839f5 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -210,4 +210,11 @@ test_expect_success GPG 'show lack of signature with custom format' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success GPG 'log.showsignature behaves like --show-signature' '\n+\ttest_config log.showsignature true &&\n+\tgit show initial >actual &&\n+\tgrep \"gpg: Signature made\" actual &&\n+\tgrep \"gpg: Good signature\" actual\n+'\n+\n test_done\n-- \n2.9.0.rc0.dirty\n\n"},{"id":"289573","messageId":"20160618122510.5105-2-mehul.jain2029@gmail.com","threadId":"42650","inReplyTo":"20160618122510.5105-1-mehul.jain2029@gmail.com","subject":"[PATCH v2 1/2] log: add \"--no-show-signature\" command line option","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-06-18T12:25:09Z","receivedAt":"2016-06-18T12:27:12Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"If an user creates an alias with \"--show-signature\" in command line,\ne.g.\n\t[alias] logss = log --show-signature\n\nthen there is no way to countermand it through command line.\n\nTeach git-log and related commands about \"--no-show-signature\" command\nline option. This will make \"git logss --no-show-signature\" run\nwithout showing GPG signature.\n\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n revision.c     |  2 ++\n t/t4202-log.sh | 10 ++++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex d30d1c4..3546ff9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1871,6 +1871,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->notes_opt.use_default_notes = 1;\n \t} else if (!strcmp(arg, \"--show-signature\")) {\n \t\trevs->show_signature = 1;\n+\t} else if (!strcmp(arg, \"--no-show-signature\")) {\n+\t\trevs->show_signature = 0;\n \t} else if (!strcmp(arg, \"--show-linear-break\") ||\n \t\t   starts_with(arg, \"--show-linear-break=\")) {\n \t\tif (starts_with(arg, \"--show-linear-break=\"))\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 128ba93..02384a3 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -890,6 +890,16 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '\n \tgrep \"^| | gpg: Good signature\" actual\n '\n \n+test_expect_success GPG '--no-show-signature overrides --show-signature' '\n+\ttest_when_finished \"git reset --hard && git checkout master\" &&\n+\tgit checkout -b nosign master &&\n+\techo foo >foo &&\n+\tgit add foo &&\n+\tgit commit -S -m signed_commit &&\n+\tgit log -1 --show-signature --no-show-signature nosign >actual &&\n+\t! grep \"^gpg:\" actual\n+'\n+\n test_expect_success 'log --graph --no-walk is forbidden' '\n \ttest_must_fail git log --graph --no-walk\n '\n-- \n2.9.0.rc0.dirty\n\n"},{"id":"289608","messageId":"CAPig+cRZyCZuec1GuxtW0p_8G1VPq1y846USM-TNmP3bbTYxQA@mail.gmail.com","threadId":"42650","inReplyTo":"20160618122510.5105-3-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 2/2] log: add log.showSignature configuration variable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-06-19T06:59:40Z","receivedAt":"2016-06-19T06:59:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 18, 2016 at 8:25 AM, Mehul Jain <mehul.jain2029@gmail.com> wrote:\n> Users may want to always use \"--show-signature\" while using git-log and\n> related commands.\n>\n> When log.showSignature is set to true, git-log and related commands will\n> behave as if \"--show-signature\" was given to them.\n>\n> Note that this config variable is meant to affect git-log, git-show,\n> git-whatchanged and git-reflog. Other commands like git-format-patch,\n> git-rev-list are not to be affected by this config variable.\n>\n> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n> ---\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> @@ -900,6 +900,31 @@ test_expect_success GPG '--no-show-signature overrides --show-signature' '\n> +test_expect_success GPG 'log.showsignature=true behaves like --show-signature' '\n> +       git checkout -b test_sign master &&\n\nIt appears that you copied the bulk of this test from the 'log --graph\n--show-signature' test and changed it to create a new branch named\n'test_sign' rather than 'signed', however...\n\n> +       echo foo >foo &&\n> +       git add foo &&\n> +       git commit -S -m signed_commit &&\n> +       test_config log.showsignature true &&\n> +       git log -1 signed >actual &&\n\n... you're incorrectly testing against the 'signed' branch rather than\nthe 'test_sign' created specifically for this test.\n\n> +       grep \"gpg: Signature made\" actual &&\n> +       grep \"gpg: Good signature\" actual\n> +'\n> +\n> +test_expect_success GPG '--no-show-signature overrides log.showsignature=true' '\n> +       test_config log.showsignature true &&\n> +       git log -1 --no-show-signature signed >actual &&\n> +       ! grep \"^gpg:\" actual\n> +'\n> +\n> +test_expect_success GPG '--show-signature overrides log.showsignature=false' '\n> +       test_when_finished \"git reset --hard && git checkout master\" &&\n\nSo, in the first of these three new tests, you're setting up some\nstate by creating and checking out a new branch named 'test_sign', and\nleaving that branch checked out while these three tests run, and\nfinally use test_when_finished() in the last of the three tests to\nrestore sanity (by returning to the 'master' branch) when that test\nexits.\n\nThis is ugly and couples these three tests too tightly. It would be\nbetter to make each test more self-contained, not relying upon state\nleft over from previous tests.\n\n> +       test_config log.showsignature false &&\n> +       git log -1 --show-signature signed >actual &&\n> +       grep \"gpg: Signature made\" actual &&\n> +       grep \"gpg: Good signature\" actual\n> +'\n\nIn fact, the original 'log --graph --show-signature' test which\ncreated the 'signed' branch, the new --no-show-signature test added in\npatch 1/2, and the three new tests here could all just work against\nthe same 'signed' branch. There is no need to create a new 'test_sign'\nbranch for these three tests, or a 'nosign' branch for the patch 1/2\ntest.\n\nTherefore, it probably would make more sense to add a new distinct\n'setup signed' test (or just enhance the existing 'setup' test) which\ncreates the 'signed' branch and update the original 'log --graph\n--show-signature' to take advantage of it, as well as each of the new\ntests introduced by this patch series. And since each test would\nmention 'signed' explicitly in its git-log invocation, there's no need\nto leave that branch checked out, so the setup test itself only needs\ntest_when_finished() to ensure that the current branch is restored to\n'master'.\n"},{"id":"289620","messageId":"CA+DCAeTvaYNm3fPfO_WH7ZKo56mBnD8McfENJtQbNwjAL9eDcw@mail.gmail.com","threadId":"42650","inReplyTo":"CAPig+cRZyCZuec1GuxtW0p_8G1VPq1y846USM-TNmP3bbTYxQA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] log: add log.showSignature configuration variable","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-06-19T11:30:53Z","receivedAt":"2016-06-19T11:31:01Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Hi Eric,\n\nThanks for your review.\n\nOn Sun, Jun 19, 2016 at 12:29 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Sat, Jun 18, 2016 at 8:25 AM, Mehul Jain <mehul.jain2029@gmail.com> wrote:\n>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n>> @@ -900,6 +900,31 @@ test_expect_success GPG '--no-show-signature overrides --show-signature' '\n>> +test_expect_success GPG 'log.showsignature=true behaves like --show-signature' '\n>> +       git checkout -b test_sign master &&\n>\n> It appears that you copied the bulk of this test from the 'log --graph\n> --show-signature' test and changed it to create a new branch named\n> 'test_sign' rather than 'signed', however...\n>\n>> +       echo foo >foo &&\n>> +       git add foo &&\n>> +       git commit -S -m signed_commit &&\n>> +       test_config log.showsignature true &&\n>> +       git log -1 signed >actual &&\n>\n> ... you're incorrectly testing against the 'signed' branch rather than\n> the 'test_sign' created specifically for this test.\n\nYes, I made a mistake here.\n\n>> +       grep \"gpg: Signature made\" actual &&\n>> +       grep \"gpg: Good signature\" actual\n>> +'\n>> +\n>> +test_expect_success GPG '--no-show-signature overrides log.showsignature=true' '\n>> +       test_config log.showsignature true &&\n>> +       git log -1 --no-show-signature signed >actual &&\n>> +       ! grep \"^gpg:\" actual\n>> +'\n>> +\n>> +test_expect_success GPG '--show-signature overrides log.showsignature=false' '\n>> +       test_when_finished \"git reset --hard && git checkout master\" &&\n>\n> So, in the first of these three new tests, you're setting up some\n> state by creating and checking out a new branch named 'test_sign', and\n> leaving that branch checked out while these three tests run, and\n> finally use test_when_finished() in the last of the three tests to\n> restore sanity (by returning to the 'master' branch) when that test\n> exits.\n>\n> This is ugly and couples these three tests too tightly. It would be\n> better to make each test more self-contained, not relying upon state\n> left over from previous tests.\n>\n>> +       test_config log.showsignature false &&\n>> +       git log -1 --show-signature signed >actual &&\n>> +       grep \"gpg: Signature made\" actual &&\n>> +       grep \"gpg: Good signature\" actual\n>> +'\n>\n> In fact, the original 'log --graph --show-signature' test which\n> created the 'signed' branch, the new --no-show-signature test added in\n> patch 1/2, and the three new tests here could all just work against\n> the same 'signed' branch. There is no need to create a new 'test_sign'\n> branch for these three tests, or a 'nosign' branch for the patch 1/2\n> test.\n>\n> Therefore, it probably would make more sense to add a new distinct\n> 'setup signed' test (or just enhance the existing 'setup' test) which\n> creates the 'signed' branch and update the original 'log --graph\n> --show-signature' to take advantage of it, as well as each of the new\n> tests introduced by this patch series. And since each test would\n> mention 'signed' explicitly in its git-log invocation, there's no need\n> to leave that branch checked out, so the setup test itself only needs\n> test_when_finished() to ensure that the current branch is restored to\n> 'master'.\n\nAdding a new test 'setup signed' will work, where I will create a new\n'signed' branch and use that branch in new --no-show-signature test\nintroduced in patch 1/2, and the three tests in current patch 2/2. Also by\ncreating a preparatory patch for this series, I will modify the 'log --graph\n--show-signature' test, so that it can also take advantage of new 'setup\nsigned' test.\n\nThough I'm wondering if whether there is a need to create the new 'setup\nsigned' test. In 'log --graph --show-signature' test, we already have the\n'signed' branch, which could be used in the test introduced here. But this\nwill couple the tests, 'log --graph ...' and new ones, tightly. Because if in\nfuture someone changes the 'log --graph ...' test, then there is a possibility\nof new tests (introduced in patch 1/2 and 2/2) to fail. So creating a new test\nfor creation of 'signed' branch seems fair.\n\nThanks,\nMehul\n"}]}