From d0aaa46fd3e53801346a4cadebf398f05d79780b Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 10 Nov 2017 11:09:42 +0000 Subject: commit: move empty message checks to libgit Move the functions that check for empty messages from bulitin/commit.c to sequencer.c so they can be shared with other commands. The functions are refactored to take an explicit cleanup mode and template filename passed by the caller. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'sequencer.h') diff --git a/sequencer.h b/sequencer.h index 6f3d3df82c..82e57713a2 100644 --- a/sequencer.h +++ b/sequencer.h @@ -58,4 +58,15 @@ extern const char sign_off_header[]; void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); void append_conflicts_hint(struct strbuf *msgbuf); +enum commit_msg_cleanup_mode { + COMMIT_MSG_CLEANUP_SPACE, + COMMIT_MSG_CLEANUP_NONE, + COMMIT_MSG_CLEANUP_SCISSORS, + COMMIT_MSG_CLEANUP_ALL +}; + +int message_is_empty(const struct strbuf *sb, + enum commit_msg_cleanup_mode cleanup_mode); +int template_untouched(const struct strbuf *sb, const char *template_file, + enum commit_msg_cleanup_mode cleanup_mode); #endif -- cgit v1.3-5-g9baa From 0505d604c9c5a361ee027d155c7d1facaf326863 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 17 Nov 2017 11:34:47 +0000 Subject: Add a function to update HEAD after creating a commit Add update_head_with_reflog() based on the code that updates HEAD after committing in builtin/commit.c that can be called by 'git commit' and other commands. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 20 ++------------------ sequencer.c | 39 ++++++++++++++++++++++++++++++++++++++- sequencer.h | 4 ++++ 3 files changed, 44 insertions(+), 19 deletions(-) (limited to 'sequencer.h') diff --git a/builtin/commit.c b/builtin/commit.c index dbc160c525..7c28144446 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1591,13 +1591,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) struct strbuf sb = STRBUF_INIT; struct strbuf author_ident = STRBUF_INIT; const char *index_file, *reflog_msg; - char *nl; struct object_id oid; struct commit_list *parents = NULL; struct stat statbuf; struct commit *current_head = NULL; struct commit_extra_header *extra = NULL; - struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; if (argc == 2 && !strcmp(argv[1], "-h")) @@ -1720,25 +1718,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) strbuf_release(&author_ident); free_commit_extra_headers(extra); - nl = strchr(sb.buf, '\n'); - if (nl) - strbuf_setlen(&sb, nl + 1 - sb.buf); - else - strbuf_addch(&sb, '\n'); - strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg)); - strbuf_insert(&sb, strlen(reflog_msg), ": ", 2); - - transaction = ref_transaction_begin(&err); - if (!transaction || - ref_transaction_update(transaction, "HEAD", &oid, - current_head - ? ¤t_head->object.oid : &null_oid, - 0, sb.buf, &err) || - ref_transaction_commit(transaction, &err)) { + if (update_head_with_reflog(current_head, &oid, reflog_msg, &sb, + &err)) { rollback_index_files(); die("%s", err.buf); } - ref_transaction_free(transaction); unlink(git_path_cherry_pick_head()); unlink(git_path_revert_head()); diff --git a/sequencer.c b/sequencer.c index 23c250f16c..fcd8e92531 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1,10 +1,10 @@ #include "cache.h" #include "config.h" #include "lockfile.h" -#include "sequencer.h" #include "dir.h" #include "object.h" #include "commit.h" +#include "sequencer.h" #include "tag.h" #include "run-command.h" #include "exec_cmd.h" @@ -752,6 +752,43 @@ int template_untouched(const struct strbuf *sb, const char *template_file, return rest_is_empty(sb, start - sb->buf); } +int update_head_with_reflog(const struct commit *old_head, + const struct object_id *new_head, + const char *action, const struct strbuf *msg, + struct strbuf *err) +{ + struct ref_transaction *transaction; + struct strbuf sb = STRBUF_INIT; + const char *nl; + int ret = 0; + + if (action) { + strbuf_addstr(&sb, action); + strbuf_addstr(&sb, ": "); + } + + nl = strchr(msg->buf, '\n'); + if (nl) { + strbuf_add(&sb, msg->buf, nl + 1 - msg->buf); + } else { + strbuf_addbuf(&sb, msg); + strbuf_addch(&sb, '\n'); + } + + transaction = ref_transaction_begin(err); + if (!transaction || + ref_transaction_update(transaction, "HEAD", new_head, + old_head ? &old_head->object.oid : &null_oid, + 0, sb.buf, err) || + ref_transaction_commit(transaction, err)) { + ret = -1; + } + ref_transaction_free(transaction); + strbuf_release(&sb); + + return ret; +} + static int is_original_commit_empty(struct commit *commit) { const struct object_id *ptree_oid; diff --git a/sequencer.h b/sequencer.h index 82e57713a2..81a2098e90 100644 --- a/sequencer.h +++ b/sequencer.h @@ -69,4 +69,8 @@ int message_is_empty(const struct strbuf *sb, enum commit_msg_cleanup_mode cleanup_mode); int template_untouched(const struct strbuf *sb, const char *template_file, enum commit_msg_cleanup_mode cleanup_mode); +int update_head_with_reflog(const struct commit *old_head, + const struct object_id *new_head, + const char *action, const struct strbuf *msg, + struct strbuf *err); #endif -- cgit v1.3-5-g9baa From a87a6f3c98ea80740fa31d2559b78f75f8138132 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 17 Nov 2017 11:34:48 +0000 Subject: commit: move post-rewrite code to libgit Move run_rewrite_hook() from bulitin/commit.c to sequencer.c so it can be shared with other commands and add a new function commit_post_rewrite() based on the code in builtin/commit.c that encapsulates rewriting notes and running the post-rewrite hook. Once the sequencer learns how to create commits without forking 'git commit' these functions will be used when squashing commits. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 42 +----------------------------------------- sequencer.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ sequencer.h | 2 ++ 3 files changed, 50 insertions(+), 41 deletions(-) (limited to 'sequencer.h') diff --git a/builtin/commit.c b/builtin/commit.c index 7c28144446..3bc5dff2c0 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -31,9 +31,7 @@ #include "gpg-interface.h" #include "column.h" #include "sequencer.h" -#include "notes-utils.h" #include "mailmap.h" -#include "sigchain.h" static const char * const builtin_commit_usage[] = { N_("git commit [] [--] ..."), @@ -1478,37 +1476,6 @@ static int git_commit_config(const char *k, const char *v, void *cb) return git_status_config(k, v, s); } -static int run_rewrite_hook(const struct object_id *oldoid, - const struct object_id *newoid) -{ - struct child_process proc = CHILD_PROCESS_INIT; - const char *argv[3]; - int code; - struct strbuf sb = STRBUF_INIT; - - argv[0] = find_hook("post-rewrite"); - if (!argv[0]) - return 0; - - argv[1] = "amend"; - argv[2] = NULL; - - proc.argv = argv; - proc.in = -1; - proc.stdout_to_stderr = 1; - - code = start_command(&proc); - if (code) - return code; - strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid)); - sigchain_push(SIGPIPE, SIG_IGN); - write_in_full(proc.in, sb.buf, sb.len); - close(proc.in); - strbuf_release(&sb); - sigchain_pop(SIGPIPE); - return finish_command(&proc); -} - int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...) { struct argv_array hook_env = ARGV_ARRAY_INIT; @@ -1739,14 +1706,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) rerere(0); run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); if (amend && !no_post_rewrite) { - struct notes_rewrite_cfg *cfg; - cfg = init_copy_notes_for_rewrite("amend"); - if (cfg) { - /* we are amending, so current_head is not NULL */ - copy_note_for_rewrite(cfg, ¤t_head->object.oid, &oid); - finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'"); - } - run_rewrite_hook(¤t_head->object.oid, &oid); + commit_post_rewrite(current_head, &oid); } if (!quiet) print_summary(prefix, &oid, !current_head); diff --git a/sequencer.c b/sequencer.c index fcd8e92531..5529e5df1c 100644 --- a/sequencer.c +++ b/sequencer.c @@ -21,6 +21,8 @@ #include "log-tree.h" #include "wt-status.h" #include "hashmap.h" +#include "notes-utils.h" +#include "sigchain.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -789,6 +791,51 @@ int update_head_with_reflog(const struct commit *old_head, return ret; } +static int run_rewrite_hook(const struct object_id *oldoid, + const struct object_id *newoid) +{ + struct child_process proc = CHILD_PROCESS_INIT; + const char *argv[3]; + int code; + struct strbuf sb = STRBUF_INIT; + + argv[0] = find_hook("post-rewrite"); + if (!argv[0]) + return 0; + + argv[1] = "amend"; + argv[2] = NULL; + + proc.argv = argv; + proc.in = -1; + proc.stdout_to_stderr = 1; + + code = start_command(&proc); + if (code) + return code; + strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid)); + sigchain_push(SIGPIPE, SIG_IGN); + write_in_full(proc.in, sb.buf, sb.len); + close(proc.in); + strbuf_release(&sb); + sigchain_pop(SIGPIPE); + return finish_command(&proc); +} + +void commit_post_rewrite(const struct commit *old_head, + const struct object_id *new_head) +{ + struct notes_rewrite_cfg *cfg; + + cfg = init_copy_notes_for_rewrite("amend"); + if (cfg) { + /* we are amending, so old_head is not NULL */ + copy_note_for_rewrite(cfg, &old_head->object.oid, new_head); + finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'"); + } + run_rewrite_hook(&old_head->object.oid, new_head); +} + static int is_original_commit_empty(struct commit *commit) { const struct object_id *ptree_oid; diff --git a/sequencer.h b/sequencer.h index 81a2098e90..ec13b679c4 100644 --- a/sequencer.h +++ b/sequencer.h @@ -73,4 +73,6 @@ int update_head_with_reflog(const struct commit *old_head, const struct object_id *new_head, const char *action, const struct strbuf *msg, struct strbuf *err); +void commit_post_rewrite(const struct commit *current_head, + const struct object_id *new_head); #endif -- cgit v1.3-5-g9baa From e47c6cafcb5a2223ea3de3d0b65f668f717cb2ab Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 24 Nov 2017 11:07:54 +0000 Subject: commit: move print_commit_summary() to libgit Move print_commit_summary() from builtin/commit.c to sequencer.c so it can be shared with other commands. The function is modified by changing the last argument to a flag so callers can specify whether they want to show the author date in addition to specifying if this is an initial commit. If the sequencer dies in print_commit_summary() (which can only happen when cherry-picking or reverting) then neither the todo list nor the abort safety file are updated to reflect the commit that was just made. print_commit_summary() can die if: - The commit that was just created cannot be found or parsed. - HEAD cannot be resolved either because some other process is updating it (which is bad news in the middle of a cherry-pick) or because it is corrupt. - log_tree_commit() cannot read some objects. In all those cases dying will leave the sequencer in a sane state for aborting; 'git cherry-pick --abort' will rewind HEAD to the last successful commit before there was a problem with HEAD or the object database. If the user somehow fixes the problem and runs 'git cherry-pick --continue' then the sequencer will try and pick the same commit again which may or may not be what the user wants depending on what caused print_commit_summary() to die. If print_commit_summary() returned an error instead then update_abort_safety_file() would try to resolve HEAD which may or may not be successful. If it is successful then running 'git rebase --abort' would not rewind HEAD to the last successful commit which is not what we want. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 128 ++++--------------------------------------------------- sequencer.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++ sequencer.h | 5 +++ 3 files changed, 133 insertions(+), 119 deletions(-) (limited to 'sequencer.h') diff --git a/builtin/commit.c b/builtin/commit.c index 3bc5dff2c0..22d260197e 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -43,31 +43,6 @@ static const char * const builtin_status_usage[] = { NULL }; -static const char implicit_ident_advice_noconfig[] = -N_("Your name and email address were configured automatically based\n" -"on your username and hostname. Please check that they are accurate.\n" -"You can suppress this message by setting them explicitly. Run the\n" -"following command and follow the instructions in your editor to edit\n" -"your configuration file:\n" -"\n" -" git config --global --edit\n" -"\n" -"After doing this, you may fix the identity used for this commit with:\n" -"\n" -" git commit --amend --reset-author\n"); - -static const char implicit_ident_advice_config[] = -N_("Your name and email address were configured automatically based\n" -"on your username and hostname. Please check that they are accurate.\n" -"You can suppress this message by setting them explicitly:\n" -"\n" -" git config --global user.name \"Your Name\"\n" -" git config --global user.email you@example.com\n" -"\n" -"After doing this, you may fix the identity used for this commit with:\n" -"\n" -" git commit --amend --reset-author\n"); - static const char empty_amend_advice[] = N_("You asked to amend the most recent commit, but doing so would make\n" "it empty. You can repeat your command with --allow-empty, or you can\n" @@ -1355,98 +1330,6 @@ int cmd_status(int argc, const char **argv, const char *prefix) return 0; } -static const char *implicit_ident_advice(void) -{ - char *user_config = expand_user_path("~/.gitconfig", 0); - char *xdg_config = xdg_config_home("config"); - int config_exists = file_exists(user_config) || file_exists(xdg_config); - - free(user_config); - free(xdg_config); - - if (config_exists) - return _(implicit_ident_advice_config); - else - return _(implicit_ident_advice_noconfig); - -} - -static void print_summary(const char *prefix, const struct object_id *oid, - int initial_commit) -{ - struct rev_info rev; - struct commit *commit; - struct strbuf format = STRBUF_INIT; - const char *head; - struct pretty_print_context pctx = {0}; - struct strbuf author_ident = STRBUF_INIT; - struct strbuf committer_ident = STRBUF_INIT; - - commit = lookup_commit(oid); - if (!commit) - die(_("couldn't look up newly created commit")); - if (parse_commit(commit)) - die(_("could not parse newly created commit")); - - strbuf_addstr(&format, "format:%h] %s"); - - format_commit_message(commit, "%an <%ae>", &author_ident, &pctx); - format_commit_message(commit, "%cn <%ce>", &committer_ident, &pctx); - if (strbuf_cmp(&author_ident, &committer_ident)) { - strbuf_addstr(&format, "\n Author: "); - strbuf_addbuf_percentquote(&format, &author_ident); - } - if (author_date_is_interesting()) { - struct strbuf date = STRBUF_INIT; - format_commit_message(commit, "%ad", &date, &pctx); - strbuf_addstr(&format, "\n Date: "); - strbuf_addbuf_percentquote(&format, &date); - strbuf_release(&date); - } - if (!committer_ident_sufficiently_given()) { - strbuf_addstr(&format, "\n Committer: "); - strbuf_addbuf_percentquote(&format, &committer_ident); - if (advice_implicit_identity) { - strbuf_addch(&format, '\n'); - strbuf_addstr(&format, implicit_ident_advice()); - } - } - strbuf_release(&author_ident); - strbuf_release(&committer_ident); - - init_revisions(&rev, prefix); - setup_revisions(0, NULL, &rev, NULL); - - rev.diff = 1; - rev.diffopt.output_format = - DIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY; - - rev.verbose_header = 1; - rev.show_root_diff = 1; - get_commit_format(format.buf, &rev); - rev.always_show_header = 0; - rev.diffopt.detect_rename = 1; - rev.diffopt.break_opt = 0; - diff_setup_done(&rev.diffopt); - - head = resolve_ref_unsafe("HEAD", 0, NULL, NULL); - if (!head) - die_errno(_("unable to resolve HEAD after creating commit")); - if (!strcmp(head, "HEAD")) - head = _("detached HEAD"); - else - skip_prefix(head, "refs/heads/", &head); - printf("[%s%s ", head, initial_commit ? _(" (root-commit)") : ""); - - if (!log_tree_commit(&rev, commit)) { - rev.always_show_header = 1; - rev.use_terminator = 1; - log_tree_commit(&rev, commit); - } - - strbuf_release(&format); -} - static int git_commit_config(const char *k, const char *v, void *cb) { struct wt_status *s = cb; @@ -1708,8 +1591,15 @@ int cmd_commit(int argc, const char **argv, const char *prefix) if (amend && !no_post_rewrite) { commit_post_rewrite(current_head, &oid); } - if (!quiet) - print_summary(prefix, &oid, !current_head); + if (!quiet) { + unsigned int flags = 0; + + if (!current_head) + flags |= SUMMARY_INITIAL_COMMIT; + if (author_date_is_interesting()) + flags |= SUMMARY_SHOW_AUTHOR_DATE; + print_commit_summary(prefix, &oid, flags); + } UNLEAK(err); UNLEAK(sb); diff --git a/sequencer.c b/sequencer.c index 5529e5df1c..334348f0d4 100644 --- a/sequencer.c +++ b/sequencer.c @@ -836,6 +836,125 @@ void commit_post_rewrite(const struct commit *old_head, run_rewrite_hook(&old_head->object.oid, new_head); } +static const char implicit_ident_advice_noconfig[] = +N_("Your name and email address were configured automatically based\n" +"on your username and hostname. Please check that they are accurate.\n" +"You can suppress this message by setting them explicitly. Run the\n" +"following command and follow the instructions in your editor to edit\n" +"your configuration file:\n" +"\n" +" git config --global --edit\n" +"\n" +"After doing this, you may fix the identity used for this commit with:\n" +"\n" +" git commit --amend --reset-author\n"); + +static const char implicit_ident_advice_config[] = +N_("Your name and email address were configured automatically based\n" +"on your username and hostname. Please check that they are accurate.\n" +"You can suppress this message by setting them explicitly:\n" +"\n" +" git config --global user.name \"Your Name\"\n" +" git config --global user.email you@example.com\n" +"\n" +"After doing this, you may fix the identity used for this commit with:\n" +"\n" +" git commit --amend --reset-author\n"); + +static const char *implicit_ident_advice(void) +{ + char *user_config = expand_user_path("~/.gitconfig", 0); + char *xdg_config = xdg_config_home("config"); + int config_exists = file_exists(user_config) || file_exists(xdg_config); + + free(user_config); + free(xdg_config); + + if (config_exists) + return _(implicit_ident_advice_config); + else + return _(implicit_ident_advice_noconfig); + +} + +void print_commit_summary(const char *prefix, const struct object_id *oid, + unsigned int flags) +{ + struct rev_info rev; + struct commit *commit; + struct strbuf format = STRBUF_INIT; + const char *head; + struct pretty_print_context pctx = {0}; + struct strbuf author_ident = STRBUF_INIT; + struct strbuf committer_ident = STRBUF_INIT; + + commit = lookup_commit(oid); + if (!commit) + die(_("couldn't look up newly created commit")); + if (parse_commit(commit)) + die(_("could not parse newly created commit")); + + strbuf_addstr(&format, "format:%h] %s"); + + format_commit_message(commit, "%an <%ae>", &author_ident, &pctx); + format_commit_message(commit, "%cn <%ce>", &committer_ident, &pctx); + if (strbuf_cmp(&author_ident, &committer_ident)) { + strbuf_addstr(&format, "\n Author: "); + strbuf_addbuf_percentquote(&format, &author_ident); + } + if (flags & SUMMARY_SHOW_AUTHOR_DATE) { + struct strbuf date = STRBUF_INIT; + + format_commit_message(commit, "%ad", &date, &pctx); + strbuf_addstr(&format, "\n Date: "); + strbuf_addbuf_percentquote(&format, &date); + strbuf_release(&date); + } + if (!committer_ident_sufficiently_given()) { + strbuf_addstr(&format, "\n Committer: "); + strbuf_addbuf_percentquote(&format, &committer_ident); + if (advice_implicit_identity) { + strbuf_addch(&format, '\n'); + strbuf_addstr(&format, implicit_ident_advice()); + } + } + strbuf_release(&author_ident); + strbuf_release(&committer_ident); + + init_revisions(&rev, prefix); + setup_revisions(0, NULL, &rev, NULL); + + rev.diff = 1; + rev.diffopt.output_format = + DIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY; + + rev.verbose_header = 1; + rev.show_root_diff = 1; + get_commit_format(format.buf, &rev); + rev.always_show_header = 0; + rev.diffopt.detect_rename = 1; + rev.diffopt.break_opt = 0; + diff_setup_done(&rev.diffopt); + + head = resolve_ref_unsafe("HEAD", 0, NULL, NULL); + if (!head) + die_errno(_("unable to resolve HEAD after creating commit")); + if (!strcmp(head, "HEAD")) + head = _("detached HEAD"); + else + skip_prefix(head, "refs/heads/", &head); + printf("[%s%s ", head, (flags & SUMMARY_INITIAL_COMMIT) ? + _(" (root-commit)") : ""); + + if (!log_tree_commit(&rev, commit)) { + rev.always_show_header = 1; + rev.use_terminator = 1; + log_tree_commit(&rev, commit); + } + + strbuf_release(&format); +} + static int is_original_commit_empty(struct commit *commit) { const struct object_id *ptree_oid; diff --git a/sequencer.h b/sequencer.h index ec13b679c4..4f616c61a3 100644 --- a/sequencer.h +++ b/sequencer.h @@ -75,4 +75,9 @@ int update_head_with_reflog(const struct commit *old_head, struct strbuf *err); void commit_post_rewrite(const struct commit *current_head, const struct object_id *new_head); + +#define SUMMARY_INITIAL_COMMIT (1 << 0) +#define SUMMARY_SHOW_AUTHOR_DATE (1 << 1) +void print_commit_summary(const char *prefix, const struct object_id *oid, + unsigned int flags); #endif -- cgit v1.3-5-g9baa From b36c5908135889bd9c48a8d44d4e07f59bf799ef Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 24 Nov 2017 11:07:56 +0000 Subject: sequencer: load commit related config Load default values for message cleanup and gpg signing of commits in preparation for committing without forking 'git commit'. Note that we interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE to be consistent with 'git commit' Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/rebase--helper.c | 13 ++++++++++++- builtin/revert.c | 15 +++++++++++++-- sequencer.c | 34 ++++++++++++++++++++++++++++++++++ sequencer.h | 1 + 4 files changed, 60 insertions(+), 3 deletions(-) (limited to 'sequencer.h') diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c index f8519363a3..68194d3aed 100644 --- a/builtin/rebase--helper.c +++ b/builtin/rebase--helper.c @@ -9,6 +9,17 @@ static const char * const builtin_rebase_helper_usage[] = { NULL }; +static int git_rebase_helper_config(const char *k, const char *v, void *cb) +{ + int status; + + status = git_sequencer_config(k, v, NULL); + if (status) + return status; + + return git_default_config(k, v, NULL); +} + int cmd_rebase__helper(int argc, const char **argv, const char *prefix) { struct replay_opts opts = REPLAY_OPTS_INIT; @@ -39,7 +50,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix) OPT_END() }; - git_config(git_default_config, NULL); + git_config(git_rebase_helper_config, NULL); opts.action = REPLAY_INTERACTIVE_REBASE; opts.allow_ff = 1; diff --git a/builtin/revert.c b/builtin/revert.c index b9d927eb09..1938825efa 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -31,6 +31,17 @@ static const char * const cherry_pick_usage[] = { NULL }; +static int common_config(const char *k, const char *v, void *cb) +{ + int status; + + status = git_sequencer_config(k, v, NULL); + if (status) + return status; + + return git_default_config(k, v, NULL); +} + static const char *action_name(const struct replay_opts *opts) { return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick"; @@ -208,7 +219,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix) if (isatty(0)) opts.edit = 1; opts.action = REPLAY_REVERT; - git_config(git_default_config, NULL); + git_config(common_config, NULL); res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("revert failed")); @@ -221,7 +232,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix) int res; opts.action = REPLAY_PICK; - git_config(git_default_config, NULL); + git_config(common_config, NULL); res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("cherry-pick failed")); diff --git a/sequencer.c b/sequencer.c index a9e0ad49fb..22392d954b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -688,6 +688,40 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, return run_command(&cmd); } +static enum commit_msg_cleanup_mode default_msg_cleanup = + COMMIT_MSG_CLEANUP_NONE; +static char *default_gpg_sign; + +int git_sequencer_config(const char *k, const char *v, void *cb) +{ + if (!strcmp(k, "commit.cleanup")) { + int status; + const char *s; + + status = git_config_string(&s, k, v); + if (status) + return status; + + if (!strcmp(s, "verbatim")) + default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE; + else if (!strcmp(s, "whitespace")) + default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE; + else if (!strcmp(s, "strip")) + default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL; + else if (!strcmp(s, "scissors")) + default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE; + + return status; + } + + if (!strcmp(k, "commit.gpgsign")) { + default_gpg_sign = git_config_bool(k, v) ? "" : NULL; + return 0; + } + + return git_gpg_config(k, v, NULL); +} + static int rest_is_empty(const struct strbuf *sb, int start) { int i, eol; diff --git a/sequencer.h b/sequencer.h index 4f616c61a3..77cb174b2a 100644 --- a/sequencer.h +++ b/sequencer.h @@ -57,6 +57,7 @@ extern const char sign_off_header[]; void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); void append_conflicts_hint(struct strbuf *msgbuf); +int git_sequencer_config(const char *k, const char *v, void *cb); enum commit_msg_cleanup_mode { COMMIT_MSG_CLEANUP_SPACE, -- cgit v1.3-5-g9baa From 28d6daed4f119940ace31e523b3b272d3d153d04 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 13 Dec 2017 11:46:21 +0000 Subject: sequencer: improve config handling The previous config handling relied on global variables, called git_default_config() even when the key had already been handled by git_sequencer_config() and did not initialize the diff configuration variables. Improve this by: i) loading the default values for message cleanup and gpg signing of commits into struct replay_opts; ii) restructuring the code to return immediately once a key is handled; and iii) calling git_diff_basic_config(). Note that unfortunately it is not possible to return early if the key is handled by git_gpg_config() as it does not indicate to the caller if the key has been handled or not. The sequencer should probably have been calling git_diff_basic_config() before as it creates a patch when there are conflicts. The shell version uses 'diff-tree' to create the patch so calling git_diff_basic_config() should match that. Although 'git commit' calls git_diff_ui_config() I don't think the output of print_commit_summary() is affected by anything that is loaded by that as print_commit_summary() always turns on rename detection so would ignore the value in the user's configuration anyway. The other values loaded by git_diff_ui_config() are about the formatting of patches so are not relevant to print_commit_summary(). Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/rebase--helper.c | 13 +------- builtin/revert.c | 15 ++------- sequencer.c | 87 ++++++++++++++++++++++++++---------------------- sequencer.h | 19 ++++++----- 4 files changed, 61 insertions(+), 73 deletions(-) (limited to 'sequencer.h') diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c index 68194d3aed..decb8f7a09 100644 --- a/builtin/rebase--helper.c +++ b/builtin/rebase--helper.c @@ -9,17 +9,6 @@ static const char * const builtin_rebase_helper_usage[] = { NULL }; -static int git_rebase_helper_config(const char *k, const char *v, void *cb) -{ - int status; - - status = git_sequencer_config(k, v, NULL); - if (status) - return status; - - return git_default_config(k, v, NULL); -} - int cmd_rebase__helper(int argc, const char **argv, const char *prefix) { struct replay_opts opts = REPLAY_OPTS_INIT; @@ -50,7 +39,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix) OPT_END() }; - git_config(git_rebase_helper_config, NULL); + sequencer_init_config(&opts); opts.action = REPLAY_INTERACTIVE_REBASE; opts.allow_ff = 1; diff --git a/builtin/revert.c b/builtin/revert.c index 1938825efa..76f0a35b07 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -31,17 +31,6 @@ static const char * const cherry_pick_usage[] = { NULL }; -static int common_config(const char *k, const char *v, void *cb) -{ - int status; - - status = git_sequencer_config(k, v, NULL); - if (status) - return status; - - return git_default_config(k, v, NULL); -} - static const char *action_name(const struct replay_opts *opts) { return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick"; @@ -219,7 +208,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix) if (isatty(0)) opts.edit = 1; opts.action = REPLAY_REVERT; - git_config(common_config, NULL); + sequencer_init_config(&opts); res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("revert failed")); @@ -232,7 +221,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix) int res; opts.action = REPLAY_PICK; - git_config(common_config, NULL); + sequencer_init_config(&opts); res = run_sequencer(argc, argv, &opts); if (res < 0) die(_("cherry-pick failed")); diff --git a/sequencer.c b/sequencer.c index 99452a0e89..7051b20b76 100644 --- a/sequencer.c +++ b/sequencer.c @@ -132,6 +132,51 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy") static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts") static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate") +static int git_sequencer_config(const char *k, const char *v, void *cb) +{ + struct replay_opts *opts = cb; + int status; + + if (!strcmp(k, "commit.cleanup")) { + const char *s; + + status = git_config_string(&s, k, v); + if (status) + return status; + + if (!strcmp(s, "verbatim")) + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE; + else if (!strcmp(s, "whitespace")) + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE; + else if (!strcmp(s, "strip")) + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL; + else if (!strcmp(s, "scissors")) + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE; + else + warning(_("invalid commit message cleanup mode '%s'"), + s); + + return status; + } + + if (!strcmp(k, "commit.gpgsign")) { + opts->gpg_sign = git_config_bool(k, v) ? "" : NULL; + return 0; + } + + status = git_gpg_config(k, v, NULL); + if (status) + return status; + + return git_diff_basic_config(k, v, NULL); +} + +void sequencer_init_config(struct replay_opts *opts) +{ + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE; + git_config(git_sequencer_config, opts); +} + static inline int is_rebase_i(const struct replay_opts *opts) { return opts->action == REPLAY_INTERACTIVE_REBASE; @@ -700,40 +745,6 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, return run_command(&cmd); } -static enum commit_msg_cleanup_mode default_msg_cleanup = - COMMIT_MSG_CLEANUP_NONE; -static char *default_gpg_sign; - -int git_sequencer_config(const char *k, const char *v, void *cb) -{ - if (!strcmp(k, "commit.cleanup")) { - int status; - const char *s; - - status = git_config_string(&s, k, v); - if (status) - return status; - - if (!strcmp(s, "verbatim")) - default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE; - else if (!strcmp(s, "whitespace")) - default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE; - else if (!strcmp(s, "strip")) - default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL; - else if (!strcmp(s, "scissors")) - default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE; - - return status; - } - - if (!strcmp(k, "commit.gpgsign")) { - default_gpg_sign = git_config_bool(k, v) ? "" : NULL; - return 0; - } - - return git_gpg_config(k, v, NULL); -} - static int rest_is_empty(const struct strbuf *sb, int start) { int i, eol; @@ -1039,7 +1050,6 @@ static int try_to_commit(struct strbuf *msg, const char *author, struct strbuf err = STRBUF_INIT; struct strbuf amend_msg = STRBUF_INIT; char *amend_author = NULL; - const char *gpg_sign; enum commit_msg_cleanup_mode cleanup; int res = 0; @@ -1072,7 +1082,8 @@ static int try_to_commit(struct strbuf *msg, const char *author, } cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL : - default_msg_cleanup; + opts->default_msg_cleanup; + if (cleanup != COMMIT_MSG_CLEANUP_NONE) strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL); if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) { @@ -1080,8 +1091,6 @@ static int try_to_commit(struct strbuf *msg, const char *author, goto out; } - gpg_sign = opts->gpg_sign ? opts->gpg_sign : default_gpg_sign; - if (write_cache_as_tree(tree.hash, 0, NULL)) { res = error(_("git write-tree failed to write a tree")); goto out; @@ -1095,7 +1104,7 @@ static int try_to_commit(struct strbuf *msg, const char *author, } if (commit_tree_extended(msg->buf, msg->len, tree.hash, parents, - oid->hash, author, gpg_sign, extra)) { + oid->hash, author, opts->gpg_sign, extra)) { res = error(_("failed to write commit object")); goto out; } diff --git a/sequencer.h b/sequencer.h index 77cb174b2a..3a5072c2ab 100644 --- a/sequencer.h +++ b/sequencer.h @@ -11,6 +11,13 @@ enum replay_action { REPLAY_INTERACTIVE_REBASE }; +enum commit_msg_cleanup_mode { + COMMIT_MSG_CLEANUP_SPACE, + COMMIT_MSG_CLEANUP_NONE, + COMMIT_MSG_CLEANUP_SCISSORS, + COMMIT_MSG_CLEANUP_ALL +}; + struct replay_opts { enum replay_action action; @@ -29,6 +36,7 @@ struct replay_opts { int mainline; char *gpg_sign; + enum commit_msg_cleanup_mode default_msg_cleanup; /* Merge strategy */ char *strategy; @@ -40,6 +48,8 @@ struct replay_opts { }; #define REPLAY_OPTS_INIT { -1 } +/* Call this to setup defaults before parsing command line options */ +void sequencer_init_config(struct replay_opts *opts); int sequencer_pick_revisions(struct replay_opts *opts); int sequencer_continue(struct replay_opts *opts); int sequencer_rollback(struct replay_opts *opts); @@ -57,15 +67,6 @@ extern const char sign_off_header[]; void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag); void append_conflicts_hint(struct strbuf *msgbuf); -int git_sequencer_config(const char *k, const char *v, void *cb); - -enum commit_msg_cleanup_mode { - COMMIT_MSG_CLEANUP_SPACE, - COMMIT_MSG_CLEANUP_NONE, - COMMIT_MSG_CLEANUP_SCISSORS, - COMMIT_MSG_CLEANUP_ALL -}; - int message_is_empty(const struct strbuf *sb, enum commit_msg_cleanup_mode cleanup_mode); int template_untouched(const struct strbuf *sb, const char *template_file, -- cgit v1.3-5-g9baa From 66618a50f9c9f008d7aef751418f12ba9bfc6b85 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 24 Jan 2018 12:34:22 +0000 Subject: sequencer: run 'prepare-commit-msg' hook Commit 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24) forgot to run the 'prepare-commit-msg' hook when creating the commit. Fix this by writing the commit message to a different file and running the hook. Using a different file means that if the commit is cancelled the original message file is unchanged. Also move the checks for an empty commit so the order matches 'git commit'. Reported-by: Dmitry Torokhov Helped-by: Ramsay Jones Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 -- sequencer.c | 69 +++++++++++++++++++++++++++++++------- sequencer.h | 1 + t/t7505-prepare-commit-msg-hook.sh | 8 ++--- 4 files changed, 61 insertions(+), 19 deletions(-) (limited to 'sequencer.h') diff --git a/builtin/commit.c b/builtin/commit.c index 22d260197e..9f97ff8b98 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -66,8 +66,6 @@ N_("If you wish to skip this commit, use:\n" "Then \"git cherry-pick --continue\" will resume cherry-picking\n" "the remaining commits.\n"); -static GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG") - static const char *use_message_buffer; static struct lock_file index_lock; /* real index */ static struct lock_file false_lock; /* used only for partial commits */ diff --git a/sequencer.c b/sequencer.c index 1b2599668f..86840b4bc1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -29,6 +29,8 @@ const char sign_off_header[] = "Signed-off-by: "; static const char cherry_picked_prefix[] = "(cherry picked from commit "; +GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG") + GIT_PATH_FUNC(git_path_seq_dir, "sequencer") static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo") @@ -888,6 +890,31 @@ void commit_post_rewrite(const struct commit *old_head, run_rewrite_hook(&old_head->object.oid, new_head); } +static int run_prepare_commit_msg_hook(struct strbuf *msg, const char *commit) +{ + struct argv_array hook_env = ARGV_ARRAY_INIT; + int ret; + const char *name; + + name = git_path_commit_editmsg(); + if (write_message(msg->buf, msg->len, name, 0)) + return -1; + + argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", get_index_file()); + argv_array_push(&hook_env, "GIT_EDITOR=:"); + if (commit) + ret = run_hook_le(hook_env.argv, "prepare-commit-msg", name, + "commit", commit, NULL); + else + ret = run_hook_le(hook_env.argv, "prepare-commit-msg", name, + "message", NULL); + if (ret) + ret = error(_("'prepare-commit-msg' hook failed")); + argv_array_clear(&hook_env); + + return ret; +} + static const char implicit_ident_advice_noconfig[] = N_("Your name and email address were configured automatically based\n" "on your username and hostname. Please check that they are accurate.\n" @@ -1048,8 +1075,9 @@ static int try_to_commit(struct strbuf *msg, const char *author, struct commit_list *parents = NULL; struct commit_extra_header *extra = NULL; struct strbuf err = STRBUF_INIT; - struct strbuf amend_msg = STRBUF_INIT; + struct strbuf commit_msg = STRBUF_INIT; char *amend_author = NULL; + const char *hook_commit = NULL; enum commit_msg_cleanup_mode cleanup; int res = 0; @@ -1066,8 +1094,9 @@ static int try_to_commit(struct strbuf *msg, const char *author, const char *orig_message = NULL; find_commit_subject(message, &orig_message); - msg = &amend_msg; + msg = &commit_msg; strbuf_addstr(msg, orig_message); + hook_commit = "HEAD"; } author = amend_author = get_author(message); unuse_commit_buffer(current_head, message); @@ -1081,16 +1110,6 @@ static int try_to_commit(struct strbuf *msg, const char *author, commit_list_insert(current_head, &parents); } - cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL : - opts->default_msg_cleanup; - - if (cleanup != COMMIT_MSG_CLEANUP_NONE) - strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL); - if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) { - res = 1; /* run 'git commit' to display error message */ - goto out; - } - if (write_cache_as_tree(tree.hash, 0, NULL)) { res = error(_("git write-tree failed to write a tree")); goto out; @@ -1103,6 +1122,30 @@ static int try_to_commit(struct strbuf *msg, const char *author, goto out; } + if (find_hook("prepare-commit-msg")) { + res = run_prepare_commit_msg_hook(msg, hook_commit); + if (res) + goto out; + if (strbuf_read_file(&commit_msg, git_path_commit_editmsg(), + 2048) < 0) { + res = error_errno(_("unable to read commit message " + "from '%s'"), + git_path_commit_editmsg()); + goto out; + } + msg = &commit_msg; + } + + cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL : + opts->default_msg_cleanup; + + if (cleanup != COMMIT_MSG_CLEANUP_NONE) + strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL); + if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) { + res = 1; /* run 'git commit' to display error message */ + goto out; + } + if (commit_tree_extended(msg->buf, msg->len, tree.hash, parents, oid->hash, author, opts->gpg_sign, extra)) { res = error(_("failed to write commit object")); @@ -1121,7 +1164,7 @@ static int try_to_commit(struct strbuf *msg, const char *author, out: free_commit_extra_headers(extra); strbuf_release(&err); - strbuf_release(&amend_msg); + strbuf_release(&commit_msg); free(amend_author); return res; diff --git a/sequencer.h b/sequencer.h index 3a5072c2ab..96c69482ad 100644 --- a/sequencer.h +++ b/sequencer.h @@ -1,6 +1,7 @@ #ifndef SEQUENCER_H #define SEQUENCER_H +const char *git_path_commit_editmsg(void); const char *git_path_seq_dir(void); #define APPEND_SIGNOFF_DEDUP (1u << 0) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index c1f68cbcbf..1f43b3cd4c 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -252,10 +252,10 @@ test_rebase () { ' } -test_rebase failure -i -test_rebase failure -p +test_rebase success -i +test_rebase success -p -test_expect_failure 'with hook (cherry-pick)' ' +test_expect_success 'with hook (cherry-pick)' ' test_when_finished "git checkout -f master" && git checkout -B other b && git cherry-pick rebase-1 && @@ -310,7 +310,7 @@ test_expect_success 'with failing hook (merge)' ' ' -test_expect_failure C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' ' +test_expect_success C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' ' test_when_finished "git checkout -f master" && git checkout -B other b && test_must_fail git cherry-pick rebase-1 2>actual && -- cgit v1.3-5-g9baa