From 402596aafa45ef371611bb795bdf8daa2bdb9286 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 7 Apr 2013 01:10:27 -0600 Subject: [PATCH 1/7] send-email: make annotate configurable Some people always do --annotate, lets not force them to always type that. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- Documentation/config.txt | 1 + Documentation/git-send-email.txt | 5 +++-- git-send-email.perl | 7 ++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index f79184c0a8..50840aab5e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1998,6 +1998,7 @@ sendemail..*:: sendemail.aliasesfile:: sendemail.aliasfiletype:: +sendemail.annotate:: sendemail.bcc:: sendemail.cc:: sendemail.cccmd:: diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 0cffef8aa5..40a9a9abc1 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -45,8 +45,9 @@ Composing ~~~~~~~~~ --annotate:: - Review and edit each patch you're about to send. See the - CONFIGURATION section for 'sendemail.multiedit'. + Review and edit each patch you're about to send. Default is the value + of 'sendemail.annotate'. See the CONFIGURATION section for + 'sendemail.multiedit'. --bcc=
:: Specify a "Bcc:" value for each email. Default is the value of diff --git a/git-send-email.perl b/git-send-email.perl index 7297472818..bd13cc812d 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -54,7 +54,7 @@ sub usage { --[no-]bcc * Email Bcc: --subject * Email "Subject:" --in-reply-to * Email "In-Reply-To:" - --annotate * Review each patch that will be sent in an editor. + --[no-]annotate * Review each patch that will be sent in an editor. --compose * Open an editor for introduction. --compose-encoding * Encoding to assume for introduction. --8bit-encoding * Encoding to assume 8bit mails if undeclared @@ -212,7 +212,8 @@ sub do_edit { "signedoffbycc" => [\$signed_off_by_cc, undef], "signedoffcc" => [\$signed_off_by_cc, undef], # Deprecated "validate" => [\$validate, 1], - "multiedit" => [\$multiedit, undef] + "multiedit" => [\$multiedit, undef], + "annotate" => [\$annotate, undef] ); my %config_settings = ( @@ -304,7 +305,7 @@ sub signal_handler { "smtp-debug:i" => \$debug_net_smtp, "smtp-domain:s" => \$smtp_domain, "identity=s" => \$identity, - "annotate" => \$annotate, + "annotate!" => \$annotate, "compose" => \$compose, "quiet" => \$quiet, "cc-cmd=s" => \$cc_cmd, From 80d35ca0aaeca654562974be6ea528e22bc76e8c Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 7 Apr 2013 12:46:20 -0500 Subject: [PATCH 2/7] format-patch: improve head calculation for cover-letter If we do it after the revision traversal we can be sure that this is indeed a commit that will be processed (i.e. not a merge) and it's the top most one (thus removing the NEEDSWORK comment, at least we show the same as 'git diff --stat' output that appears in the cover-letter). While we are at it, since we know there's nothing to generate, exit sooner in all cases, like --cover-letter currently does. Also, if there's nothing to generate and cover-letter is specified, a different code-path might be triggered that is not currently covered in the test-case, so add a test for it. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- builtin/log.c | 22 ++++------------------ t/t4014-format-patch.sh | 5 +++++ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 0f318107e5..679282706a 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1311,24 +1311,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.show_root_diff = 1; if (cover_letter) { - /* - * NEEDSWORK:randomly pick one positive commit to show - * diffstat; this is often the tip and the command - * happens to do the right thing in most cases, but a - * complex command like "--cover-letter a b c ^bottom" - * picks "c" and shows diffstat between bottom..c - * which may not match what the series represents at - * all and totally broken. - */ - int i; - for (i = 0; i < rev.pending.nr; i++) { - struct object *o = rev.pending.objects[i].item; - if (!(o->flags & UNINTERESTING)) - head = (struct commit *)o; - } - /* There is nothing to show; it is not an error, though. */ - if (!head) - return 0; if (!branch_name) branch_name = find_branch_name(&rev); } @@ -1364,6 +1346,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) list = xrealloc(list, nr * sizeof(list[0])); list[nr - 1] = commit; } + if (nr == 0) + /* nothing to do */ + return 0; + head = list[0]; total = nr; if (!keep_subject && auto_number && total > 1) numbered = 1; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index b993dae645..0ada8c78e7 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1284,4 +1284,9 @@ test_expect_success 'cover letter using branch description (6)' ' grep hello actual >/dev/null ' +test_expect_success 'cover letter with nothing' ' + git format-patch --stdout --cover-letter >actual && + test_line_count = 0 actual +' + test_done From 427a8ec5e75f4450620e5df3e1f1071f48c7772c Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 7 Apr 2013 12:46:21 -0500 Subject: [PATCH 3/7] format-patch: refactor branch name calculation By moving the part that relies on rev->pending earlier, where we are already checking the special case where there's only one ref. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- builtin/log.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 679282706a..804626b0ea 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1052,15 +1052,6 @@ static char *find_branch_name(struct rev_info *rev) if (0 <= positive) { ref = rev->cmdline.rev[positive].name; tip_sha1 = rev->cmdline.rev[positive].item->sha1; - } else if (!rev->cmdline.nr && rev->pending.nr == 1 && - !strcmp(rev->pending.objects[0].name, "HEAD")) { - /* - * No actual ref from command line, but "HEAD" from - * rev->def was added in setup_revisions() - * e.g. format-patch --cover-letter -12 - */ - ref = "HEAD"; - tip_sha1 = rev->pending.objects[0].item->sha1; } else { return NULL; } @@ -1280,28 +1271,36 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) } if (rev.pending.nr == 1) { + int check_head = 0; + if (rev.max_count < 0 && !rev.show_root_diff) { /* * This is traditional behaviour of "git format-patch * origin" that prepares what the origin side still * does not have. */ - unsigned char sha1[20]; - const char *ref; - rev.pending.objects[0].item->flags |= UNINTERESTING; add_head_to_pending(&rev); - ref = resolve_ref_unsafe("HEAD", sha1, 1, NULL); - if (ref && !prefixcmp(ref, "refs/heads/")) - branch_name = xstrdup(ref + strlen("refs/heads/")); - else - branch_name = xstrdup(""); /* no branch */ + check_head = 1; } /* * Otherwise, it is "format-patch -22 HEAD", and/or * "format-patch --root HEAD". The user wants * get_revision() to do the usual traversal. */ + + if (!strcmp(rev.pending.objects[0].name, "HEAD")) + check_head = 1; + + if (check_head) { + unsigned char sha1[20]; + const char *ref; + ref = resolve_ref_unsafe("HEAD", sha1, 1, NULL); + if (ref && !prefixcmp(ref, "refs/heads/")) + branch_name = xstrdup(ref + strlen("refs/heads/")); + else + branch_name = xstrdup(""); /* no branch */ + } } /* From aa089cd9abc5332cddebc60c21f797fffd1ab471 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 7 Apr 2013 12:46:22 -0500 Subject: [PATCH 4/7] log: update to OPT_BOOL OPT_BOOLEAN is deprecated, and this is what we want. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- builtin/log.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 804626b0ea..40347adde4 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -100,9 +100,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, int quiet = 0, source = 0, mailmap = 0; const struct option builtin_log_options[] = { - OPT_BOOLEAN(0, "quiet", &quiet, N_("suppress diff output")), - OPT_BOOLEAN(0, "source", &source, N_("show source")), - OPT_BOOLEAN(0, "use-mailmap", &mailmap, N_("Use mail map file")), + OPT_BOOL(0, "quiet", &quiet, N_("suppress diff output")), + OPT_BOOL(0, "source", &source, N_("show source")), + OPT_BOOL(0, "use-mailmap", &mailmap, N_("Use mail map file")), { OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate options"), PARSE_OPT_OPTARG, decorate_callback}, OPT_END() @@ -1092,12 +1092,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) { OPTION_CALLBACK, 'N', "no-numbered", &numbered, NULL, N_("use [PATCH] even with multiple patches"), PARSE_OPT_NOARG, no_numbered_callback }, - OPT_BOOLEAN('s', "signoff", &do_signoff, N_("add Signed-off-by:")), - OPT_BOOLEAN(0, "stdout", &use_stdout, + OPT_BOOL('s', "signoff", &do_signoff, N_("add Signed-off-by:")), + OPT_BOOL(0, "stdout", &use_stdout, N_("print patches to standard out")), - OPT_BOOLEAN(0, "cover-letter", &cover_letter, + OPT_BOOL(0, "cover-letter", &cover_letter, N_("generate a cover letter")), - OPT_BOOLEAN(0, "numbered-files", &just_numbers, + OPT_BOOL(0, "numbered-files", &just_numbers, N_("use simple number sequence for output file names")), OPT_STRING(0, "suffix", &fmt_patch_suffix, N_("sfx"), N_("use instead of '.patch'")), From 2a4c26076c3c99300a51439702791c17e7d21e6c Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 7 Apr 2013 12:46:23 -0500 Subject: [PATCH 5/7] format-patch: add format.coverLetter configuration variable Also, add a new option: 'auto', so if there's more than one patch, the cover letter is generated, otherwise it's not. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- Documentation/config.txt | 5 +++++ Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 32 ++++++++++++++++++++++++------ t/t4014-format-patch.sh | 28 ++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 50840aab5e..83b924494c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1096,6 +1096,11 @@ format.signoff:: the rights to submit this work under the same open source license. Please see the 'SubmittingPatches' document for further discussion. +format.coverLetter:: + A boolean that controls whether to generate a cover-letter when + format-patch is invoked, but in addition can be set to "auto", to + generate a cover-letter only when there's more than one patch. + filter..clean:: The command which is used to convert the content of a worktree file to a blob upon checkin. See linkgit:gitattributes[5] for diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 3a62f50eda..39118774af 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -20,7 +20,7 @@ SYNOPSIS [--ignore-if-in-upstream] [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) ] [--to=] [--cc=] - [--cover-letter] [--quiet] [--notes[=]] + [--[no-]cover-letter] [--quiet] [--notes[=]] [] [ | ] @@ -195,7 +195,7 @@ will want to ensure that threading is disabled for `git send-email`. `Cc:`, and custom) headers added so far from config or command line. ---cover-letter:: +--[no-]cover-letter:: In addition to the patches, generate a cover letter file containing the shortlog and the overall diffstat. You can fill in a description in the file before sending it out. @@ -260,6 +260,7 @@ attachments, and sign off patches with configuration variables. cc = attach [ = mime-boundary-string ] signoff = true + coverletter = auto ------------ diff --git a/builtin/log.c b/builtin/log.c index 40347adde4..9d2b23aa80 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -622,6 +622,14 @@ static void add_header(const char *value) static int thread; static int do_signoff; static const char *signature = git_version_string; +static int config_cover_letter; + +enum { + COVER_UNSET, + COVER_OFF, + COVER_ON, + COVER_AUTO +}; static int git_format_config(const char *var, const char *value, void *cb) { @@ -683,6 +691,14 @@ static int git_format_config(const char *var, const char *value, void *cb) } if (!strcmp(var, "format.signature")) return git_config_string(&signature, var, value); + if (!strcmp(var, "format.coverletter")) { + if (value && !strcasecmp(value, "auto")) { + config_cover_letter = COVER_AUTO; + return 0; + } + config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF; + return 0; + } return git_log_config(var, value, cb); } @@ -1074,7 +1090,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) int start_number = -1; int just_numbers = 0; int ignore_if_in_upstream = 0; - int cover_letter = 0; + int cover_letter = -1; int boundary_count = 0; int no_binary_diff = 0; struct commit *origin = NULL, *head = NULL; @@ -1309,11 +1325,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) */ rev.show_root_diff = 1; - if (cover_letter) { - if (!branch_name) - branch_name = find_branch_name(&rev); - } - if (ignore_if_in_upstream) { /* Don't say anything if head and upstream are the same. */ if (rev.pending.nr == 2) { @@ -1354,6 +1365,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) numbered = 1; if (numbered) rev.total = total + start_number - 1; + if (cover_letter == -1) { + if (config_cover_letter == COVER_AUTO) + cover_letter = (total > 1); + else + cover_letter = (config_cover_letter == COVER_ON); + } + if (in_reply_to || thread || cover_letter) rev.ref_message_ids = xcalloc(1, sizeof(struct string_list)); if (in_reply_to) { @@ -1365,6 +1383,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) if (cover_letter) { if (thread) gen_message_id(&rev, "cover"); + if (!branch_name) + branch_name = find_branch_name(&rev); make_cover_letter(&rev, use_stdout, origin, nr, list, head, branch_name, quiet); total++; diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 0ada8c78e7..8368181d6c 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1289,4 +1289,32 @@ test_expect_success 'cover letter with nothing' ' test_line_count = 0 actual ' +test_expect_success 'cover letter auto' ' + mkdir -p tmp && + test_when_finished "rm -rf tmp; + git config --unset format.coverletter" && + + git config format.coverletter auto && + git format-patch -o tmp -1 >list && + test_line_count = 1 list && + git format-patch -o tmp -2 >list && + test_line_count = 3 list +' + +test_expect_success 'cover letter auto user override' ' + mkdir -p tmp && + test_when_finished "rm -rf tmp; + git config --unset format.coverletter" && + + git config format.coverletter auto && + git format-patch -o tmp --cover-letter -1 >list && + test_line_count = 2 list && + git format-patch -o tmp --cover-letter -2 >list && + test_line_count = 3 list && + git format-patch -o tmp --no-cover-letter -1 >list && + test_line_count = 1 list && + git format-patch -o tmp --no-cover-letter -2 >list && + test_line_count = 2 list +' + test_done From d7ddad012b1badaf96075e880d546ef3525f23ff Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 7 Apr 2013 12:46:24 -0500 Subject: [PATCH 6/7] format-patch: trivial cleanups Now that the cover-letter code has been shuffled, we can do some cleanups. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- builtin/log.c | 71 +++++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 9d2b23aa80..ad46f72950 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -810,9 +810,37 @@ static void add_branch_description(struct strbuf *buf, const char *branch_name) } } +static char *find_branch_name(struct rev_info *rev) +{ + int i, positive = -1; + unsigned char branch_sha1[20]; + const unsigned char *tip_sha1; + const char *ref; + char *full_ref, *branch = NULL; + + for (i = 0; i < rev->cmdline.nr; i++) { + if (rev->cmdline.rev[i].flags & UNINTERESTING) + continue; + if (positive < 0) + positive = i; + else + return NULL; + } + if (positive < 0) + return NULL; + ref = rev->cmdline.rev[positive].name; + tip_sha1 = rev->cmdline.rev[positive].item->sha1; + if (dwim_ref(ref, strlen(ref), branch_sha1, &full_ref) && + !prefixcmp(full_ref, "refs/heads/") && + !hashcmp(tip_sha1, branch_sha1)) + branch = xstrdup(full_ref + strlen("refs/heads/")); + free(full_ref); + return branch; +} + static void make_cover_letter(struct rev_info *rev, int use_stdout, struct commit *origin, - int nr, struct commit **list, struct commit *head, + int nr, struct commit **list, const char *branch_name, int quiet) { @@ -826,6 +854,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, struct diff_options opts; int need_8bit_cte = 0; struct pretty_print_context pp = {0}; + struct commit *head = list[0]; if (rev->commit_format != CMIT_FMT_EMAIL) die(_("Cover letter needs email format")); @@ -843,6 +872,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, if (has_non_ascii(list[i]->buffer)) need_8bit_cte = 1; + if (!branch_name) + branch_name = find_branch_name(rev); + msg = body; pp.fmt = CMIT_FMT_EMAIL; pp.date_mode = DATE_RFC2822; @@ -1049,36 +1081,6 @@ static int cc_callback(const struct option *opt, const char *arg, int unset) return 0; } -static char *find_branch_name(struct rev_info *rev) -{ - int i, positive = -1; - unsigned char branch_sha1[20]; - const unsigned char *tip_sha1; - const char *ref; - char *full_ref, *branch = NULL; - - for (i = 0; i < rev->cmdline.nr; i++) { - if (rev->cmdline.rev[i].flags & UNINTERESTING) - continue; - if (positive < 0) - positive = i; - else - return NULL; - } - if (0 <= positive) { - ref = rev->cmdline.rev[positive].name; - tip_sha1 = rev->cmdline.rev[positive].item->sha1; - } else { - return NULL; - } - if (dwim_ref(ref, strlen(ref), branch_sha1, &full_ref) && - !prefixcmp(full_ref, "refs/heads/") && - !hashcmp(tip_sha1, branch_sha1)) - branch = xstrdup(full_ref + strlen("refs/heads/")); - free(full_ref); - return branch; -} - int cmd_format_patch(int argc, const char **argv, const char *prefix) { struct commit *commit; @@ -1093,7 +1095,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) int cover_letter = -1; int boundary_count = 0; int no_binary_diff = 0; - struct commit *origin = NULL, *head = NULL; + struct commit *origin = NULL; const char *in_reply_to = NULL; struct patch_ids ids; struct strbuf buf = STRBUF_INIT; @@ -1359,7 +1361,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) if (nr == 0) /* nothing to do */ return 0; - head = list[0]; total = nr; if (!keep_subject && auto_number && total > 1) numbered = 1; @@ -1383,10 +1384,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) if (cover_letter) { if (thread) gen_message_id(&rev, "cover"); - if (!branch_name) - branch_name = find_branch_name(&rev); make_cover_letter(&rev, use_stdout, - origin, nr, list, head, branch_name, quiet); + origin, nr, list, branch_name, quiet); total++; start_number--; } From 0597ffa5ecff1b83f384357ab88afaf21838caa2 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Sun, 14 Apr 2013 17:27:04 -0500 Subject: [PATCH 7/7] rebase-am: explicitly disable cover-letter If the user has a cover-letter configuration set to anything other than 'false', 'git format-patch' may generate a cover letter, which has no place in "format-patch | am" pipeline. The internal invocation of format-patch must explicitly override the configuration from the command line, just like --src-prefix and other options already do. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- git-rebase--am.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 97f31dc7af..f84854f09a 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -31,8 +31,8 @@ else rm -f "$GIT_DIR/rebased-patches" git format-patch -k --stdout --full-index --ignore-if-in-upstream \ - --src-prefix=a/ --dst-prefix=b/ \ - --no-renames $root_flag "$revisions" >"$GIT_DIR/rebased-patches" + --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \ + $root_flag "$revisions" >"$GIT_DIR/rebased-patches" ret=$? if test 0 != $ret