@matheusbsbs / matheus-bagliano / commits / 37f7a85793
Fork 0

Teach commit about CHERRY_PICK_HEAD

Previously the user was advised to use commit -c CHERRY_PICK_HEAD after a conflicting cherry-pick. While this would preserve the original commit's authorship, it would sadly discard cherry-pick's carefully crafted MERGE_MSG (which contains the list of conflicts as well as the original commit-id in the case of cherry-pick -x). On the other hand, if a bare 'commit' were performed, it would preserve the MERGE_MSG while resetting the authorship. In other words, there was no way to simultaneously take the authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG. This change fixes that situation. A bare 'commit' will now take the authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG. If the user wishes to reset authorship, that must now be done explicitly via --reset-author. A side-benefit of passing commit authorship along this way is that we can eliminate redundant authorship parsing code from revert.c. (Also removed an unused include from revert.c) Signed-off-by: Jay Soffian <jaysoffian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jay Soffian committed Feb 19, 2011 at 23:12 UTC 37f7a8579363a98efc48dfb6964a519034fc9acc
7 files changed +158 -104
Documentation/git-commit.txt
+4 -3
@@ -84,9 +84,10 @@ OPTIONS
84 linkgit:git-rebase[1] for details.
85
86 --reset-author::
87 - When used with -C/-c/--amend options, declare that the
88 - authorship of the resulting commit now belongs of the committer.
89 - This also renews the author timestamp.
87 + When used with -C/-c/--amend options, or when committing after a
88 + a conflicting cherry-pick, declare that the authorship of the
89 + resulting commit now belongs of the committer. This also renews
90 + the author timestamp.
91
92 --short::
93 When doing a dry-run, give the output in the short-format. See
builtin/commit.c
+113 -42
@@ -54,9 +54,17 @@ static const char empty_amend_advice[] =
54 "it empty. You can repeat your command with --allow-empty, or you can\n"
55 "remove the commit entirely with \"git reset HEAD^\".\n";
56
57 +static const char empty_cherry_pick_advice[] =
58 +"The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
59 +"If you wish to commit it anyway, use:\n"
60 +"\n"
61 +" git commit --allow-empty\n"
62 +"\n"
63 +"Otherwise, please use 'git reset'\n";
64 +
65 static unsigned char head_sha1[20];
66
59 -static char *use_message_buffer;
67 +static const char *use_message_buffer;
68 static const char commit_editmsg[] = "COMMIT_EDITMSG";
69 static struct lock_file index_lock; /* real index */
70 static struct lock_file false_lock; /* used only for partial commits */
@@ -68,6 +76,11 @@ static enum {
76
77 static const char *logfile, *force_author;
78 static const char *template_file;
79 +/*
80 + * The _message variables are commit names from which to take
81 + * the commit message and/or authorship.
82 + */
83 +static const char *author_message, *author_message_buffer;
84 static char *edit_message, *use_message;
85 static char *fixup_message, *squash_message;
86 static int all, edit_flag, also, interactive, only, amend, signoff;
@@ -88,7 +101,8 @@ static enum {
101 } cleanup_mode;
102 static char *cleanup_arg;
103
91 -static int use_editor = 1, initial_commit, in_merge, include_status = 1;
104 +static enum commit_whence whence;
105 +static int use_editor = 1, initial_commit, include_status = 1;
106 static int show_ignored_in_status;
107 static const char *only_include_assumed;
108 static struct strbuf message;
@@ -163,6 +177,36 @@ static struct option builtin_commit_options[] = {
177 OPT_END()
178 };
179
180 +static void determine_whence(struct wt_status *s)
181 +{
182 + if (file_exists(git_path("MERGE_HEAD")))
183 + whence = FROM_MERGE;
184 + else if (file_exists(git_path("CHERRY_PICK_HEAD")))
185 + whence = FROM_CHERRY_PICK;
186 + else
187 + whence = FROM_COMMIT;
188 + if (s)
189 + s->whence = whence;
190 +}
191 +
192 +static const char *whence_s(void)
193 +{
194 + char *s = "";
195 +
196 + switch (whence) {
197 + case FROM_COMMIT:
198 + break;
199 + case FROM_MERGE:
200 + s = "merge";
201 + break;
202 + case FROM_CHERRY_PICK:
203 + s = "cherry-pick";
204 + break;
205 + }
206 +
207 + return s;
208 +}
209 +
210 static void rollback_index_files(void)
211 {
212 switch (commit_style) {
@@ -378,8 +422,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
422 */
423 commit_style = COMMIT_PARTIAL;
424
381 - if (in_merge)
382 - die("cannot do a partial commit during a merge.");
425 + if (whence != FROM_COMMIT)
426 + die("cannot do a partial commit during a %s.", whence_s());
427
428 memset(&partial, 0, sizeof(partial));
429 partial.strdup_strings = 1;
@@ -469,18 +513,18 @@ static void determine_author_info(struct strbuf *author_ident)
513 email = getenv("GIT_AUTHOR_EMAIL");
514 date = getenv("GIT_AUTHOR_DATE");
515
472 - if (use_message && !renew_authorship) {
516 + if (author_message) {
517 const char *a, *lb, *rb, *eol;
518
475 - a = strstr(use_message_buffer, "\nauthor ");
519 + a = strstr(author_message_buffer, "\nauthor ");
520 if (!a)
477 - die("invalid commit: %s", use_message);
521 + die("invalid commit: %s", author_message);
522
523 lb = strchrnul(a + strlen("\nauthor "), '<');
524 rb = strchrnul(lb, '>');
525 eol = strchrnul(rb, '\n');
526 if (!*lb || !*rb || !*eol)
483 - die("invalid commit: %s", use_message);
527 + die("invalid commit: %s", author_message);
528
529 if (lb == a + strlen("\nauthor "))
530 /* \nauthor <foo@example.com> */
@@ -641,11 +685,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
685 }
686
687 /*
644 - * This final case does not modify the template message,
645 - * it just sets the argument to the prepare-commit-msg hook.
688 + * The remaining cases don't modify the template message, but
689 + * just set the argument(s) to the prepare-commit-msg hook.
690 */
647 - else if (in_merge)
691 + else if (whence == FROM_MERGE)
692 hook_arg1 = "merge";
693 + else if (whence == FROM_CHERRY_PICK) {
694 + hook_arg1 = "commit";
695 + hook_arg2 = "CHERRY_PICK_HEAD";
696 + }
697
698 if (squash_message) {
699 /*
@@ -694,16 +742,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
742 strbuf_addstr(&committer_ident, git_committer_info(0));
743 if (use_editor && include_status) {
744 char *ai_tmp, *ci_tmp;
697 - if (in_merge)
745 + if (whence != FROM_COMMIT)
746 fprintf(fp,
747 "#\n"
700 - "# It looks like you may be committing a MERGE.\n"
748 + "# It looks like you may be committing a %s.\n"
749 "# If this is not correct, please remove the file\n"
750 "# %s\n"
751 "# and try again.\n"
752 "#\n",
705 - git_path("MERGE_HEAD"));
706 -
753 + whence_s(),
754 + git_path(whence == FROM_MERGE
755 + ? "MERGE_HEAD"
756 + : "CHERRY_PICK_HEAD"));
757 fprintf(fp,
758 "\n"
759 "# Please enter the commit message for your changes.");
@@ -766,11 +816,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
816
817 fclose(fp);
818
769 - if (!commitable && !in_merge && !allow_empty &&
819 + /*
820 + * Reject an attempt to record a non-merge empty commit without
821 + * explicit --allow-empty. In the cherry-pick case, it may be
822 + * empty due to conflict resolution, which the user should okay.
823 + */
824 + if (!commitable && whence != FROM_MERGE && !allow_empty &&
825 !(amend && is_a_merge(head_sha1))) {
826 run_status(stdout, index_file, prefix, 0, s);
827 if (amend)
828 fputs(empty_amend_advice, stderr);
829 + else if (whence == FROM_CHERRY_PICK)
830 + fputs(empty_cherry_pick_advice, stderr);
831 return 0;
832 }
833
@@ -898,6 +955,28 @@ static void handle_untracked_files_arg(struct wt_status *s)
955 die("Invalid untracked files mode '%s'", untracked_files_arg);
956 }
957
958 +static const char *read_commit_message(const char *name)
959 +{
960 + const char *out_enc, *out;
961 + struct commit *commit;
962 +
963 + commit = lookup_commit_reference_by_name(name);
964 + if (!commit)
965 + die("could not lookup commit %s", name);
966 + out_enc = get_commit_output_encoding();
967 + out = logmsg_reencode(commit, out_enc);
968 +
969 + /*
970 + * If we failed to reencode the buffer, just copy it
971 + * byte for byte so the user can try to fix it up.
972 + * This also handles the case where input and output
973 + * encodings are identical.
974 + */
975 + if (out == NULL)
976 + out = xstrdup(commit->buffer);
977 + return out;
978 +}
979 +
980 static int parse_and_validate_options(int argc, const char *argv[],
981 const char * const usage[],
982 const char *prefix,
@@ -927,8 +1006,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
1006 /* Sanity check options */
1007 if (amend && initial_commit)
1008 die("You have nothing to amend.");
930 - if (amend && in_merge)
931 - die("You are in the middle of a merge -- cannot amend.");
1009 + if (amend && whence != FROM_COMMIT)
1010 + die("You are in the middle of a %s -- cannot amend.", whence_s());
1011 if (fixup_message && squash_message)
1012 die("Options --squash and --fixup cannot be used together");
1013 if (use_message)
@@ -947,26 +1026,18 @@ static int parse_and_validate_options(int argc, const char *argv[],
1026 use_message = edit_message;
1027 if (amend && !use_message && !fixup_message)
1028 use_message = "HEAD";
950 - if (!use_message && renew_authorship)
1029 + if (!use_message && whence != FROM_CHERRY_PICK && renew_authorship)
1030 die("--reset-author can be used only with -C, -c or --amend.");
1031 if (use_message) {
953 - const char *out_enc;
954 - struct commit *commit;
955 -
956 - commit = lookup_commit_reference_by_name(use_message);
957 - if (!commit)
958 - die("could not lookup commit %s", use_message);
959 - out_enc = get_commit_output_encoding();
960 - use_message_buffer = logmsg_reencode(commit, out_enc);
961 -
962 - /*
963 - * If we failed to reencode the buffer, just copy it
964 - * byte for byte so the user can try to fix it up.
965 - * This also handles the case where input and output
966 - * encodings are identical.
967 - */
968 - if (use_message_buffer == NULL)
969 - use_message_buffer = xstrdup(commit->buffer);
1032 + use_message_buffer = read_commit_message(use_message);
1033 + if (!renew_authorship) {
1034 + author_message = use_message;
1035 + author_message_buffer = use_message_buffer;
1036 + }
1037 + }
1038 + if (whence == FROM_CHERRY_PICK && !renew_authorship) {
1039 + author_message = "CHERRY_PICK_HEAD";
1040 + author_message_buffer = read_commit_message(author_message);
1041 }
1042
1043 if (!!also + !!only + !!all + !!interactive > 1)
@@ -1117,7 +1188,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
1188 wt_status_prepare(&s);
1189 gitmodules_config();
1190 git_config(git_status_config, &s);
1120 - in_merge = file_exists(git_path("MERGE_HEAD"));
1191 + determine_whence(&s);
1192 argc = parse_options(argc, argv, prefix,
1193 builtin_status_options,
1194 builtin_status_usage, 0);
@@ -1140,7 +1211,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
1211 }
1212
1213 s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
1143 - s.in_merge = in_merge;
1214 s.ignore_submodule_arg = ignore_submodule_arg;
1215 wt_status_collect(&s);
1216
@@ -1302,8 +1372,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
1372
1373 wt_status_prepare(&s);
1374 git_config(git_commit_config, &s);
1305 - in_merge = file_exists(git_path("MERGE_HEAD"));
1306 - s.in_merge = in_merge;
1375 + determine_whence(&s);
1376
1377 if (s.use_color == -1)
1378 s.use_color = git_use_color_default;
@@ -1340,7 +1409,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
1409
1410 for (c = commit->parents; c; c = c->next)
1411 pptr = &commit_list_insert(c->item, pptr)->next;
1343 - } else if (in_merge) {
1412 + } else if (whence == FROM_MERGE) {
1413 struct strbuf m = STRBUF_INIT;
1414 FILE *fp;
1415
@@ -1369,7 +1438,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
1438 parents = reduce_heads(parents);
1439 } else {
1440 if (!reflog_msg)
1372 - reflog_msg = "commit";
1441 + reflog_msg = (whence == FROM_CHERRY_PICK)
1442 + ? "commit (cherry-pick)"
1443 + : "commit";
1444 pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
1445 }
1446
builtin/revert.c
+1 -55
@@ -3,7 +3,6 @@
3 #include "object.h"
4 #include "commit.h"
5 #include "tag.h"
6 -#include "wt-status.h"
6 #include "run-command.h"
7 #include "exec_cmd.h"
8 #include "utf8.h"
@@ -181,56 +180,6 @@ static void add_message_to_msg(struct strbuf *msgbuf, const char *message)
180 strbuf_addstr(msgbuf, p);
181 }
182
184 -static void set_author_ident_env(const char *message)
185 -{
186 - const char *p = message;
187 - if (!p)
188 - die ("Could not read commit message of %s",
189 - sha1_to_hex(commit->object.sha1));
190 - while (*p && *p != '\n') {
191 - const char *eol;
192 -
193 - for (eol = p; *eol && *eol != '\n'; eol++)
194 - ; /* do nothing */
195 - if (!prefixcmp(p, "author ")) {
196 - char *line, *pend, *email, *timestamp;
197 -
198 - p += 7;
199 - line = xmemdupz(p, eol - p);
200 - email = strchr(line, '<');
201 - if (!email)
202 - die ("Could not extract author email from %s",
203 - sha1_to_hex(commit->object.sha1));
204 - if (email == line)
205 - pend = line;
206 - else
207 - for (pend = email; pend != line + 1 &&
208 - isspace(pend[-1]); pend--);
209 - ; /* do nothing */
210 - *pend = '\0';
211 - email++;
212 - timestamp = strchr(email, '>');
213 - if (!timestamp)
214 - die ("Could not extract author time from %s",
215 - sha1_to_hex(commit->object.sha1));
216 - *timestamp = '\0';
217 - for (timestamp++; *timestamp && isspace(*timestamp);
218 - timestamp++)
219 - ; /* do nothing */
220 - setenv("GIT_AUTHOR_NAME", line, 1);
221 - setenv("GIT_AUTHOR_EMAIL", email, 1);
222 - setenv("GIT_AUTHOR_DATE", timestamp, 1);
223 - free(line);
224 - return;
225 - }
226 - p = eol;
227 - if (*p == '\n')
228 - p++;
229 - }
230 - die ("No author information found in %s",
231 - sha1_to_hex(commit->object.sha1));
232 -}
233 -
183 static void write_cherry_pick_head(void)
184 {
185 int fd;
@@ -273,9 +222,7 @@ static void print_advice(void)
222
223 advise("after resolving the conflicts, mark the corrected paths");
224 advise("with 'git add <paths>' or 'git rm <paths>'");
276 -
277 - if (action == CHERRY_PICK)
278 - advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'");
225 + advise("and commit the result with 'git commit'");
226 }
227
228 static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -503,7 +450,6 @@ static int do_pick_commit(void)
450 base_label = msg.parent_label;
451 next = commit;
452 next_label = msg.label;
506 - set_author_ident_env(msg.message);
453 add_message_to_msg(&msgbuf, msg.message);
454 if (no_replay) {
455 strbuf_addstr(&msgbuf, "(cherry picked from commit ");
t/t3507-cherry-pick-conflict.sh
+1 -1
@@ -52,7 +52,7 @@ test_expect_success 'advice from failed cherry-pick' "
52 error: could not apply \$picked... picked
53 hint: after resolving the conflicts, mark the corrected paths
54 hint: with 'git add <paths>' or 'git rm <paths>'
55 - hint: and commit the result with 'git commit -c CHERRY_PICK_HEAD'
55 + hint: and commit the result with 'git commit'
56 EOF
57 test_must_fail git cherry-pick picked 2>actual &&
58
t/t7509-commit.sh
+29
@@ -157,4 +157,33 @@ test_expect_success '--reset-author should be rejected without -c/-C/--amend' '
157 test_must_fail git commit -a --reset-author -m done
158 '
159
160 +test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' '
161 + echo "cherry-pick 1a" >>foo &&
162 + test_tick &&
163 + git commit -am "cherry-pick 1" --author="Cherry <cherry@pick.er>" &&
164 + git tag cherry-pick-head &&
165 + git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
166 + echo "This is a MERGE_MSG" >.git/MERGE_MSG &&
167 + echo "cherry-pick 1b" >>foo &&
168 + test_tick &&
169 + git commit -a &&
170 + author_header cherry-pick-head >expect &&
171 + author_header HEAD >actual &&
172 + test_cmp expect actual &&
173 +
174 + echo "This is a MERGE_MSG" >expect &&
175 + message_body HEAD >actual &&
176 + test_cmp expect actual
177 +'
178 +
179 +test_expect_success '--reset-author with CHERRY_PICK_HEAD' '
180 + git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
181 + echo "cherry-pick 2" >>foo &&
182 + test_tick &&
183 + git commit -am "cherry-pick 2" --reset-author &&
184 + echo "author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
185 + author_header HEAD >actual &&
186 + test_cmp expect actual
187 +'
188 +
189 test_done
wt-status.c
+2 -2
@@ -60,7 +60,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
60 color_fprintf_ln(s->fp, c, "# Unmerged paths:");
61 if (!advice_status_hints)
62 return;
63 - if (s->in_merge)
63 + if (s->whence != FROM_COMMIT)
64 ;
65 else if (!s->is_initial)
66 color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
@@ -77,7 +77,7 @@ static void wt_status_print_cached_header(struct wt_status *s)
77 color_fprintf_ln(s->fp, c, "# Changes to be committed:");
78 if (!advice_status_hints)
79 return;
80 - if (s->in_merge)
80 + if (s->whence != FROM_COMMIT)
81 ; /* NEEDSWORK: use "git reset --unresolve"??? */
82 else if (!s->is_initial)
83 color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
wt-status.h
+8 -1
@@ -24,6 +24,13 @@ enum untracked_status_type {
24 SHOW_ALL_UNTRACKED_FILES
25 };
26
27 +/* from where does this commit originate */
28 +enum commit_whence {
29 + FROM_COMMIT, /* normal */
30 + FROM_MERGE, /* commit came from merge */
31 + FROM_CHERRY_PICK /* commit came from cherry-pick */
32 +};
33 +
34 struct wt_status_change_data {
35 int worktree_status;
36 int index_status;
@@ -40,7 +47,7 @@ struct wt_status {
47 const char **pathspec;
48 int verbose;
49 int amend;
43 - int in_merge;
50 + enum commit_whence whence;
51 int nowarn;
52 int use_color;
53 int relative_paths;