{"thread":{"id":"58914","subject":"[PATCH] commit: add signoff optional dedup/no-dedup modes","startedAt":"2022-12-07T14:33:36Z","lastAt":"2022-12-07T14:33:36Z","messageCount":1,"participants":["David Caro via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"468714","messageId":"pull.1438.git.1670423522572.gitgitgadget@gmail.com","threadId":"58914","inReplyTo":null,"subject":"[PATCH] commit: add signoff optional dedup/no-dedup modes","fromName":"David Caro via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-12-07T14:32:02Z","receivedAt":"2022-12-07T14:33:36Z","isPatch":true,"sender":{"key":"dcaro@wikimedia.org","avatar":"https://gravatar.com/avatar/c38df122f2f80dd69364cf54709edd67d85d305120b69bf14bf48df912fd8fc4?d=mp&s=160"},"body":"From: David Caro <me@dcaro.es>\n\nThis allows selecting if you want your Signed-off-by: trailer\ndeduplicated or not.\n\nSigned-off-by: David Caro <me@dcaro.es>\n---\n    commit: allow passing dedup/no-dedup mode to --signoff\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1438%2Fdavid-caro%2Favoid_sob_duplication-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1438/david-caro/avoid_sob_duplication-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1438\n\n Documentation/git-commit.txt          |  2 +-\n Documentation/signoff-option.txt      | 14 +++++-\n builtin/commit.c                      | 19 +++++---\n t/t7501-commit-basic-functionality.sh | 69 +++++++++++++++++++++++++++\n 4 files changed, 95 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 225c6c9f2e5..7edf1ec4554 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -8,7 +8,7 @@ git-commit - Record changes to the repository\n SYNOPSIS\n --------\n [verse]\n-'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\n+'git commit' [-a | --interactive | --patch] [-s[<mode>]] [-v] [-u<mode>] [--amend]\n \t   [--dry-run] [(-c | -C | --squash) <commit> | --fixup [(amend|reword):]<commit>)]\n \t   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]\n \t   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]\ndiff --git a/Documentation/signoff-option.txt b/Documentation/signoff-option.txt\nindex 12aa2333e46..d8b06f1e791 100644\n--- a/Documentation/signoff-option.txt\n+++ b/Documentation/signoff-option.txt\n@@ -1,7 +1,7 @@\n ifdef::git-commit[]\n--s::\n+-s[<mode>]::\n endif::git-commit[]\n---signoff::\n+--signoff[=<mode>]::\n --no-signoff::\n \tAdd a `Signed-off-by` trailer by the committer at the end of the commit\n \tlog message.  The meaning of a signoff depends on the project\n@@ -16,3 +16,13 @@ endif::git-commit[]\n +\n The --no-signoff option can be used to countermand an earlier --signoff\n option on the command line.\n++\n+The mode parameter is optional (defaults to no-dedup), and is used to specify\n+if it should deduplicate the `Signed-off-by` trailer or not when trying to\n+add it to the trailers.\n++\n+The possible options are:\n++\n+\t- 'no-dedup' - Will add the trailer unless it's already the last line.\n+\t- 'dedup' - Will not add the trailer if it's already anywhere in the\n+\t  trailers.\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 06b1330346f..cd769af5bf0 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -40,7 +40,7 @@\n #include \"pretty.h\"\n \n static const char * const builtin_commit_usage[] = {\n-\tN_(\"git commit [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\\n\"\n+\tN_(\"git commit [-a | --interactive | --patch] [-s[<mode>]] [-v] [-u<mode>] [--amend]\\n\"\n \t   \"           [--dry-run] [(-c | -C | --squash) <commit> | --fixup [(amend|reword):]<commit>)]\\n\"\n \t   \"           [-F <file> | -m <msg>] [--reset-author] [--allow-empty]\\n\"\n \t   \"           [--allow-empty-message] [--no-verify] [-e] [--author=<author>]\\n\"\n@@ -116,13 +116,13 @@ static const char *author_message, *author_message_buffer;\n static char *edit_message, *use_message;\n static char *fixup_message, *fixup_commit, *squash_message;\n static const char *fixup_prefix;\n-static int all, also, interactive, patch_interactive, only, amend, signoff;\n+static int all, also, interactive, patch_interactive, only, amend;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n-static char *sign_commit, *pathspec_from_file;\n+static char *sign_commit, *pathspec_from_file, *signoff_arg;\n static struct strvec trailer_args = STRVEC_INIT;\n \n /*\n@@ -888,8 +888,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tif (clean_message_contents)\n \t\tstrbuf_stripspace(&sb, 0);\n \n-\tif (signoff)\n-\t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n+\tif (signoff_arg) {\n+\t\tif (!strcmp(signoff_arg, \"dedup\"))\n+\t\t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 1);\n+\t\telse if (!strcmp(signoff_arg, \"no-dedup\"))\n+\t\t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n+\t\telse\n+\t\t\tdie(_(\"Invalid signoff mode: %s\"), signoff_arg);\n+\t}\n \n \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n \t\tdie_errno(_(\"could not write commit template\"));\n@@ -1645,7 +1651,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_CALLBACK_F(0, \"trailer\", &trailer_args, N_(\"trailer\"), N_(\"add custom trailer(s)\"), PARSE_OPT_NONEG, opt_pass_trailer),\n-\t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n+\t\t{ OPTION_STRING, 's', \"signoff\", &signoff_arg, N_(\"mode\"),\n+\t\t  N_(\"add a Signed-off-by trailer, optional modes: no-dedup, dedup. (Default: no-dedup)\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"no-dedup\" },\n \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n \t\tOPT_CLEANUP(&cleanup_arg),\ndiff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh\nindex fb5417d5e7e..64e710879ab 100755\n--- a/t/t7501-commit-basic-functionality.sh\n+++ b/t/t7501-commit-basic-functionality.sh\n@@ -541,6 +541,75 @@ test_expect_success 'signoff not confused by ---' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'signoff default mode adds when not last in trailers' '\n+\tcat >message <<-EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\t\tSigned-off-by: A. U. <author@example.com>\n+\tEOF\n+\tcat >expected <<-EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\t\tSigned-off-by: A. U. <author@example.com>\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit commit --allow-empty --signoff -F message &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'signoff no-dedup mode adds when not last in trailers' '\n+\tcat >message <<-EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\t\tSigned-off-by: A. U. <author@example.com>\n+\tEOF\n+\tcat >expected <<-EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\t\tSigned-off-by: A. U. <author@example.com>\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit commit --allow-empty --signoff=no-dedup -F message &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_expect_success 'signoff dedup mode does not duplicate when not last in trailers' '\n+\tcat >message <<-EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\t\tSigned-off-by: A. U. <author@example.com>\n+\tEOF\n+\tcat >expected <<-EOF &&\n+\t\tsubject\n+\n+\t\tbody\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\t\tSigned-off-by: A. U. <author@example.com>\n+\tEOF\n+\tgit commit --allow-empty --signoff=dedup -F message &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'multiple -m' '\n \n \t>negative &&\n\nbase-commit: 2e71cbbddd64695d43383c25c7a054ac4ff86882\n-- \ngitgitgadget\n"}]}