From 49697cb72122cf84b44111124821c9a4bcba3ab6 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 15 Oct 2019 10:25:31 +0000 Subject: move run_commit_hook() to libgit and use it there This function was declared in commit.h but was implemented in builtin/commit.c so was not part of libgit. Move it to libgit so we can use it in the sequencer. This simplifies the implementation of run_prepare_commit_msg_hook() and will be used in the next commit. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 2adcf5a639..cdc0d1dfba 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1127,25 +1127,22 @@ static int run_prepare_commit_msg_hook(struct repository *r, struct strbuf *msg, const char *commit) { - struct argv_array hook_env = ARGV_ARRAY_INIT; - int ret; - const char *name; + int ret = 0; + const char *name, *arg1 = NULL, *arg2 = NULL; 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", r->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) + if (commit) { + arg1 = "commit"; + arg2 = commit; + } else { + arg1 = "message"; + } + if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name, + arg1, arg2, NULL)) ret = error(_("'prepare-commit-msg' hook failed")); - argv_array_clear(&hook_env); return ret; } -- cgit v1.2.3 From 4627bc777e9ade5e3a85d6b8e8630fc4b6e2f8f6 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Tue, 15 Oct 2019 10:25:32 +0000 Subject: sequencer: run post-commit hook Prior to commit 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24) the sequencer would always run the post-commit hook after each pick or revert as it forked `git commit` to create the commit. The conversion to committing without forking `git commit` omitted to call the post-commit hook after creating the commit. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 1 + 1 file changed, 1 insertion(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index cdc0d1dfba..da2decbd3a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1401,6 +1401,7 @@ static int try_to_commit(struct repository *r, goto out; } + run_commit_hook(0, r->index_file, "post-commit", NULL); if (flags & AMEND_MSG) commit_post_rewrite(r, current_head, oid); -- cgit v1.2.3 From c068bcc59b4f16322a77b6a47b53d44b05c51fec Mon Sep 17 00:00:00 2001 From: Rohit Ashiwal Date: Fri, 1 Nov 2019 19:29:59 +0530 Subject: sequencer: allow callers of read_author_script() to ignore fields The current callers of the read_author_script() function read name, email and date from the author script. Allow callers to signal that they are not interested in some among these three fields by passing NULL. Note that fields that are ignored still must exist and be formatted correctly in the author script. Signed-off-by: Rohit Ashiwal Signed-off-by: Junio C Hamano --- sequencer.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 2adcf5a639..b759c940f8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -824,9 +824,19 @@ int read_author_script(const char *path, char **name, char **email, char **date, error(_("missing 'GIT_AUTHOR_DATE'")); if (date_i < 0 || email_i < 0 || date_i < 0 || err) goto finish; - *name = kv.items[name_i].util; - *email = kv.items[email_i].util; - *date = kv.items[date_i].util; + + if (name) + *name = kv.items[name_i].util; + else + free(kv.items[name_i].util); + if (email) + *email = kv.items[email_i].util; + else + free(kv.items[email_i].util); + if (date) + *date = kv.items[date_i].util; + else + free(kv.items[date_i].util); retval = 0; finish: string_list_clear(&kv, !!retval); -- cgit v1.2.3 From cbd8db17acb77ea646c739bf96c31fe7484bc491 Mon Sep 17 00:00:00 2001 From: Rohit Ashiwal Date: Fri, 1 Nov 2019 19:30:00 +0530 Subject: rebase -i: support --committer-date-is-author-date rebase am already has this flag to "lie" about the committer date by changing it to the author date. Let's add the same for interactive machinery. Signed-off-by: Rohit Ashiwal Signed-off-by: Junio C Hamano --- sequencer.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index b759c940f8..dfd7f2565f 100644 --- a/sequencer.c +++ b/sequencer.c @@ -147,6 +147,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") * command-line. */ static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") +static GIT_PATH_FUNC(rebase_path_cdate_is_adate, "rebase-merge/cdate_is_adate") static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") @@ -879,6 +880,17 @@ static char *get_author(const char *message) return NULL; } +/* Returns a "date" string that needs to be free()'d by the caller */ +static char *read_author_date_or_null(void) +{ + char *date; + + if (read_author_script(rebase_path_author_script(), + NULL, NULL, &date, 0)) + return NULL; + return date; +} + static const char staged_changes_advice[] = N_("you have staged changes in your working tree\n" "If these changes are meant to be squashed into the previous commit, run:\n" @@ -938,6 +950,24 @@ static int run_git_commit(struct repository *r, cmd.git_cmd = 1; + if (opts->committer_date_is_author_date) { + int res = -1; + struct strbuf datebuf = STRBUF_INIT; + char *date = read_author_date_or_null(); + + if (!date) + return -1; + + strbuf_addf(&datebuf, "@%s", date); + res = setenv("GIT_COMMITTER_DATE", datebuf.buf, 1); + + strbuf_release(&datebuf); + free(date); + + if (res) + return -1; + } + if (is_rebase_i(opts) && read_env_script(&cmd.env_array)) { const char *gpg_opt = gpg_sign_opt_quoted(opts); @@ -1331,7 +1361,6 @@ static int try_to_commit(struct repository *r, if (parse_head(r, ¤t_head)) return -1; - if (flags & AMEND_MSG) { const char *exclude_gpgsig[] = { "gpgsig", NULL }; const char *out_enc = get_commit_output_encoding(); @@ -1359,6 +1388,30 @@ static int try_to_commit(struct repository *r, commit_list_insert(current_head, &parents); } + if (opts->committer_date_is_author_date) { + int len = strlen(author); + struct ident_split ident; + struct strbuf date = STRBUF_INIT; + + if (split_ident_line(&ident, author, len) < 0) { + res = error(_("malformed ident line")); + goto out; + } + if (!ident.date_begin) { + res = error(_("corrupted author without date information")); + goto out; + } + + strbuf_addf(&date, "@%.*s %.*s", + (int)(ident.date_end - ident.date_begin), ident.date_begin, + (int)(ident.tz_end - ident.tz_begin), ident.tz_begin); + res = setenv("GIT_COMMITTER_DATE", date.buf, 1); + strbuf_release(&date); + + if (res) + goto out; + } + if (write_index_as_tree(&tree, r->index, r->index_file, 0, NULL)) { res = error(_("git write-tree failed to write a tree")); goto out; @@ -2480,6 +2533,11 @@ static int read_populate_opts(struct replay_opts *opts) opts->signoff = 1; } + if (file_exists(rebase_path_cdate_is_adate())) { + opts->allow_ff = 0; + opts->committer_date_is_author_date = 1; + } + if (file_exists(rebase_path_reschedule_failed_exec())) opts->reschedule_failed_exec = 1; @@ -2562,6 +2620,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, write_file(rebase_path_gpg_sign_opt(), "-S%s\n", opts->gpg_sign); if (opts->signoff) write_file(rebase_path_signoff(), "--signoff\n"); + if (opts->committer_date_is_author_date) + write_file(rebase_path_cdate_is_adate(), "%s", ""); if (opts->reschedule_failed_exec) write_file(rebase_path_reschedule_failed_exec(), "%s", ""); @@ -3650,7 +3710,8 @@ static int pick_commits(struct repository *r, setenv(GIT_REFLOG_ACTION, action_name(opts), 0); if (opts->allow_ff) assert(!(opts->signoff || opts->no_commit || - opts->record_origin || opts->edit)); + opts->record_origin || opts->edit || + opts->committer_date_is_author_date)); if (read_and_refresh_cache(r, opts)) return -1; -- cgit v1.2.3 From 0185c683c90baed447e96c18aafb705c91012b25 Mon Sep 17 00:00:00 2001 From: Rohit Ashiwal Date: Fri, 1 Nov 2019 19:30:01 +0530 Subject: sequencer: rename amend_author to author_to_rename The purpose of amend_author was to free() the malloc()'d string obtained from get_author() while amending a commit. But we can also use the variable to free() the author at our convenience. Rename it to convey this meaning. Signed-off-by: Rohit Ashiwal Signed-off-by: Junio C Hamano --- sequencer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index dfd7f2565f..df239babe9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1354,7 +1354,7 @@ static int try_to_commit(struct repository *r, struct commit_extra_header *extra = NULL; struct strbuf err = STRBUF_INIT; struct strbuf commit_msg = STRBUF_INIT; - char *amend_author = NULL; + char *author_to_free = NULL; const char *hook_commit = NULL; enum commit_msg_cleanup_mode cleanup; int res = 0; @@ -1375,7 +1375,7 @@ static int try_to_commit(struct repository *r, strbuf_addstr(msg, orig_message); hook_commit = "HEAD"; } - author = amend_author = get_author(message); + author = author_to_free = get_author(message); unuse_commit_buffer(current_head, message); if (!author) { res = error(_("unable to parse commit author")); @@ -1474,7 +1474,7 @@ out: free_commit_extra_headers(extra); strbuf_release(&err); strbuf_release(&commit_msg); - free(amend_author); + free(author_to_free); return res; } -- cgit v1.2.3 From 08187b4cbac2b2f870bb9c786d545b67f0262f74 Mon Sep 17 00:00:00 2001 From: Rohit Ashiwal Date: Fri, 1 Nov 2019 19:30:02 +0530 Subject: rebase -i: support --ignore-date rebase am already has this flag to "lie" about the author date by changing it to the committer (current) date. Let's add the same for interactive machinery. Signed-off-by: Rohit Ashiwal Signed-off-by: Junio C Hamano --- sequencer.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 3 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index df239babe9..f53694fd0b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -148,6 +148,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") */ static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") static GIT_PATH_FUNC(rebase_path_cdate_is_adate, "rebase-merge/cdate_is_adate") +static GIT_PATH_FUNC(rebase_path_ignore_date, "rebase-merge/ignore_date") static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") @@ -891,6 +892,36 @@ static char *read_author_date_or_null(void) return date; } +/* Construct a free()able author string with current time as the author date */ +static char *ignore_author_date(const char *author) +{ + int len = strlen(author); + struct ident_split ident; + struct strbuf new_author = STRBUF_INIT; + + if (split_ident_line(&ident, author, len) < 0) { + error(_("malformed ident line")); + return NULL; + } + len = ident.mail_end - ident.name_begin + 1; + + strbuf_addf(&new_author, "%.*s ", len, ident.name_begin); + datestamp(&new_author); + return strbuf_detach(&new_author, NULL); +} + +static void push_dates(struct child_process *child, int change_committer_date) +{ + time_t now = time(NULL); + struct strbuf date = STRBUF_INIT; + + strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now); + argv_array_pushf(&child->env_array, "GIT_AUTHOR_DATE=%s", date.buf); + if (change_committer_date) + argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf); + strbuf_release(&date); +} + static const char staged_changes_advice[] = N_("you have staged changes in your working tree\n" "If these changes are meant to be squashed into the previous commit, run:\n" @@ -959,7 +990,8 @@ static int run_git_commit(struct repository *r, return -1; strbuf_addf(&datebuf, "@%s", date); - res = setenv("GIT_COMMITTER_DATE", datebuf.buf, 1); + res = setenv("GIT_COMMITTER_DATE", + opts->ignore_date ? "" : datebuf.buf, 1); strbuf_release(&datebuf); free(date); @@ -983,6 +1015,8 @@ static int run_git_commit(struct repository *r, argv_array_push(&cmd.args, "--amend"); if (opts->gpg_sign) argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); + if (opts->ignore_date) + push_dates(&cmd, opts->committer_date_is_author_date); if (defmsg) argv_array_pushl(&cmd.args, "-F", defmsg, NULL); else if (!(flags & EDIT_MSG)) @@ -1405,7 +1439,8 @@ static int try_to_commit(struct repository *r, strbuf_addf(&date, "@%.*s %.*s", (int)(ident.date_end - ident.date_begin), ident.date_begin, (int)(ident.tz_end - ident.tz_begin), ident.tz_begin); - res = setenv("GIT_COMMITTER_DATE", date.buf, 1); + res = setenv("GIT_COMMITTER_DATE", + opts->ignore_date ? "" : date.buf, 1); strbuf_release(&date); if (res) @@ -1455,6 +1490,15 @@ static int try_to_commit(struct repository *r, reset_ident_date(); + if (opts->ignore_date) { + author = ignore_author_date(author); + if (!author) { + res = -1; + goto out; + } + free(author_to_free); + author_to_free = (char *)author; + } if (commit_tree_extended(msg->buf, msg->len, &tree, parents, oid, author, opts->gpg_sign, extra)) { res = error(_("failed to write commit object")); @@ -2538,6 +2582,11 @@ static int read_populate_opts(struct replay_opts *opts) opts->committer_date_is_author_date = 1; } + if (file_exists(rebase_path_ignore_date())) { + opts->allow_ff = 0; + opts->ignore_date = 1; + } + if (file_exists(rebase_path_reschedule_failed_exec())) opts->reschedule_failed_exec = 1; @@ -2622,6 +2671,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, write_file(rebase_path_signoff(), "--signoff\n"); if (opts->committer_date_is_author_date) write_file(rebase_path_cdate_is_adate(), "%s", ""); + if (opts->ignore_date) + write_file(rebase_path_ignore_date(), "%s", ""); if (opts->reschedule_failed_exec) write_file(rebase_path_reschedule_failed_exec(), "%s", ""); @@ -3439,6 +3490,8 @@ static int do_merge(struct repository *r, argv_array_push(&cmd.args, git_path_merge_msg(r)); if (opts->gpg_sign) argv_array_push(&cmd.args, opts->gpg_sign); + if (opts->ignore_date) + push_dates(&cmd, opts->committer_date_is_author_date); /* Add the tips to be merged */ for (j = to_merge; j; j = j->next) @@ -3711,7 +3764,8 @@ static int pick_commits(struct repository *r, if (opts->allow_ff) assert(!(opts->signoff || opts->no_commit || opts->record_origin || opts->edit || - opts->committer_date_is_author_date)); + opts->committer_date_is_author_date || + opts->ignore_date)); if (read_and_refresh_cache(r, opts)) return -1; -- cgit v1.2.3 From 15beaaa3d1f6b555900446deb5e376b4f806d734 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Tue, 5 Nov 2019 17:07:23 +0000 Subject: Fix spelling errors in code comments Reported-by: Jens Schleusener Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- sequencer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 9d5964fd81..93e54a0497 100644 --- a/sequencer.c +++ b/sequencer.c @@ -131,7 +131,7 @@ static GIT_PATH_FUNC(rebase_path_rewritten_pending, "rebase-merge/rewritten-pending") /* - * The path of the file containig the OID of the "squash onto" commit, i.e. + * The path of the file containing the OID of the "squash onto" commit, i.e. * the dummy commit used for `reset [new root]`. */ static GIT_PATH_FUNC(rebase_path_squash_onto, "rebase-merge/squash-onto") @@ -4644,7 +4644,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, label_oid(oid, "branch-point", &state); } - /* Add HEAD as implict "tip of branch" */ + /* Add HEAD as implicit "tip of branch" */ if (!iter->next) tips_tail = &commit_list_insert(iter->item, tips_tail)->next; @@ -4826,7 +4826,7 @@ void todo_list_add_exec_commands(struct todo_list *todo_list, * are considered part of the pick, so we insert the commands *after* * those chains if there are any. * - * As we insert the exec commands immediatly after rearranging + * As we insert the exec commands immediately after rearranging * any fixups and before the user edits the list, a fixup chain * can never contain comments (any comments are empty picks that * have been commented out because the user did not specify -- cgit v1.2.3 From 0798d16fe38e45453b626c699d92de6f9d71f5ac Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Fri, 8 Nov 2019 16:43:46 +0700 Subject: sequencer: reencode to utf-8 before arrange rebase's todo list On musl libc, ISO-2022-JP encoder is too eager to switch back to 1 byte encoding, musl's iconv always switch back after every combining character. Comparing glibc and musl's output for this command $ sed q t/t3900/ISO-2022-JP.txt| iconv -f ISO-2022-JP -t utf-8 | iconv -f utf-8 -t ISO-2022-JP | xxd glibc: 00000000: 1b24 4224 4f24 6c24 5224 5b24 551b 2842 .$B$O$l$R$[$U.(B 00000010: 0a . musl: 00000000: 1b24 4224 4f1b 2842 1b24 4224 6c1b 2842 .$B$O.(B.$B$l.(B 00000010: 1b24 4224 521b 2842 1b24 4224 5b1b 2842 .$B$R.(B.$B$[.(B 00000020: 1b24 4224 551b 2842 0a .$B$U.(B. Although musl iconv's output isn't optimal, it's still correct. From commit 7d509878b8, ("pretty.c: format string with truncate respects logOutputEncoding", 2014-05-21), we're encoding the message to utf-8 first, then format it and convert the message to the actual output encoding on git commit --squash. Thus, t3900::test_commit_autosquash_flags is failing on musl libc. Reencode to utf-8 before arranging rebase's todo list. By doing this, we also remove a breakage noticed by a test added in the previous commit. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 9d5964fd81..69430fe23f 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5169,7 +5169,7 @@ int todo_list_rearrange_squash(struct todo_list *todo_list) *commit_todo_item_at(&commit_todo, item->commit) = item; parse_commit(item->commit); - commit_buffer = get_commit_buffer(item->commit, NULL); + commit_buffer = logmsg_reencode(item->commit, NULL, "UTF-8"); find_commit_subject(commit_buffer, &subject); format_subject(&buf, subject, " "); subject = subjects[i] = strbuf_detach(&buf, &subject_len); -- cgit v1.2.3 From 019a9d836230c8851aa8b0d4dc2e0dea42662a90 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Fri, 8 Nov 2019 16:43:47 +0700 Subject: sequencer: reencode revert/cherry-pick's todo list Keep revert/cherry-pick's todo list in line with rebase todo list. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- sequencer.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 69430fe23f..a19954f2bf 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2564,14 +2564,17 @@ static int walk_revs_populate_todo(struct todo_list *todo_list, enum todo_command command = opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT; const char *command_string = todo_command_info[command].str; + const char *encoding; struct commit *commit; if (prepare_revs(opts)) return -1; + encoding = get_log_output_encoding(); + while ((commit = get_revision(opts->revs))) { struct todo_item *item = append_new_todo(todo_list); - const char *commit_buffer = get_commit_buffer(commit, NULL); + const char *commit_buffer = logmsg_reencode(commit, NULL, encoding); const char *subject; int subject_len; -- cgit v1.2.3 From b375744274113889c85bee69445375ce51e96648 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Fri, 8 Nov 2019 16:43:48 +0700 Subject: sequencer: reencode squashing commit's message On fixup/squash-ing rebase, git will create new commit in i18n.commitencoding, reencode the commit message to that said encode. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- sequencer.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index a19954f2bf..833a928929 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1576,6 +1576,7 @@ static int update_squash_messages(struct repository *r, struct strbuf buf = STRBUF_INIT; int res; const char *message, *body; + const char *encoding = get_commit_output_encoding(); if (opts->current_fixup_count > 0) { struct strbuf header = STRBUF_INIT; @@ -1602,7 +1603,7 @@ static int update_squash_messages(struct repository *r, return error(_("need a HEAD to fixup")); if (!(head_commit = lookup_commit_reference(r, &head))) return error(_("could not read HEAD")); - if (!(head_message = get_commit_buffer(head_commit, NULL))) + if (!(head_message = logmsg_reencode(head_commit, NULL, encoding))) return error(_("could not read HEAD's commit message")); find_commit_subject(head_message, &body); @@ -1623,7 +1624,7 @@ static int update_squash_messages(struct repository *r, unuse_commit_buffer(head_commit, head_message); } - if (!(message = get_commit_buffer(commit, NULL))) + if (!(message = logmsg_reencode(commit, NULL, encoding))) return error(_("could not read commit message of %s"), oid_to_hex(&commit->object.oid)); find_commit_subject(message, &body); @@ -4154,9 +4155,10 @@ static int commit_staged_changes(struct repository *r, */ struct commit *commit; const char *path = rebase_path_squash_msg(); + const char *encoding = get_commit_output_encoding(); if (parse_head(r, &commit) || - !(p = get_commit_buffer(commit, NULL)) || + !(p = logmsg_reencode(commit, NULL, encoding)) || write_message(p, strlen(p), path, 0)) { unuse_commit_buffer(commit, p); return error(_("could not write file: " -- cgit v1.2.3 From 5772b0c745ea7f57b94880f377e84a79e2675f38 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Mon, 11 Nov 2019 13:03:40 +0700 Subject: sequencer: reencode old merge-commit message During rebasing, old merge's message (encoded in old encoding) will be used as message for new merge commit (created by rebase). In case of the value of i18n.commitencoding has been changed after the old merge time. We will receive an unusable message for this new merge. Correct it. This change also notice a breakage with git-rebase label system. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- sequencer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 833a928929..d735d09f98 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3374,7 +3374,8 @@ static int do_merge(struct repository *r, } if (commit) { - const char *message = get_commit_buffer(commit, NULL); + const char *encoding = get_commit_output_encoding(); + const char *message = logmsg_reencode(commit, NULL, encoding); const char *body; int len; -- cgit v1.2.3 From 52f52e5ae4937de2bc798828c47c49f469b2cc85 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Mon, 11 Nov 2019 13:03:41 +0700 Subject: sequencer: reencode commit message for am/rebase --show-current-patch The message file will be used as commit message for the git-{am,rebase} --continue. Signed-off-by: Doan Tran Cong Danh Signed-off-by: Junio C Hamano --- sequencer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index d735d09f98..4d12ad3cc6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2972,7 +2972,8 @@ static int make_patch(struct repository *r, strbuf_addf(&buf, "%s/message", get_dir(opts)); if (!file_exists(buf.buf)) { - const char *commit_buffer = get_commit_buffer(commit, NULL); + const char *encoding = get_commit_output_encoding(); + const char *commit_buffer = logmsg_reencode(commit, NULL, encoding); find_commit_subject(commit_buffer, &subject); res |= write_message(subject, strlen(subject), buf.buf, 1); unuse_commit_buffer(commit, commit_buffer); -- cgit v1.2.3 From 867bc1d236b8955414b3dbacf28c7f0c2e337cf4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 17 Nov 2019 23:16:09 +0000 Subject: rebase-merges: move labels' whitespace mangling into `label_oid()` One of the trickier aspects of the design of `git rebase --rebase-merges` is the way labels are generated for the initial todo list: those labels are supposed to be intuitive and first and foremost unique. To that end, `label_oid()` appends a unique suffix when necessary. Those labels not only need to be unique, but they also need to be valid refs. To make sure of that, `make_script_with_merges()` replaces whitespace by dashes. That would appear to be the wrong layer for that sanitizing step, though: all callers of `label_oid()` should get that same benefit. Even if it does not make a difference currently (the only called of `label_oid()` that passes a label that might need to be sanitized _is_ `make_script_with_merges()`), let's move the responsibility for sanitizing labels into the `label_oid()` function. This commit is best viewed with `-w` because it unfortunately needs to change the indentation of a large block of code in `label_oid()`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 56 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 8952cfa89b..85c66f489f 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4423,7 +4423,6 @@ static const char *label_oid(struct object_id *oid, const char *label, struct labels_entry *labels_entry; struct string_entry *string_entry; struct object_id dummy; - size_t len; int i; string_entry = oidmap_get(&state->commit2label, oid); @@ -4443,10 +4442,10 @@ static const char *label_oid(struct object_id *oid, const char *label, * abbreviation for any uninteresting commit's names that does not * clash with any other label. */ + strbuf_reset(&state->buf); if (!label) { char *p; - strbuf_reset(&state->buf); strbuf_grow(&state->buf, GIT_MAX_HEXSZ); label = p = state->buf.buf; @@ -4469,32 +4468,37 @@ static const char *label_oid(struct object_id *oid, const char *label, p[i] = save; } } - } else if (((len = strlen(label)) == the_hash_algo->hexsz && - !get_oid_hex(label, &dummy)) || - (len == 1 && *label == '#') || - hashmap_get_from_hash(&state->labels, - strihash(label), label)) { - /* - * If the label already exists, or if the label is a valid full - * OID, or the label is a '#' (which we use as a separator - * between merge heads and oneline), we append a dash and a - * number to make it unique. - */ + } else { struct strbuf *buf = &state->buf; - strbuf_reset(buf); - strbuf_add(buf, label, len); + for (; *label; label++) + strbuf_addch(buf, isspace(*label) ? '-' : *label); + label = buf->buf; - for (i = 2; ; i++) { - strbuf_setlen(buf, len); - strbuf_addf(buf, "-%d", i); - if (!hashmap_get_from_hash(&state->labels, - strihash(buf->buf), - buf->buf)) - break; - } + if ((buf->len == the_hash_algo->hexsz && + !get_oid_hex(label, &dummy)) || + (buf->len == 1 && *label == '#') || + hashmap_get_from_hash(&state->labels, + strihash(label), label)) { + /* + * If the label already exists, or if the label is a + * valid full OID, or the label is a '#' (which we use + * as a separator between merge heads and oneline), we + * append a dash and a number to make it unique. + */ + size_t len = buf->len; - label = buf->buf; + for (i = 2; ; i++) { + strbuf_setlen(buf, len); + strbuf_addf(buf, "-%d", i); + if (!hashmap_get_from_hash(&state->labels, + strihash(buf->buf), + buf->buf)) + break; + } + + label = buf->buf; + } } FLEX_ALLOC_STR(labels_entry, label, label); @@ -4596,10 +4600,6 @@ static int make_script_with_merges(struct pretty_print_context *pp, else strbuf_addbuf(&label, &oneline); - for (p1 = label.buf; *p1; p1++) - if (isspace(*p1)) - *(char *)p1 = '-'; - strbuf_reset(&buf); strbuf_addf(&buf, "%s -C %s", cmd_merge, oid_to_hex(&commit->object.oid)); -- cgit v1.2.3 From cd5522271f6b985114b33332e148bff2283b0440 Mon Sep 17 00:00:00 2001 From: Matthew Rogers Date: Sun, 17 Nov 2019 23:16:10 +0000 Subject: rebase -r: let `label` generate safer labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `label` todo command in interactive rebases creates temporary refs in the `refs/rewritten/` namespace. These refs are stored as loose refs, i.e. as files in `.git/refs/rewritten/`, therefore they have to conform with file name limitations on the current filesystem in addition to the accepted ref format. This poses a problem in particular on NTFS/FAT, where e.g. the colon, double-quote and pipe characters are disallowed as part of a file name. Let's safeguard against this by replacing not only white-space characters by dashes, but all non-alpha-numeric ones. However, we exempt non-ASCII UTF-8 characters from that, as it should be quite possible to reflect branch names such as `↯↯↯` in refs/file names. Signed-off-by: Matthew Rogers Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 85c66f489f..fece07b680 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4471,8 +4471,26 @@ static const char *label_oid(struct object_id *oid, const char *label, } else { struct strbuf *buf = &state->buf; + /* + * Sanitize labels by replacing non-alpha-numeric characters + * (including white-space ones) by dashes, as they might be + * illegal in file names (and hence in ref names). + * + * Note that we retain non-ASCII UTF-8 characters (identified + * via the most significant bit). They should be all acceptable + * in file names. We do not validate the UTF-8 here, that's not + * the job of this function. + */ for (; *label; label++) - strbuf_addch(buf, isspace(*label) ? '-' : *label); + if ((*label & 0x80) || isalnum(*label)) + strbuf_addch(buf, *label); + /* avoid leading dash and double-dashes */ + else if (buf->len && buf->buf[buf->len - 1] != '-') + strbuf_addch(buf, '-'); + if (!buf->len) { + strbuf_addstr(buf, "rev-"); + strbuf_add_unique_abbrev(buf, oid, default_abbrev); + } label = buf->buf; if ((buf->len == the_hash_algo->hexsz && -- cgit v1.2.3 From e02058a72967b18bd906674de6191f42d03b0763 Mon Sep 17 00:00:00 2001 From: Doan Tran Cong Danh Date: Mon, 18 Nov 2019 18:57:47 +0700 Subject: sequencer: handle rebase-merges for "onto" message In order to work correctly, git-rebase --rebase-merges needs to make initial todo list with unique labels. Those unique labels is being handled by employing a hashmap and appending an unique number if any duplicate is found. But, we forget that beside those labels for side branches, we also have a special label `onto' for our so-called new-base. In a special case that any of those labels for side branches named `onto', git will run into trouble. Correct it. Signed-off-by: Doan Tran Cong Danh Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index fece07b680..9147d02f53 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4560,10 +4560,15 @@ static int make_script_with_merges(struct pretty_print_context *pp, strbuf_init(&state.buf, 32); if (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) { + struct labels_entry *onto_label_entry; struct object_id *oid = &revs->cmdline.rev[0].item->oid; FLEX_ALLOC_STR(entry, string, "onto"); oidcpy(&entry->entry.oid, oid); oidmap_put(&state.commit2label, entry); + + FLEX_ALLOC_STR(onto_label_entry, label, "onto"); + hashmap_entry_init(&onto_label_entry->entry, strihash("onto")); + hashmap_add(&state.labels, &onto_label_entry->entry); } /* -- cgit v1.2.3 From 2d05ef2778b99e05e6a8a40c5b8d59a3c4ab1274 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Fri, 22 Nov 2019 19:43:03 +0000 Subject: sequencer: fix empty commit check when amending This fixes a regression introduced in 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24). When amending a commit try_to_commit() was using the wrong parent when checking if the commit would be empty. When amending we need to check against HEAD^ not HEAD. t3403 may not seem like the natural home for the new tests but a further patch series will improve the advice printed by `git commit`. That series will mutate these tests to check that the advice includes suggesting `rebase --skip` to skip the fixup that would empty the commit. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index da2decbd3a..f4f81cbddc 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1351,11 +1351,27 @@ static int try_to_commit(struct repository *r, goto out; } - if (!(flags & ALLOW_EMPTY) && oideq(current_head ? - get_commit_tree_oid(current_head) : - the_hash_algo->empty_tree, &tree)) { - res = 1; /* run 'git commit' to display error message */ - goto out; + if (!(flags & ALLOW_EMPTY)) { + struct commit *first_parent = current_head; + + if (flags & AMEND_MSG) { + if (current_head->parents) { + first_parent = current_head->parents->item; + if (repo_parse_commit(r, first_parent)) { + res = error(_("could not parse HEAD commit")); + goto out; + } + } else { + first_parent = NULL; + } + } + if (oideq(first_parent + ? get_commit_tree_oid(first_parent) + : the_hash_algo->empty_tree, + &tree)) { + res = 1; /* run 'git commit' to display error message */ + goto out; + } } if (find_hook("prepare-commit-msg")) { -- cgit v1.2.3 From befd4f6a81d382cc2b34186b619f734fa5f8070f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Sat, 23 Nov 2019 18:20:46 +0100 Subject: sequencer: don't re-read todo for revert and cherry-pick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When 'git revert' or 'git cherry-pick --edit' is invoked with multiple commits, then after editing the first commit message is finished both these commands should continue with processing the second commit and launch another editor for its commit message, assuming there are no conflicts, of course. Alas, this inadvertently changed with commit a47ba3c777 (rebase -i: check for updated todo after squash and reword, 2019-08-19): after editing the first commit message is finished, both 'git revert' and 'git cherry-pick --edit' exit with error, claiming that "nothing to commit, working tree clean". The reason for the changed behaviour is twofold: - Prior to a47ba3c777 the up-to-dateness of the todo list file was only checked after 'exec' instructions, and that commit moved those checks to the common code path. The intention was that this check should be performed after instructions spawning an editor ('squash' and 'reword') as well, so the ongoing 'rebase -i' notices when the user runs a 'git rebase --edit-todo' while squashing/rewording a commit message. However, as it happened that check is now performed even after 'revert' and 'pick' instructions when they involved editing the commit message. And 'revert' by default while 'pick' optionally (with 'git cherry-pick --edit') involves editing the commit message. - When invoking 'git revert' or 'git cherry-pick --edit' with multiple commits they don't read a todo list file but assemble the todo list in memory, thus the associated stat data used to check whether the file has been updated is all zeroed out initially. Then the sequencer writes all instructions (including the very first) to the todo file, executes the first 'revert/pick' instruction, and after the user finished editing the commit message the changes of a47ba3c777 kick in, and it checks whether the todo file has been modified. The initial all-zero stat data obviously differs from the todo file's current stat data, so the sequencer concludes that the file has been modified. Technically it is not wrong, of course, because the file just has been written indeed by the sequencer itself, though the file's contents still match what the sequencer was invoked with in the beginning. Consequently, after re-reading the todo file the sequencer executes the same first instruction _again_, thus ending up in that "nothing to commit" situation. The todo list was never meant to be edited during multi-commit 'git revert' or 'cherry-pick' operations, so perform that "has the todo file been modified" check only when the sequencer was invoked as part of an interactive rebase. Reported-by: Brian Norris Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 2adcf5a639..3b05d0277d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3791,7 +3791,7 @@ static int pick_commits(struct repository *r, item->commit, arg, item->arg_len, opts, res, 0); - } else if (check_todo && !res) { + } else if (is_rebase_i(opts) && check_todo && !res) { struct stat st; if (stat(get_todo_path(opts), &st)) { -- cgit v1.2.3 From 8638114e063f60fef678d51ca904da7c4e1ab3c0 Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:28 +0100 Subject: sequencer: update `total_nr' when adding an item to a todo list `total_nr' is the total number of items, counting both done and todo, that are in a todo list. But unlike `nr', it was not updated when an item was appended to the list. This variable is mostly used by command prompts (ie. git-prompt.sh and the like). By forgetting to update it, the original code made it not reflect the reality, but this flaw was masked by the code calling unnecessarily read_populate_todo() again to update the variable to its correct value. At the end of this series, the unnecessary call will be removed, and the inconsistency addressed by this patch would start to matter. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 1 + 1 file changed, 1 insertion(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index d648aaf416..575b852a5a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2070,6 +2070,7 @@ void todo_list_release(struct todo_list *todo_list) static struct todo_item *append_new_todo(struct todo_list *todo_list) { ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc); + todo_list->total_nr++; return todo_list->items + todo_list->nr++; } -- cgit v1.2.3 From 34065541e3f99ce23ac431032daf9d72072e650b Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:29 +0100 Subject: sequencer: update `done_nr' when skipping commands in a todo list In a todo list, `done_nr' is the number of commands that were executed or skipped, but skip_unnecessary_picks() did not update it. This variable is mostly used by command prompts (ie. git-prompt.sh and the like). As in the previous commit, this inconsistent behaviour is not a problem yet, but it would start to matter at the end of this series the same reason. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 1 + 1 file changed, 1 insertion(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 575b852a5a..42313f8de6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5054,6 +5054,7 @@ static int skip_unnecessary_picks(struct repository *r, MOVE_ARRAY(todo_list->items, todo_list->items + i, todo_list->nr - i); todo_list->nr -= i; todo_list->current = 0; + todo_list->done_nr += i; if (is_fixup(peek_command(todo_list, 0))) record_in_rewritten(base_oid, peek_command(todo_list, 0)); -- cgit v1.2.3 From 3f34f2d8a4da82ddda48a591cbb091f24a5f3e58 Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:30 +0100 Subject: sequencer: move the code writing total_nr on the disk to a new function The total number of commands can be used to show the progression of the rebasing in a shell. It is written to the disk by read_populate_todo() when the todo list is loaded from sequencer_continue() or pick_commits(), but not by complete_action(). This moves the part writing total_nr to a new function so it can be called from complete_action(). Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 42313f8de6..ec7ea8d9e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2342,6 +2342,16 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose) sequencer_remove_state(&opts); } +static void todo_list_write_total_nr(struct todo_list *todo_list) +{ + FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); + + if (f) { + fprintf(f, "%d\n", todo_list->total_nr); + fclose(f); + } +} + static int read_populate_todo(struct repository *r, struct todo_list *todo_list, struct replay_opts *opts) @@ -2387,7 +2397,6 @@ static int read_populate_todo(struct repository *r, if (is_rebase_i(opts)) { struct todo_list done = TODO_LIST_INIT; - FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 && !todo_list_parse_insn_buffer(r, done.buf.buf, &done)) @@ -2399,10 +2408,7 @@ static int read_populate_todo(struct repository *r, + count_commands(todo_list); todo_list_release(&done); - if (f) { - fprintf(f, "%d\n", todo_list->total_nr); - fclose(f); - } + todo_list_write_total_nr(todo_list); } return 0; -- cgit v1.2.3 From 393adf7a6f600adca8cb75ec4e7136d523e8840d Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:32 +0100 Subject: sequencer: directly call pick_commits() from complete_action() Currently, complete_action(), used by builtin/rebase.c to start a new rebase, calls sequencer_continue() to do it. Before the former calls pick_commits(), it - calls read_and_refresh_cache() -- this is unnecessary here as we've just called require_clean_work_tree() in complete_action() - calls read_populate_opts() -- this is unnecessary as we're starting a new rebase, so `opts' is fully populated - loads the todo list -- this is unnecessary as we've just populated the todo list in complete_action() - commits any staged changes -- this is unnecessary as we're starting a new rebase, so there are no staged changes - calls record_in_rewritten() -- this is unnecessary as we're starting a new rebase. This changes complete_action() to directly call pick_commits() to avoid these unnecessary steps. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index ec7ea8d9e5..ec0b793fc5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5140,15 +5140,21 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla return error_errno(_("could not write '%s'"), todo_file); } - todo_list_release(&new_todo); + res = -1; if (checkout_onto(r, opts, onto_name, &oid, orig_head)) - return -1; + goto cleanup; if (require_clean_work_tree(r, "rebase", "", 1, 1)) - return -1; + goto cleanup; - return sequencer_continue(r, opts); + todo_list_write_total_nr(&new_todo); + res = pick_commits(r, &new_todo, opts); + +cleanup: + todo_list_release(&new_todo); + + return res; } struct subject2item_entry { -- cgit v1.2.3 From f6b9413bafbe22202007f9c891082c1df82fce52 Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Fri, 29 Nov 2019 00:02:03 +0100 Subject: sequencer: fix a memory leak in sequencer_continue() When continuing an interactive rebase after a merge conflict was solved, if the resolution could not be committed, sequencer_continue() would return early without releasing its todo list, resulting in a memory leak. This plugs this leak by jumping to the end of the function, where the todo list is deallocated. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 34ebf8ed94..cc037776bf 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4256,8 +4256,10 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts) if (is_rebase_i(opts)) { if ((res = read_populate_todo(r, &todo_list, opts))) goto release_todo_list; - if (commit_staged_changes(r, opts, &todo_list)) - return -1; + if (commit_staged_changes(r, opts, &todo_list)) { + res = -1; + goto release_todo_list; + } } else if (!file_exists(get_todo_path(opts))) return continue_single_pick(r); else if ((res = read_populate_todo(r, &todo_list, opts))) -- cgit v1.2.3