* [RFC PATCH 00/11] rebase -i run without forking rebase--interactive
@ 2019-03-19 19:03 Phillip Wood
2019-03-19 19:03 ` [RFC PATCH 01/11] sequencer: always discard index after checkout Phillip Wood
` (14 more replies)
0 siblings, 15 replies; 40+ messages in thread
From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
When the builtin rebase starts an interactive rebase it parses the
options and then repackages them and forks `rebase--interactive`. This
series refactors rebase--interactive so that interactive rebases can
be started by the builtin rebase without forking. My motivation was to
make it easier to debug the sequencer but this should help future
maintainability.
This series involves some code movement so viewing the diffs with
--color-moved is recommended.
These patches are based on a merge of master [e902e9bcae ("The second
batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841
("rebase--interactive: move transform_todo_file()", 2019-03-05). They
can be fetched from the tag rebase-i-no-fork/rfc at
https://github.com/phillipwood/git.git
Phillip Wood (11):
sequencer: always discard index after checkout
rebase: rename write_basic_state()
rebase: use OPT_RERERE_AUTOUPDATE()
rebase -i: combine rebase--interactive.c with rebase.c
rebase -i: remove duplication
rebase -i: use struct commit when parsing options
rebase -i: use struct object_id for squash_onto
rebase -i: use struct rebase_options to parse args
rebase -i: use struct rebase_options in do_interactive_rebase()
rebase: use a common action enum
rebase -i: run without forking rebase--interactive
Makefile | 1 -
builtin/rebase--interactive.c | 377 --------------------
builtin/rebase.c | 625 ++++++++++++++++++++++++++--------
parse-options-cb.c | 34 ++
parse-options.h | 4 +
sequencer.c | 42 ++-
sequencer.h | 7 +-
7 files changed, 556 insertions(+), 534 deletions(-)
delete mode 100644 builtin/rebase--interactive.c
--
2.21.0
^ permalink raw reply [flat|nested] 40+ messages in thread* [RFC PATCH 01/11] sequencer: always discard index after checkout 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-20 1:50 ` Duy Nguyen 2019-03-19 19:03 ` [RFC PATCH 02/11] rebase: rename write_basic_state() Phillip Wood ` (13 subsequent siblings) 14 siblings, 1 reply; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> As the checkout runs in a separate process our index will be out of date so it should be discarded. The existing callers are not doing this consistently so do it here to avoid the callers having to worry about it. This fixes some test failures that happen if do_interactive_rebase() is called without forking rebase--interactive which we will implement shortly. Running git rebase -i master topic starting on master created empty todo lists because all the commits in topic were marked as cherry-picks. After topic was checked out in prepare_branch_to_be_rebased() the working tree contained the contents from topic but the index contained master and the cache entries were still valid. This meant that diff_populate_filespec() which loads the blobs when calculating patch-id's ended up reading the contents for master from the working tree which actually contained topic. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- Notes: It would perhaps be better to pass around the_index rather than the_repository builtin/rebase--interactive.c | 2 +- sequencer.c | 27 +++++++++++++++++---------- sequencer.h | 3 ++- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c index 4535523bf5..d1a4ac1b84 100644 --- a/builtin/rebase--interactive.c +++ b/builtin/rebase--interactive.c @@ -171,7 +171,7 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, struct argv_array make_script_args = ARGV_ARRAY_INIT; struct todo_list todo_list = TODO_LIST_INIT; - if (prepare_branch_to_be_rebased(opts, switch_to)) + if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) return -1; if (get_revision_ranges(upstream, onto, &head_hash, diff --git a/sequencer.c b/sequencer.c index 281a8ade19..ccc0160800 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3418,10 +3418,11 @@ static const char *reflog_message(struct replay_opts *opts, return buf.buf; } -static int run_git_checkout(struct replay_opts *opts, const char *commit, - const char *action) +static int run_git_checkout(struct repository *r, struct replay_opts *opts, + const char *commit, const char *action) { struct child_process cmd = CHILD_PROCESS_INIT; + int ret; cmd.git_cmd = 1; @@ -3430,25 +3431,31 @@ static int run_git_checkout(struct replay_opts *opts, const char *commit, argv_array_pushf(&cmd.env_array, GIT_REFLOG_ACTION "=%s", action); if (opts->verbose) - return run_command(&cmd); + ret = run_command(&cmd); else - return run_command_silent_on_success(&cmd); + ret = run_command_silent_on_success(&cmd); + + if (!ret) + discard_index(r->index); + + return ret; } -int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit) +int prepare_branch_to_be_rebased(struct repository *r, struct replay_opts *opts, + const char *commit) { const char *action; if (commit && *commit) { action = reflog_message(opts, "start", "checkout %s", commit); - if (run_git_checkout(opts, commit, action)) + if (run_git_checkout(r, opts, commit, action)) return error(_("could not checkout %s"), commit); } return 0; } -static int checkout_onto(struct replay_opts *opts, +static int checkout_onto(struct repository *r, struct replay_opts *opts, const char *onto_name, const char *onto, const char *orig_head) { @@ -3458,7 +3465,7 @@ static int checkout_onto(struct replay_opts *opts, if (get_oid(orig_head, &oid)) return error(_("%s: not a valid OID"), orig_head); - if (run_git_checkout(opts, onto, action)) { + if (run_git_checkout(r, opts, onto, action)) { apply_autostash(opts); sequencer_remove_state(opts); return error(_("could not detach HEAD")); @@ -4786,7 +4793,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) || todo_list_check(todo_list, &new_todo)) { fprintf(stderr, _(edit_todo_list_advice)); - checkout_onto(opts, onto_name, onto, orig_head); + checkout_onto(r, opts, onto_name, onto, orig_head); todo_list_release(&new_todo); return -1; @@ -4805,7 +4812,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla todo_list_release(&new_todo); - if (checkout_onto(opts, onto_name, oid_to_hex(&oid), orig_head)) + if (checkout_onto(r, opts, onto_name, oid_to_hex(&oid), orig_head)) return -1; if (require_clean_work_tree(r, "rebase", "", 1, 1)) diff --git a/sequencer.h b/sequencer.h index a515ee4457..6c55aa4200 100644 --- a/sequencer.h +++ b/sequencer.h @@ -175,7 +175,8 @@ void commit_post_rewrite(struct repository *r, const struct commit *current_head, const struct object_id *new_head); -int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit); +int prepare_branch_to_be_rebased(struct repository *r, struct replay_opts *opts, + const char *commit); #define SUMMARY_INITIAL_COMMIT (1 << 0) #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1) -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 01/11] sequencer: always discard index after checkout 2019-03-19 19:03 ` [RFC PATCH 01/11] sequencer: always discard index after checkout Phillip Wood @ 2019-03-20 1:50 ` Duy Nguyen 2019-03-21 14:35 ` Phillip Wood 0 siblings, 1 reply; 40+ messages in thread From: Duy Nguyen @ 2019-03-20 1:50 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin On Wed, Mar 20, 2019 at 2:04 AM Phillip Wood <phillip.wood123@gmail.com> wrote: > It would perhaps be better to pass around the_index rather than > the_repository Not by a large margin. For sequencer.c most operations require more than just the index and passing 'struct repository *' around has been the norm. And as soon as you need to load the index back (not sure if you should do it here btw, after discard_index, since we have the index loaded before) you need 'struct repository' not 'struct index_state'. > builtin/rebase--interactive.c | 2 +- > sequencer.c | 27 +++++++++++++++++---------- > sequencer.h | 3 ++- > 3 files changed, 20 insertions(+), 12 deletions(-) -- Duy ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 01/11] sequencer: always discard index after checkout 2019-03-20 1:50 ` Duy Nguyen @ 2019-03-21 14:35 ` Phillip Wood 0 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-21 14:35 UTC (permalink / raw) To: Duy Nguyen, Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin On 20/03/2019 01:50, Duy Nguyen wrote: > On Wed, Mar 20, 2019 at 2:04 AM Phillip Wood <phillip.wood123@gmail.com> wrote: >> It would perhaps be better to pass around the_index rather than >> the_repository > > Not by a large margin. For sequencer.c most operations require more > than just the index and passing 'struct repository *' around has been > the norm. And as soon as you need to load the index back (not sure if > you should do it here btw, after discard_index, since we have the > index loaded before) you need 'struct repository' not 'struct > index_state'. Thanks, I think I'll leave it as it is then. After we checkout the new base we reload the index in the loop that picks the commits. For 'rebase <upstream> <branch>' after we checkout <branch> we create the todo-list which involves a revision walk and then checkout the new base. I'm not entirely sure if it needs reloading before we create the todo list but I think it probably not as I don't think rebase--interactive.c loads the index (which would explain why this only becomes an issue when we stop forking rebase--interactive from rebase because then rebase.c has loaded the index to check there are no uncommitted changes) Best Wishes Phillip > >> builtin/rebase--interactive.c | 2 +- >> sequencer.c | 27 +++++++++++++++++---------- >> sequencer.h | 3 ++- >> 3 files changed, 20 insertions(+), 12 deletions(-) ^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH 02/11] rebase: rename write_basic_state() 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 01/11] sequencer: always discard index after checkout Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 03/11] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood ` (12 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> This clashes with a function in sequencer.c Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 52114cbf0d..b634879c5b 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -245,7 +245,7 @@ static int read_basic_state(struct rebase_options *opts) return 0; } -static int write_basic_state(struct rebase_options *opts) +static int rebase_write_basic_state(struct rebase_options *opts) { write_file(state_dir_path("head-name", opts), "%s", opts->head_name ? opts->head_name : "detached HEAD"); @@ -640,7 +640,7 @@ static int run_am(struct rebase_options *opts) } if (is_directory(opts->state_dir)) - write_basic_state(opts); + rebase_write_basic_state(opts); return status; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 03/11] rebase: use OPT_RERERE_AUTOUPDATE() 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 01/11] sequencer: always discard index after checkout Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 02/11] rebase: rename write_basic_state() Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 04/11] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood ` (11 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> As we have a macro for this it makes sense to use it. Having cmd_rebase() and cmd_rebase__interactive() use the same values for this option will be helpful when we start running interactive rebases without forking rebase--interactive. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index b634879c5b..ccf178326c 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -206,14 +206,13 @@ static int read_basic_state(struct rebase_options *opts) &buf)) return -1; if (!strcmp(buf.buf, "--rerere-autoupdate")) - opts->allow_rerere_autoupdate = 1; + opts->allow_rerere_autoupdate = RERERE_AUTOUPDATE; else if (!strcmp(buf.buf, "--no-rerere-autoupdate")) - opts->allow_rerere_autoupdate = 0; + opts->allow_rerere_autoupdate = RERERE_NOAUTOUPDATE; else warning(_("ignoring invalid allow_rerere_autoupdate: " "'%s'"), buf.buf); - } else - opts->allow_rerere_autoupdate = -1; + } if (file_exists(state_dir_path("gpg_sign_opt", opts))) { strbuf_reset(&buf); @@ -263,10 +262,11 @@ static int rebase_write_basic_state(struct rebase_options *opts) if (opts->strategy_opts) write_file(state_dir_path("strategy_opts", opts), "%s", opts->strategy_opts); - if (opts->allow_rerere_autoupdate >= 0) + if (opts->allow_rerere_autoupdate > 0) write_file(state_dir_path("allow_rerere_autoupdate", opts), "-%s-rerere-autoupdate", - opts->allow_rerere_autoupdate ? "" : "-no"); + opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE ? + "" : "-no"); if (opts->gpg_sign_opt) write_file(state_dir_path("gpg_sign_opt", opts), "%s", opts->gpg_sign_opt); @@ -625,9 +625,9 @@ static int run_am(struct rebase_options *opts) argv_array_push(&am.args, "--rebasing"); argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg); argv_array_push(&am.args, "--patch-format=mboxrd"); - if (opts->allow_rerere_autoupdate > 0) + if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) argv_array_push(&am.args, "--rerere-autoupdate"); - else if (opts->allow_rerere_autoupdate == 0) + else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE) argv_array_push(&am.args, "--no-rerere-autoupdate"); if (opts->gpg_sign_opt) argv_array_push(&am.args, opts->gpg_sign_opt); @@ -713,9 +713,9 @@ static int run_specific_rebase(struct rebase_options *opts) argv_array_pushf(&child.args, "--cmd=%s", opts->cmd); if (opts->allow_empty_message) argv_array_push(&child.args, "--allow-empty-message"); - if (opts->allow_rerere_autoupdate > 0) + if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) argv_array_push(&child.args, "--rerere-autoupdate"); - else if (opts->allow_rerere_autoupdate == 0) + else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE) argv_array_push(&child.args, "--no-rerere-autoupdate"); if (opts->gpg_sign_opt) argv_array_push(&child.args, opts->gpg_sign_opt); @@ -764,9 +764,9 @@ static int run_specific_rebase(struct rebase_options *opts) add_var(&script_snippet, "action", opts->action ? opts->action : ""); add_var(&script_snippet, "signoff", opts->signoff ? "--signoff" : ""); add_var(&script_snippet, "allow_rerere_autoupdate", - opts->allow_rerere_autoupdate < 0 ? "" : opts->allow_rerere_autoupdate ? - "--rerere-autoupdate" : "--no-rerere-autoupdate"); + opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE ? + "--rerere-autoupdate" : "--no-rerere-autoupdate" : ""); add_var(&script_snippet, "keep_empty", opts->keep_empty ? "yes" : ""); add_var(&script_snippet, "autosquash", opts->autosquash ? "t" : ""); add_var(&script_snippet, "gpg_sign_opt", opts->gpg_sign_opt); @@ -1007,7 +1007,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) .type = REBASE_UNSPECIFIED, .flags = REBASE_NO_QUIET, .git_am_opts = ARGV_ARRAY_INIT, - .allow_rerere_autoupdate = -1, .allow_empty_message = 1, .git_format_patch_opt = STRBUF_INIT, }; @@ -1102,10 +1101,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) OPT_SET_INT('p', "preserve-merges", &options.type, N_("try to recreate merges instead of ignoring " "them"), REBASE_PRESERVE_MERGES), - OPT_BOOL(0, "rerere-autoupdate", - &options.allow_rerere_autoupdate, - N_("allow rerere to update index with resolved " - "conflict")), + OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate), OPT_BOOL('k', "keep-empty", &options.keep_empty, N_("preserve empty commits during rebase")), OPT_BOOL(0, "autosquash", &options.autosquash, -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 04/11] rebase -i: combine rebase--interactive.c with rebase.c 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (2 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 03/11] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 05/11] rebase -i: remove duplication Phillip Wood ` (10 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> In order to run `rebase -i` without forking `rebase--interactive` it will be convenient to have all the code from rebase--interactive.c in rebase.c. This is a straight forward copy of the code from rebase--interactive.c, it will be simplified slightly in the next commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- Makefile | 1 - builtin/rebase--interactive.c | 377 ---------------------------------- builtin/rebase.c | 367 +++++++++++++++++++++++++++++++++ 3 files changed, 367 insertions(+), 378 deletions(-) delete mode 100644 builtin/rebase--interactive.c diff --git a/Makefile b/Makefile index 537493822b..c644089244 100644 --- a/Makefile +++ b/Makefile @@ -1130,7 +1130,6 @@ BUILTIN_OBJS += builtin/push.o BUILTIN_OBJS += builtin/range-diff.o BUILTIN_OBJS += builtin/read-tree.o BUILTIN_OBJS += builtin/rebase.o -BUILTIN_OBJS += builtin/rebase--interactive.o BUILTIN_OBJS += builtin/receive-pack.o BUILTIN_OBJS += builtin/reflog.o BUILTIN_OBJS += builtin/remote.o diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c deleted file mode 100644 index d1a4ac1b84..0000000000 --- a/builtin/rebase--interactive.c +++ /dev/null @@ -1,377 +0,0 @@ -#define USE_THE_INDEX_COMPATIBILITY_MACROS -#include "builtin.h" -#include "cache.h" -#include "config.h" -#include "parse-options.h" -#include "sequencer.h" -#include "rebase-interactive.h" -#include "argv-array.h" -#include "refs.h" -#include "rerere.h" -#include "run-command.h" - -static GIT_PATH_FUNC(path_state_dir, "rebase-merge/") -static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto") -static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive") - -static int add_exec_commands(struct string_list *commands) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT; - int res; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) { - todo_list_release(&todo_list); - return error(_("unusable todo list: '%s'"), todo_file); - } - - todo_list_add_exec_commands(&todo_list, commands); - res = todo_list_write_to_file(the_repository, &todo_list, - todo_file, NULL, NULL, -1, 0); - todo_list_release(&todo_list); - - if (res) - return error_errno(_("could not write '%s'."), todo_file); - return 0; -} - -static int rearrange_squash_in_todo_file(void) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT; - int res = 0; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) { - todo_list_release(&todo_list); - return error(_("unusable todo list: '%s'"), todo_file); - } - - res = todo_list_rearrange_squash(&todo_list); - if (!res) - res = todo_list_write_to_file(the_repository, &todo_list, - todo_file, NULL, NULL, -1, 0); - - todo_list_release(&todo_list); - - if (res) - return error_errno(_("could not write '%s'."), todo_file); - return 0; -} - -static int transform_todo_file(unsigned flags) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT; - int res; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) { - todo_list_release(&todo_list); - return error(_("unusable todo list: '%s'"), todo_file); - } - - res = todo_list_write_to_file(the_repository, &todo_list, todo_file, - NULL, NULL, -1, flags); - todo_list_release(&todo_list); - - if (res) - return error_errno(_("could not write '%s'."), todo_file); - return 0; -} - -static int edit_todo_file(unsigned flags) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT, - new_todo = TODO_LIST_INIT; - int res = 0; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - - strbuf_stripspace(&todo_list.buf, 1); - res = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags); - if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file, - NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS))) - res = error_errno(_("could not write '%s'"), todo_file); - - todo_list_release(&todo_list); - todo_list_release(&new_todo); - - return res; -} - -static int get_revision_ranges(const char *upstream, const char *onto, - const char **head_hash, - char **revisions, char **shortrevisions) -{ - const char *base_rev = upstream ? upstream : onto, *shorthead; - struct object_id orig_head; - - if (get_oid("HEAD", &orig_head)) - return error(_("no HEAD?")); - - *head_hash = find_unique_abbrev(&orig_head, GIT_MAX_HEXSZ); - *revisions = xstrfmt("%s...%s", base_rev, *head_hash); - - shorthead = find_unique_abbrev(&orig_head, DEFAULT_ABBREV); - - if (upstream) { - const char *shortrev; - struct object_id rev_oid; - - get_oid(base_rev, &rev_oid); - shortrev = find_unique_abbrev(&rev_oid, DEFAULT_ABBREV); - - *shortrevisions = xstrfmt("%s..%s", shortrev, shorthead); - } else - *shortrevisions = xstrdup(shorthead); - - return 0; -} - -static int init_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head) -{ - FILE *interactive; - - if (!is_directory(path_state_dir()) && mkdir_in_gitdir(path_state_dir())) - return error_errno(_("could not create temporary %s"), path_state_dir()); - - delete_reflog("REBASE_HEAD"); - - interactive = fopen(path_interactive(), "w"); - if (!interactive) - return error_errno(_("could not mark as interactive")); - fclose(interactive); - - return write_basic_state(opts, head_name, onto, orig_head); -} - -static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, - const char *switch_to, const char *upstream, - const char *onto, const char *onto_name, - const char *squash_onto, const char *head_name, - const char *restrict_revision, char *raw_strategies, - struct string_list *commands, unsigned autosquash) -{ - int ret; - const char *head_hash = NULL; - char *revisions = NULL, *shortrevisions = NULL; - struct argv_array make_script_args = ARGV_ARRAY_INIT; - struct todo_list todo_list = TODO_LIST_INIT; - - if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) - return -1; - - if (get_revision_ranges(upstream, onto, &head_hash, - &revisions, &shortrevisions)) - return -1; - - if (raw_strategies) - parse_strategy_opts(opts, raw_strategies); - - if (init_basic_state(opts, head_name, onto, head_hash)) { - free(revisions); - free(shortrevisions); - - return -1; - } - - if (!upstream && squash_onto) - write_file(path_squash_onto(), "%s\n", squash_onto); - - argv_array_pushl(&make_script_args, "", revisions, NULL); - if (restrict_revision) - argv_array_push(&make_script_args, restrict_revision); - - ret = sequencer_make_script(the_repository, &todo_list.buf, - make_script_args.argc, make_script_args.argv, - flags); - - if (ret) - error(_("could not generate todo list")); - else { - discard_cache(); - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) - BUG("unusable todo list"); - - ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name, - onto, head_hash, commands, autosquash, &todo_list); - } - - free(revisions); - free(shortrevisions); - todo_list_release(&todo_list); - argv_array_clear(&make_script_args); - - return ret; -} - -static const char * const builtin_rebase_interactive_usage[] = { - N_("git rebase--interactive [<options>]"), - NULL -}; - -int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) -{ - struct replay_opts opts = REPLAY_OPTS_INIT; - unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; - int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL, - *squash_onto = NULL, *upstream = NULL, *head_name = NULL, - *switch_to = NULL, *cmd = NULL; - struct string_list commands = STRING_LIST_INIT_DUP; - char *raw_strategies = NULL; - enum { - NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, - SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC - } command = 0; - struct option options[] = { - OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), - OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")), - OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, - N_("allow commits with empty messages")), - OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")), - OPT_BOOL(0, "rebase-cousins", &rebase_cousins, - N_("keep original branch points of cousins")), - OPT_BOOL(0, "autosquash", &autosquash, - N_("move commits that begin with squash!/fixup!")), - OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")), - OPT__VERBOSE(&opts.verbose, N_("be verbose")), - OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), - CONTINUE), - OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), - OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), - EDIT_TODO), - OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), - SHOW_CURRENT_PATCH), - OPT_CMDMODE(0, "shorten-ids", &command, - N_("shorten commit ids in the todo list"), SHORTEN_OIDS), - OPT_CMDMODE(0, "expand-ids", &command, - N_("expand commit ids in the todo list"), EXPAND_OIDS), - OPT_CMDMODE(0, "check-todo-list", &command, - N_("check the todo list"), CHECK_TODO_LIST), - OPT_CMDMODE(0, "rearrange-squash", &command, - N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), - OPT_CMDMODE(0, "add-exec-commands", &command, - N_("insert exec commands in todo list"), ADD_EXEC), - OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")), - OPT_STRING(0, "restrict-revision", &restrict_revision, - N_("restrict-revision"), N_("restrict revision")), - OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), - N_("squash onto")), - OPT_STRING(0, "upstream", &upstream, N_("upstream"), - N_("the upstream commit")), - OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), - { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), - N_("GPG-sign commits"), - PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, - OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"), - N_("rebase strategy")), - OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"), - N_("strategy options")), - OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"), - N_("the branch or commit to checkout")), - OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")), - OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")), - OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto), - OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec, - N_("automatically re-schedule any `exec` that fails")), - OPT_END() - }; - - sequencer_init_config(&opts); - git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); - - opts.action = REPLAY_INTERACTIVE_REBASE; - opts.allow_ff = 1; - opts.allow_empty = 1; - - if (argc == 1) - usage_with_options(builtin_rebase_interactive_usage, options); - - argc = parse_options(argc, argv, NULL, options, - builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0); - - opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); - - flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; - flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; - flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; - flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; - flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; - - if (rebase_cousins >= 0 && !rebase_merges) - warning(_("--[no-]rebase-cousins has no effect without " - "--rebase-merges")); - - if (cmd && *cmd) { - string_list_split(&commands, cmd, '\n', -1); - - /* rebase.c adds a new line to cmd after every command, - * so here the last command is always empty */ - string_list_remove_empty_items(&commands, 0); - } - - switch (command) { - case NONE: - if (!onto && !upstream) - die(_("a base commit must be provided with --upstream or --onto")); - - ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, - onto_name, squash_onto, head_name, restrict_revision, - raw_strategies, &commands, autosquash); - break; - case SKIP: { - struct string_list merge_rr = STRING_LIST_INIT_DUP; - - rerere_clear(the_repository, &merge_rr); - /* fallthrough */ - case CONTINUE: - ret = sequencer_continue(the_repository, &opts); - break; - } - case EDIT_TODO: - ret = edit_todo_file(flags); - break; - case SHOW_CURRENT_PATCH: { - struct child_process cmd = CHILD_PROCESS_INIT; - - cmd.git_cmd = 1; - argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); - ret = run_command(&cmd); - - break; - } - case SHORTEN_OIDS: - case EXPAND_OIDS: - ret = transform_todo_file(flags); - break; - case CHECK_TODO_LIST: - ret = check_todo_list_from_file(the_repository); - break; - case REARRANGE_SQUASH: - ret = rearrange_squash_in_todo_file(); - break; - case ADD_EXEC: - ret = add_exec_commands(&commands); - break; - default: - BUG("invalid command '%d'", command); - } - - string_list_clear(&commands, 0); - return !!ret; -} diff --git a/builtin/rebase.c b/builtin/rebase.c index ccf178326c..80252d72df 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -25,6 +25,8 @@ #include "commit-reach.h" #include "rerere.h" #include "branch.h" +#include "sequencer.h" +#include "rebase-interactive.h" static char const * const builtin_rebase_usage[] = { N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] " @@ -35,6 +37,9 @@ static char const * const builtin_rebase_usage[] = { NULL }; +static GIT_PATH_FUNC(path_state_dir, "rebase-merge/") +static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto") +static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive") static GIT_PATH_FUNC(apply_dir, "rebase-apply") static GIT_PATH_FUNC(merge_dir, "rebase-merge") @@ -46,6 +51,368 @@ enum rebase_type { REBASE_PRESERVE_MERGES }; +static int add_exec_commands(struct string_list *commands) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT; + int res; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) { + todo_list_release(&todo_list); + return error(_("unusable todo list: '%s'"), todo_file); + } + + todo_list_add_exec_commands(&todo_list, commands); + res = todo_list_write_to_file(the_repository, &todo_list, + todo_file, NULL, NULL, -1, 0); + todo_list_release(&todo_list); + + if (res) + return error_errno(_("could not write '%s'."), todo_file); + return 0; +} + +static int rearrange_squash_in_todo_file(void) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT; + int res = 0; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) { + todo_list_release(&todo_list); + return error(_("unusable todo list: '%s'"), todo_file); + } + + res = todo_list_rearrange_squash(&todo_list); + if (!res) + res = todo_list_write_to_file(the_repository, &todo_list, + todo_file, NULL, NULL, -1, 0); + + todo_list_release(&todo_list); + + if (res) + return error_errno(_("could not write '%s'."), todo_file); + return 0; +} + +static int transform_todo_file(unsigned flags) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT; + int res; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) { + todo_list_release(&todo_list); + return error(_("unusable todo list: '%s'"), todo_file); + } + + res = todo_list_write_to_file(the_repository, &todo_list, todo_file, + NULL, NULL, -1, flags); + todo_list_release(&todo_list); + + if (res) + return error_errno(_("could not write '%s'."), todo_file); + return 0; +} + +static int edit_todo_file(unsigned flags) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT, + new_todo = TODO_LIST_INIT; + int res = 0; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + + strbuf_stripspace(&todo_list.buf, 1); + res = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags); + if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file, + NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS))) + res = error_errno(_("could not write '%s'"), todo_file); + + todo_list_release(&todo_list); + todo_list_release(&new_todo); + + return res; +} + +static int get_revision_ranges(const char *upstream, const char *onto, + const char **head_hash, + char **revisions, char **shortrevisions) +{ + const char *base_rev = upstream ? upstream : onto, *shorthead; + struct object_id orig_head; + + if (get_oid("HEAD", &orig_head)) + return error(_("no HEAD?")); + + *head_hash = find_unique_abbrev(&orig_head, GIT_MAX_HEXSZ); + *revisions = xstrfmt("%s...%s", base_rev, *head_hash); + + shorthead = find_unique_abbrev(&orig_head, DEFAULT_ABBREV); + + if (upstream) { + const char *shortrev; + struct object_id rev_oid; + + get_oid(base_rev, &rev_oid); + shortrev = find_unique_abbrev(&rev_oid, DEFAULT_ABBREV); + + *shortrevisions = xstrfmt("%s..%s", shortrev, shorthead); + } else + *shortrevisions = xstrdup(shorthead); + + return 0; +} + +static int init_basic_state(struct replay_opts *opts, const char *head_name, + const char *onto, const char *orig_head) +{ + FILE *interactive; + + if (!is_directory(path_state_dir()) && mkdir_in_gitdir(path_state_dir())) + return error_errno(_("could not create temporary %s"), path_state_dir()); + + delete_reflog("REBASE_HEAD"); + + interactive = fopen(path_interactive(), "w"); + if (!interactive) + return error_errno(_("could not mark as interactive")); + fclose(interactive); + + return write_basic_state(opts, head_name, onto, orig_head); +} + +static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, + const char *switch_to, const char *upstream, + const char *onto, const char *onto_name, + const char *squash_onto, const char *head_name, + const char *restrict_revision, char *raw_strategies, + struct string_list *commands, unsigned autosquash) +{ + int ret; + const char *head_hash = NULL; + char *revisions = NULL, *shortrevisions = NULL; + struct argv_array make_script_args = ARGV_ARRAY_INIT; + struct todo_list todo_list = TODO_LIST_INIT; + + if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) + return -1; + + if (get_revision_ranges(upstream, onto, &head_hash, + &revisions, &shortrevisions)) + return -1; + + if (raw_strategies) + parse_strategy_opts(opts, raw_strategies); + + if (init_basic_state(opts, head_name, onto, head_hash)) { + free(revisions); + free(shortrevisions); + + return -1; + } + + if (!upstream && squash_onto) + write_file(path_squash_onto(), "%s\n", squash_onto); + + argv_array_pushl(&make_script_args, "", revisions, NULL); + if (restrict_revision) + argv_array_push(&make_script_args, restrict_revision); + + ret = sequencer_make_script(the_repository, &todo_list.buf, + make_script_args.argc, make_script_args.argv, + flags); + + if (ret) + error(_("could not generate todo list")); + else { + discard_cache(); + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) + BUG("unusable todo list"); + + ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name, + onto, head_hash, commands, autosquash, &todo_list); + } + + free(revisions); + free(shortrevisions); + todo_list_release(&todo_list); + argv_array_clear(&make_script_args); + + return ret; +} + +static const char * const builtin_rebase_interactive_usage[] = { + N_("git rebase--interactive [<options>]"), + NULL +}; + +int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) +{ + struct replay_opts opts = REPLAY_OPTS_INIT; + unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; + int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; + const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL, + *squash_onto = NULL, *upstream = NULL, *head_name = NULL, + *switch_to = NULL, *cmd = NULL; + struct string_list commands = STRING_LIST_INIT_DUP; + char *raw_strategies = NULL; + enum { + NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, + SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC + } command = 0; + struct option options[] = { + OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), + OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")), + OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, + N_("allow commits with empty messages")), + OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")), + OPT_BOOL(0, "rebase-cousins", &rebase_cousins, + N_("keep original branch points of cousins")), + OPT_BOOL(0, "autosquash", &autosquash, + N_("move commits that begin with squash!/fixup!")), + OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")), + OPT__VERBOSE(&opts.verbose, N_("be verbose")), + OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), + CONTINUE), + OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), + OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), + EDIT_TODO), + OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), + SHOW_CURRENT_PATCH), + OPT_CMDMODE(0, "shorten-ids", &command, + N_("shorten commit ids in the todo list"), SHORTEN_OIDS), + OPT_CMDMODE(0, "expand-ids", &command, + N_("expand commit ids in the todo list"), EXPAND_OIDS), + OPT_CMDMODE(0, "check-todo-list", &command, + N_("check the todo list"), CHECK_TODO_LIST), + OPT_CMDMODE(0, "rearrange-squash", &command, + N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), + OPT_CMDMODE(0, "add-exec-commands", &command, + N_("insert exec commands in todo list"), ADD_EXEC), + OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")), + OPT_STRING(0, "restrict-revision", &restrict_revision, + N_("restrict-revision"), N_("restrict revision")), + OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), + N_("squash onto")), + OPT_STRING(0, "upstream", &upstream, N_("upstream"), + N_("the upstream commit")), + OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), + { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), + N_("GPG-sign commits"), + PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"), + N_("rebase strategy")), + OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"), + N_("strategy options")), + OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"), + N_("the branch or commit to checkout")), + OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")), + OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")), + OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto), + OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec, + N_("automatically re-schedule any `exec` that fails")), + OPT_END() + }; + + sequencer_init_config(&opts); + git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); + + opts.action = REPLAY_INTERACTIVE_REBASE; + opts.allow_ff = 1; + opts.allow_empty = 1; + + if (argc == 1) + usage_with_options(builtin_rebase_interactive_usage, options); + + argc = parse_options(argc, argv, NULL, options, + builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0); + + opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); + + flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; + flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; + flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; + flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; + + if (rebase_cousins >= 0 && !rebase_merges) + warning(_("--[no-]rebase-cousins has no effect without " + "--rebase-merges")); + + if (cmd && *cmd) { + string_list_split(&commands, cmd, '\n', -1); + + /* rebase.c adds a new line to cmd after every command, + * so here the last command is always empty */ + string_list_remove_empty_items(&commands, 0); + } + + switch (command) { + case NONE: + if (!onto && !upstream) + die(_("a base commit must be provided with --upstream or --onto")); + + ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, + onto_name, squash_onto, head_name, restrict_revision, + raw_strategies, &commands, autosquash); + break; + case SKIP: { + struct string_list merge_rr = STRING_LIST_INIT_DUP; + + rerere_clear(the_repository, &merge_rr); + /* fallthrough */ + case CONTINUE: + ret = sequencer_continue(the_repository, &opts); + break; + } + case EDIT_TODO: + ret = edit_todo_file(flags); + break; + case SHOW_CURRENT_PATCH: { + struct child_process cmd = CHILD_PROCESS_INIT; + + cmd.git_cmd = 1; + argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); + ret = run_command(&cmd); + + break; + } + case SHORTEN_OIDS: + case EXPAND_OIDS: + ret = transform_todo_file(flags); + break; + case CHECK_TODO_LIST: + ret = check_todo_list_from_file(the_repository); + break; + case REARRANGE_SQUASH: + ret = rearrange_squash_in_todo_file(); + break; + case ADD_EXEC: + ret = add_exec_commands(&commands); + break; + default: + BUG("invalid command '%d'", command); + } + + string_list_clear(&commands, 0); + return !!ret; +} + static int use_builtin_rebase(void) { struct child_process cp = CHILD_PROCESS_INIT; -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 05/11] rebase -i: remove duplication 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (3 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 04/11] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 06/11] rebase -i: use struct commit when parsing options Phillip Wood ` (9 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> path_state_dir() and merge_dir() refer to the same path so remove one of them. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 80252d72df..e55bd0348a 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -37,7 +37,6 @@ static char const * const builtin_rebase_usage[] = { NULL }; -static GIT_PATH_FUNC(path_state_dir, "rebase-merge/") static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto") static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive") static GIT_PATH_FUNC(apply_dir, "rebase-apply") @@ -182,8 +181,8 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, { FILE *interactive; - if (!is_directory(path_state_dir()) && mkdir_in_gitdir(path_state_dir())) - return error_errno(_("could not create temporary %s"), path_state_dir()); + if (!is_directory(merge_dir()) && mkdir_in_gitdir(merge_dir())) + return error_errno(_("could not create temporary %s"), merge_dir()); delete_reflog("REBASE_HEAD"); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 06/11] rebase -i: use struct commit when parsing options 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (4 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 05/11] rebase -i: remove duplication Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 07/11] rebase -i: use struct object_id for squash_onto Phillip Wood ` (8 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> This is in preparation for using `struct rebase_options` when parsing options in cmd_rebase__interactive(). Using a string for onto, restrict_revision and upstream, was a hangover from the scripted version of rebase. The functions that use these variables are updated to take a `struct commit`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- Notes: I'm not sure if parse_opt_commit() should be in parse-options-cb.c or not, it depends if it is going to be useful elsewhere. builtin/rebase.c | 42 ++++++++++++++++++++++++------------------ parse-options-cb.c | 17 +++++++++++++++++ parse-options.h | 1 + sequencer.c | 21 +++++++++++---------- sequencer.h | 4 ++-- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index e55bd0348a..0584b331bf 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -147,27 +147,28 @@ static int edit_todo_file(unsigned flags) return res; } -static int get_revision_ranges(const char *upstream, const char *onto, +static int get_revision_ranges(struct commit *upstream, struct commit *onto, const char **head_hash, char **revisions, char **shortrevisions) { - const char *base_rev = upstream ? upstream : onto, *shorthead; + struct commit *base_rev = upstream ? upstream : onto; + const char *shorthead; struct object_id orig_head; if (get_oid("HEAD", &orig_head)) return error(_("no HEAD?")); *head_hash = find_unique_abbrev(&orig_head, GIT_MAX_HEXSZ); - *revisions = xstrfmt("%s...%s", base_rev, *head_hash); + *revisions = xstrfmt("%s...%s", oid_to_hex(&base_rev->object.oid), + *head_hash); shorthead = find_unique_abbrev(&orig_head, DEFAULT_ABBREV); if (upstream) { const char *shortrev; - struct object_id rev_oid; - get_oid(base_rev, &rev_oid); - shortrev = find_unique_abbrev(&rev_oid, DEFAULT_ABBREV); + shortrev = find_unique_abbrev(&base_rev->object.oid, + DEFAULT_ABBREV); *shortrevisions = xstrfmt("%s..%s", shortrev, shorthead); } else @@ -177,7 +178,7 @@ static int get_revision_ranges(const char *upstream, const char *onto, } static int init_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head) + struct commit *onto, const char *orig_head) { FILE *interactive; @@ -195,10 +196,10 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, } static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, - const char *switch_to, const char *upstream, - const char *onto, const char *onto_name, + const char *switch_to, struct commit *upstream, + struct commit *onto, const char *onto_name, const char *squash_onto, const char *head_name, - const char *restrict_revision, char *raw_strategies, + struct commit *restrict_revision, char *raw_strategies, struct string_list *commands, unsigned autosquash) { int ret; @@ -229,7 +230,8 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, argv_array_pushl(&make_script_args, "", revisions, NULL); if (restrict_revision) - argv_array_push(&make_script_args, restrict_revision); + argv_array_push(&make_script_args, + oid_to_hex(&restrict_revision->object.oid)); ret = sequencer_make_script(the_repository, &todo_list.buf, make_script_args.argc, make_script_args.argv, @@ -265,9 +267,10 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) struct replay_opts opts = REPLAY_OPTS_INIT; unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL, - *squash_onto = NULL, *upstream = NULL, *head_name = NULL, + const char *onto_name = NULL, + *squash_onto = NULL, *head_name = NULL, *switch_to = NULL, *cmd = NULL; + struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL; struct string_list commands = STRING_LIST_INIT_DUP; char *raw_strategies = NULL; enum { @@ -303,13 +306,16 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), OPT_CMDMODE(0, "add-exec-commands", &command, N_("insert exec commands in todo list"), ADD_EXEC), - OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")), - OPT_STRING(0, "restrict-revision", &restrict_revision, - N_("restrict-revision"), N_("restrict revision")), + { OPTION_CALLBACK, 0, "onto", &onto, N_("onto"), N_("onto"), + PARSE_OPT_NONEG, parse_opt_commit, 0 }, + { OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision, + N_("restrict-revision"), N_("restrict revision"), + PARSE_OPT_NONEG, parse_opt_commit, 0 }, OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), N_("squash onto")), - OPT_STRING(0, "upstream", &upstream, N_("upstream"), - N_("the upstream commit")), + { OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"), + N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit, + 0 }, OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), N_("GPG-sign commits"), diff --git a/parse-options-cb.c b/parse-options-cb.c index 2733393546..2206eb763c 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -96,6 +96,23 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_commit(const struct option *opt, const char *arg, int unset) +{ + struct object_id oid; + struct commit *commit; + struct commit **target = opt->value; + + if (!arg) + return -1; + if (get_oid(arg, &oid)) + return error("malformed object name %s", arg); + commit = lookup_commit_reference(the_repository, &oid); + if (!commit) + return error("no such commit %s", arg); + *target = commit; + return 0; +} + int parse_opt_object_name(const struct option *opt, const char *arg, int unset) { struct object_id oid; diff --git a/parse-options.h b/parse-options.h index 7d83e2971d..5a75646618 100644 --- a/parse-options.h +++ b/parse-options.h @@ -266,6 +266,7 @@ int parse_opt_color_flag_cb(const struct option *, const char *, int); int parse_opt_verbosity_cb(const struct option *, const char *, int); int parse_opt_object_name(const struct option *, const char *, int); int parse_opt_commits(const struct option *, const char *, int); +int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); diff --git a/sequencer.c b/sequencer.c index ccc0160800..610b7ece14 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2418,14 +2418,15 @@ static void write_strategy_opts(struct replay_opts *opts) } int write_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head) + struct commit *onto, const char *orig_head) { const char *quiet = getenv("GIT_QUIET"); if (head_name) write_file(rebase_path_head_name(), "%s\n", head_name); if (onto) - write_file(rebase_path_onto(), "%s\n", onto); + write_file(rebase_path_onto(), "%s\n", + oid_to_hex(&onto->object.oid)); if (orig_head) write_file(rebase_path_orig_head(), "%s\n", orig_head); @@ -3456,7 +3457,7 @@ int prepare_branch_to_be_rebased(struct repository *r, struct replay_opts *opts, } static int checkout_onto(struct repository *r, struct replay_opts *opts, - const char *onto_name, const char *onto, + const char *onto_name, const struct object_id *onto, const char *orig_head) { struct object_id oid; @@ -3465,7 +3466,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts, if (get_oid(orig_head, &oid)) return error(_("%s: not a valid OID"), orig_head); - if (run_git_checkout(r, opts, onto, action)) { + if (run_git_checkout(r, opts, oid_to_hex(onto), action)) { apply_autostash(opts); sequencer_remove_state(opts); return error(_("could not detach HEAD")); @@ -4741,16 +4742,16 @@ static int skip_unnecessary_picks(struct repository *r, int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags, const char *shortrevisions, const char *onto_name, - const char *onto, const char *orig_head, struct string_list *commands, - unsigned autosquash, struct todo_list *todo_list) + struct commit *onto, const char *orig_head, + struct string_list *commands, unsigned autosquash, + struct todo_list *todo_list) { const char *shortonto, *todo_file = rebase_path_todo(); struct todo_list new_todo = TODO_LIST_INIT; struct strbuf *buf = &todo_list->buf; - struct object_id oid; + struct object_id oid = onto->object.oid; int res; - get_oid(onto, &oid); shortonto = find_unique_abbrev(&oid, DEFAULT_ABBREV); if (buf->len == 0) { @@ -4793,7 +4794,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) || todo_list_check(todo_list, &new_todo)) { fprintf(stderr, _(edit_todo_list_advice)); - checkout_onto(r, opts, onto_name, onto, orig_head); + checkout_onto(r, opts, onto_name, &onto->object.oid, orig_head); todo_list_release(&new_todo); return -1; @@ -4812,7 +4813,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla todo_list_release(&new_todo); - if (checkout_onto(r, opts, onto_name, oid_to_hex(&oid), orig_head)) + if (checkout_onto(r, opts, onto_name, &oid, orig_head)) return -1; if (require_clean_work_tree(r, "rebase", "", 1, 1)) diff --git a/sequencer.h b/sequencer.h index 6c55aa4200..e640ca21f2 100644 --- a/sequencer.h +++ b/sequencer.h @@ -150,7 +150,7 @@ void todo_list_add_exec_commands(struct todo_list *todo_list, int check_todo_list_from_file(struct repository *r); int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags, const char *shortrevisions, const char *onto_name, - const char *onto, const char *orig_head, struct string_list *commands, + struct commit *onto, const char *orig_head, struct string_list *commands, unsigned autosquash, struct todo_list *todo_list); int todo_list_rearrange_squash(struct todo_list *todo_list); @@ -191,4 +191,4 @@ int read_author_script(const char *path, char **name, char **email, char **date, void parse_strategy_opts(struct replay_opts *opts, char *raw_opts); int write_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head); + struct commit *onto, const char *orig_head); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 07/11] rebase -i: use struct object_id for squash_onto 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (5 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 06/11] rebase -i: use struct commit when parsing options Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args Phillip Wood ` (7 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> More preparation for using `struct rebase_options` in cmd_rebase__interactive(). Using a string was a hangover from the scripted version of rebase, update the functions that use `squash_onto` to take a `sturct object_id`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 21 +++++++++++++-------- parse-options-cb.c | 17 +++++++++++++++++ parse-options.h | 3 +++ 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 0584b331bf..c93f2aa629 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -198,7 +198,7 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, const char *switch_to, struct commit *upstream, struct commit *onto, const char *onto_name, - const char *squash_onto, const char *head_name, + struct object_id *squash_onto, const char *head_name, struct commit *restrict_revision, char *raw_strategies, struct string_list *commands, unsigned autosquash) { @@ -226,7 +226,8 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, } if (!upstream && squash_onto) - write_file(path_squash_onto(), "%s\n", squash_onto); + write_file(path_squash_onto(), "%s\n", + oid_to_hex(squash_onto)); argv_array_pushl(&make_script_args, "", revisions, NULL); if (restrict_revision) @@ -267,10 +268,11 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) struct replay_opts opts = REPLAY_OPTS_INIT; unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto_name = NULL, - *squash_onto = NULL, *head_name = NULL, - *switch_to = NULL, *cmd = NULL; + const char *onto_name = NULL, *head_name = NULL, *switch_to = NULL, + *cmd = NULL; struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL; + struct object_id squash_onto = null_oid; + struct object_id *squash_onto_opt = NULL; struct string_list commands = STRING_LIST_INIT_DUP; char *raw_strategies = NULL; enum { @@ -311,8 +313,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) { OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision, N_("restrict-revision"), N_("restrict revision"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, - OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), - N_("squash onto")), + { OPTION_CALLBACK, 0, "squash-onto", &squash_onto, N_("squash-onto"), + N_("squash onto"), PARSE_OPT_NONEG, parse_opt_object_id, 0 }, { OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"), N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, @@ -349,6 +351,9 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); + if (!is_null_oid(&squash_onto)) + squash_onto_opt = &squash_onto; + flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; @@ -373,7 +378,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) die(_("a base commit must be provided with --upstream or --onto")); ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, - onto_name, squash_onto, head_name, restrict_revision, + onto_name, squash_onto_opt, head_name, restrict_revision, raw_strategies, &commands, autosquash); break; case SKIP: { diff --git a/parse-options-cb.c b/parse-options-cb.c index 2206eb763c..28ad5cd94b 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -129,6 +129,23 @@ int parse_opt_object_name(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_object_id(const struct option *opt, const char *arg, int unset) +{ + struct object_id oid; + struct object_id *target = opt->value; + + if (unset) { + *target = null_oid; + return 0; + } + if (!arg) + return -1; + if (get_oid(arg, &oid)) + return error(_("malformed object name '%s'"), arg); + *target = oid; + return 0; +} + int parse_opt_tertiary(const struct option *opt, const char *arg, int unset) { int *target = opt->value; diff --git a/parse-options.h b/parse-options.h index 5a75646618..0ab1103bc7 100644 --- a/parse-options.h +++ b/parse-options.h @@ -264,7 +264,10 @@ int parse_opt_abbrev_cb(const struct option *, const char *, int); int parse_opt_expiry_date_cb(const struct option *, const char *, int); int parse_opt_color_flag_cb(const struct option *, const char *, int); int parse_opt_verbosity_cb(const struct option *, const char *, int); +/* value is struct oid_array* */ int parse_opt_object_name(const struct option *, const char *, int); +/* value is struct object_id* */ +int parse_opt_object_id(const struct option *, const char *, int); int parse_opt_commits(const struct option *, const char *, int); int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (6 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 07/11] rebase -i: use struct object_id for squash_onto Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-21 4:21 ` Junio C Hamano 2019-03-21 21:13 ` Alban Gruin 2019-03-19 19:03 ` [RFC PATCH 09/11] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood ` (6 subsequent siblings) 14 siblings, 2 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> In order to run `rebase -i` without forking `rebase--interactive` it will be convenient to use the same structure when parsing the options in cmd_rebase() and cmd_rebase__interactive(). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 203 ++++++++++++++++++++++++++--------------------- 1 file changed, 112 insertions(+), 91 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index c93f2aa629..33a2495032 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -50,6 +50,73 @@ enum rebase_type { REBASE_PRESERVE_MERGES }; +struct rebase_options { + enum rebase_type type; + const char *state_dir; + struct commit *upstream; + const char *upstream_name; + const char *upstream_arg; + char *head_name; + struct object_id orig_head; + struct commit *onto; + const char *onto_name; + const char *revisions; + const char *switch_to; + int root; + struct object_id *squash_onto; + struct commit *restrict_revision; + int dont_finish_rebase; + enum { + REBASE_NO_QUIET = 1<<0, + REBASE_VERBOSE = 1<<1, + REBASE_DIFFSTAT = 1<<2, + REBASE_FORCE = 1<<3, + REBASE_INTERACTIVE_EXPLICIT = 1<<4, + } flags; + struct argv_array git_am_opts; + const char *action; + int signoff; + int allow_rerere_autoupdate; + int keep_empty; + int autosquash; + char *gpg_sign_opt; + int autostash; + char *cmd; + int allow_empty_message; + int rebase_merges, rebase_cousins; + char *strategy, *strategy_opts; + struct strbuf git_format_patch_opt; + int reschedule_failed_exec; +}; + +#define REBASE_OPTIONS_INIT { \ + .type = REBASE_UNSPECIFIED, \ + .flags = REBASE_NO_QUIET, \ + .git_am_opts = ARGV_ARRAY_INIT, \ + .git_format_patch_opt = STRBUF_INIT \ + } + +static struct replay_opts get_replay_opts(const struct rebase_options *opts) +{ + struct replay_opts replay = REPLAY_OPTS_INIT; + + sequencer_init_config(&replay); + + replay.action = REPLAY_INTERACTIVE_REBASE; + replay.signoff = opts->signoff; + replay.allow_ff = !(opts->flags & REBASE_FORCE); + if (opts->allow_rerere_autoupdate) + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; + replay.allow_empty = 1; + replay.allow_empty_message = opts->allow_empty_message; + replay.verbose = opts->flags & REBASE_VERBOSE; + replay.reschedule_failed_exec = opts->reschedule_failed_exec; + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); + replay.strategy = opts->strategy; + + return replay; +} + static int add_exec_commands(struct string_list *commands) { const char *todo_file = rebase_path_todo(); @@ -265,32 +332,30 @@ static const char * const builtin_rebase_interactive_usage[] = { int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) { - struct replay_opts opts = REPLAY_OPTS_INIT; - unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; - int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto_name = NULL, *head_name = NULL, *switch_to = NULL, - *cmd = NULL; - struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL; + struct rebase_options opts = REBASE_OPTIONS_INIT; + unsigned flags = 0; + int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; - struct object_id *squash_onto_opt = NULL; struct string_list commands = STRING_LIST_INIT_DUP; - char *raw_strategies = NULL; enum { NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC } command = 0; struct option options[] = { - OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), - OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")), + OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), + REBASE_FORCE), + OPT_BOOL(0, "keep-empty", &opts.keep_empty, N_("keep empty commits")), OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, N_("allow commits with empty messages")), - OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")), - OPT_BOOL(0, "rebase-cousins", &rebase_cousins, + OPT_BOOL(0, "rebase-merges", &opts.rebase_merges, N_("rebase merge commits")), + OPT_BOOL(0, "rebase-cousins", &opts.rebase_cousins, N_("keep original branch points of cousins")), - OPT_BOOL(0, "autosquash", &autosquash, + OPT_BOOL(0, "autosquash", &opts.autosquash, N_("move commits that begin with squash!/fixup!")), OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")), - OPT__VERBOSE(&opts.verbose, N_("be verbose")), + OPT_BIT('v', "verbose", &opts.flags, + N_("display a diffstat of what changed upstream"), + REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), CONTINUE), OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), @@ -308,86 +373,86 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), OPT_CMDMODE(0, "add-exec-commands", &command, N_("insert exec commands in todo list"), ADD_EXEC), - { OPTION_CALLBACK, 0, "onto", &onto, N_("onto"), N_("onto"), + { OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, - { OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision, + { OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision, N_("restrict-revision"), N_("restrict revision"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, { OPTION_CALLBACK, 0, "squash-onto", &squash_onto, N_("squash-onto"), N_("squash onto"), PARSE_OPT_NONEG, parse_opt_object_id, 0 }, - { OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"), + { OPTION_CALLBACK, 0, "upstream", &opts.upstream, N_("upstream"), N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, - OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), - { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), + OPT_STRING(0, "head-name", &opts.head_name, N_("head-name"), N_("head name")), + { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"), N_("GPG-sign commits"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"), N_("rebase strategy")), - OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"), + OPT_STRING(0, "strategy-opts", &opts.strategy_opts, N_("strategy-opts"), N_("strategy options")), - OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"), + OPT_STRING(0, "switch-to", &opts.switch_to, N_("switch-to"), N_("the branch or commit to checkout")), - OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")), - OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")), - OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto), + OPT_STRING(0, "onto-name", &opts.onto_name, N_("onto-name"), N_("onto name")), + OPT_STRING(0, "cmd", &opts.cmd, N_("cmd"), N_("the command to run")), + OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_autoupdate), OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec, N_("automatically re-schedule any `exec` that fails")), OPT_END() }; - sequencer_init_config(&opts); + opts.rebase_cousins = -1; + git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); - opts.action = REPLAY_INTERACTIVE_REBASE; - opts.allow_ff = 1; - opts.allow_empty = 1; - if (argc == 1) usage_with_options(builtin_rebase_interactive_usage, options); argc = parse_options(argc, argv, NULL, options, builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0); - opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); - if (!is_null_oid(&squash_onto)) - squash_onto_opt = &squash_onto; + opts.squash_onto = &squash_onto; - flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; + flags |= opts.keep_empty ? TODO_LIST_KEEP_EMPTY : 0; flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; - flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; - flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; + flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; - if (rebase_cousins >= 0 && !rebase_merges) + if (opts.rebase_cousins >= 0 && !opts.rebase_merges) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - if (cmd && *cmd) { - string_list_split(&commands, cmd, '\n', -1); + if (opts.cmd && *opts.cmd) { + string_list_split(&commands, opts.cmd, '\n', -1); /* rebase.c adds a new line to cmd after every command, * so here the last command is always empty */ string_list_remove_empty_items(&commands, 0); } switch (command) { - case NONE: - if (!onto && !upstream) + case NONE: { + struct replay_opts replay_opts = get_replay_opts(&opts); + if (!opts.onto && !opts.upstream) die(_("a base commit must be provided with --upstream or --onto")); - ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, - onto_name, squash_onto_opt, head_name, restrict_revision, - raw_strategies, &commands, autosquash); + ret = do_interactive_rebase(&replay_opts, flags, opts.switch_to, opts.upstream, opts.onto, + opts.onto_name, opts.squash_onto, opts.head_name, opts.restrict_revision, + opts.strategy_opts, &commands, opts.autosquash); break; + } case SKIP: { struct string_list merge_rr = STRING_LIST_INIT_DUP; rerere_clear(the_repository, &merge_rr); + } /* fallthrough */ - case CONTINUE: - ret = sequencer_continue(the_repository, &opts); + case CONTINUE: { + struct replay_opts replay_opts = get_replay_opts(&opts); + + ret = sequencer_continue(the_repository, &replay_opts); break; } case EDIT_TODO: @@ -446,45 +511,6 @@ static int use_builtin_rebase(void) return ret; } -struct rebase_options { - enum rebase_type type; - const char *state_dir; - struct commit *upstream; - const char *upstream_name; - const char *upstream_arg; - char *head_name; - struct object_id orig_head; - struct commit *onto; - const char *onto_name; - const char *revisions; - const char *switch_to; - int root; - struct object_id *squash_onto; - struct commit *restrict_revision; - int dont_finish_rebase; - enum { - REBASE_NO_QUIET = 1<<0, - REBASE_VERBOSE = 1<<1, - REBASE_DIFFSTAT = 1<<2, - REBASE_FORCE = 1<<3, - REBASE_INTERACTIVE_EXPLICIT = 1<<4, - } flags; - struct argv_array git_am_opts; - const char *action; - int signoff; - int allow_rerere_autoupdate; - int keep_empty; - int autosquash; - char *gpg_sign_opt; - int autostash; - char *cmd; - int allow_empty_message; - int rebase_merges, rebase_cousins; - char *strategy, *strategy_opts; - struct strbuf git_format_patch_opt; - int reschedule_failed_exec; -}; - static int is_interactive(struct rebase_options *opts) { return opts->type == REBASE_INTERACTIVE || @@ -1380,13 +1406,7 @@ static int check_exec_cmd(const char *cmd) int cmd_rebase(int argc, const char **argv, const char *prefix) { - struct rebase_options options = { - .type = REBASE_UNSPECIFIED, - .flags = REBASE_NO_QUIET, - .git_am_opts = ARGV_ARRAY_INIT, - .allow_empty_message = 1, - .git_format_patch_opt = STRBUF_INIT, - }; + struct rebase_options options = REBASE_OPTIONS_INIT; const char *branch_name; int ret, flags, total_argc, in_progress = 0; int ok_to_skip_pre_rebase = 0; @@ -1540,6 +1560,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) trace_repo_setup(prefix); setup_work_tree(); + options.allow_empty_message = 1; git_config(rebase_config, &options); strbuf_reset(&buf); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args 2019-03-19 19:03 ` [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args Phillip Wood @ 2019-03-21 4:21 ` Junio C Hamano 2019-03-21 14:59 ` Phillip Wood 2019-03-21 21:13 ` Alban Gruin 1 sibling, 1 reply; 40+ messages in thread From: Junio C Hamano @ 2019-03-21 4:21 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood Phillip Wood <phillip.wood123@gmail.com> writes: > +static struct replay_opts get_replay_opts(const struct rebase_options *opts) > +{ > + struct replay_opts replay = REPLAY_OPTS_INIT; > + > + sequencer_init_config(&replay); > + > + replay.action = REPLAY_INTERACTIVE_REBASE; > + replay.signoff = opts->signoff; > + replay.allow_ff = !(opts->flags & REBASE_FORCE); > + if (opts->allow_rerere_autoupdate) > + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; > + replay.allow_empty = 1; > + replay.allow_empty_message = opts->allow_empty_message; > + replay.verbose = opts->flags & REBASE_VERBOSE; > + replay.reschedule_failed_exec = opts->reschedule_failed_exec; > + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); > + replay.strategy = opts->strategy; > + > + return replay; > +} This calls init_config() and then sets .action; does it revert to what dl/merge-cleanup-scissors-fix wants to do, which flipped the order to fix some bug? It is a bit hard to tell. Unfortunately because of the earlier huge code movement the changes to _this_ file does not conflict and cleanly merges, but because the other file is removed by this series while a topic in flight updates it, the semantic conflict like this luckily gets discovered. Especially since this is still an RFC, I'd preferred to see it without moving around the code too much (instead, exporting some symbols that need to be visible with each other after renaming them to more appropriate names that are fit in the global namespace). ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args 2019-03-21 4:21 ` Junio C Hamano @ 2019-03-21 14:59 ` Phillip Wood 2019-03-22 3:34 ` Junio C Hamano 0 siblings, 1 reply; 40+ messages in thread From: Phillip Wood @ 2019-03-21 14:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood Hi Junio On 21/03/2019 04:21, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > >> +static struct replay_opts get_replay_opts(const struct rebase_options *opts) >> +{ >> + struct replay_opts replay = REPLAY_OPTS_INIT; >> + >> + sequencer_init_config(&replay); >> + >> + replay.action = REPLAY_INTERACTIVE_REBASE; >> + replay.signoff = opts->signoff; >> + replay.allow_ff = !(opts->flags & REBASE_FORCE); >> + if (opts->allow_rerere_autoupdate) >> + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; >> + replay.allow_empty = 1; >> + replay.allow_empty_message = opts->allow_empty_message; >> + replay.verbose = opts->flags & REBASE_VERBOSE; >> + replay.reschedule_failed_exec = opts->reschedule_failed_exec; >> + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); >> + replay.strategy = opts->strategy; >> + >> + return replay; >> +} > > This calls init_config() and then sets .action; does it revert to > what dl/merge-cleanup-scissors-fix wants to do, which flipped the > order to fix some bug? It is a bit hard to tell. dl/merge-cleanup-scissors-fix changes sequencer_init_config() to depend on the value of action so action should to be set first. What is in pu at the moment in not quite right (though I'm not sure what the practical implications are as it looks like the rebase tests are passing[1]). [1] https://travis-ci.org/git/git/jobs/509332448 > Unfortunately because of the earlier huge code movement the changes > to _this_ file does not conflict and cleanly merges, but because the > other file is removed by this series while a topic in flight updates > it, the semantic conflict like this luckily gets discovered. > > Especially since this is still an RFC, I'd preferred to see it > without moving around the code too much (instead, exporting some > symbols that need to be visible with each other after renaming them > to more appropriate names that are fit in the global namespace). I'm happy to try doing that, though the textual conflict would be in a different place in the file to the semantic conflict but at least they'd both be in the same file. I think it would only need to share the definitions of struct rebase_options, enum rebase_action and the declaration of run_rebase_interactive(). Would you be happy with the addition of builtin/rebase.h (there don't seem to be another headers in that directory). We could leave rebase--interactive.c around until rebase--preserve-merges.sh is finally removed. Best Wishes Phillip ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args 2019-03-21 14:59 ` Phillip Wood @ 2019-03-22 3:34 ` Junio C Hamano 0 siblings, 0 replies; 40+ messages in thread From: Junio C Hamano @ 2019-03-22 3:34 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood Phillip Wood <phillip.wood123@gmail.com> writes: > Hi Junio > > On 21/03/2019 04:21, Junio C Hamano wrote: >> Phillip Wood <phillip.wood123@gmail.com> writes: >> >>> +static struct replay_opts get_replay_opts(const struct rebase_options *opts) >>> +{ >>> + struct replay_opts replay = REPLAY_OPTS_INIT; >>> + >>> + sequencer_init_config(&replay); >>> + >>> + replay.action = REPLAY_INTERACTIVE_REBASE; >>> + replay.signoff = opts->signoff; >>> + replay.allow_ff = !(opts->flags & REBASE_FORCE); >>> + if (opts->allow_rerere_autoupdate) >>> + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; >>> + replay.allow_empty = 1; >>> + replay.allow_empty_message = opts->allow_empty_message; >>> + replay.verbose = opts->flags & REBASE_VERBOSE; >>> + replay.reschedule_failed_exec = opts->reschedule_failed_exec; >>> + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); >>> + replay.strategy = opts->strategy; >>> + >>> + return replay; >>> +} >> >> This calls init_config() and then sets .action; does it revert to >> what dl/merge-cleanup-scissors-fix wants to do, which flipped the >> order to fix some bug? It is a bit hard to tell. > > dl/merge-cleanup-scissors-fix changes sequencer_init_config() to > depend on the value of action so action should to be set first. What > is in pu at the moment in not quite right (though I'm not sure what > the practical implications are as it looks like the rebase tests are > passing[1]). > > [1] https://travis-ci.org/git/git/jobs/509332448 Yes, it actually is the reason why I left the seemingly-wrong merge in 'pu' on purpose to see if we have enough test coverage. > they'd both be in the same file. I think it would only need to share > the definitions of struct rebase_options, enum rebase_action and the > declaration of run_rebase_interactive(). Would you be happy with the > addition of builtin/rebase.h (there don't seem to be another headers > in that directory). We could leave rebase--interactive.c around until > rebase--preserve-merges.sh is finally removed. I think dl/merge-cleanup-scissors-fix is getting solid enough, so a better alternative may be to base this on top of reset e902e9bcae ;# The second batch merge ag/sequencer-reduce-rewriting-todo merge dl/merge-cleanup-scissors-fix instead of basing it only on ag/sequencer-reduce-rewriting-todo. Thanks. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args 2019-03-19 19:03 ` [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args Phillip Wood 2019-03-21 4:21 ` Junio C Hamano @ 2019-03-21 21:13 ` Alban Gruin 2019-04-10 19:16 ` Phillip Wood 1 sibling, 1 reply; 40+ messages in thread From: Alban Gruin @ 2019-03-21 21:13 UTC (permalink / raw) To: Phillip Wood, Git Mailing List; +Cc: Johannes Schindelin Hi Phillip, It’s nice to see your work on this on the list. Le 19/03/2019 à 20:03, Phillip Wood a écrit : > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > In order to run `rebase -i` without forking `rebase--interactive` it > will be convenient to use the same structure when parsing the options in > cmd_rebase() and cmd_rebase__interactive(). > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > builtin/rebase.c | 203 ++++++++++++++++++++++++++--------------------- > 1 file changed, 112 insertions(+), 91 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index c93f2aa629..33a2495032 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -50,6 +50,73 @@ enum rebase_type { > REBASE_PRESERVE_MERGES > }; > > +struct rebase_options { > + enum rebase_type type; > + const char *state_dir; > + struct commit *upstream; > + const char *upstream_name; > + const char *upstream_arg; > + char *head_name; > + struct object_id orig_head; > + struct commit *onto; > + const char *onto_name; > + const char *revisions; > + const char *switch_to; > + int root; > + struct object_id *squash_onto; > + struct commit *restrict_revision; > + int dont_finish_rebase; > + enum { > + REBASE_NO_QUIET = 1<<0, > + REBASE_VERBOSE = 1<<1, > + REBASE_DIFFSTAT = 1<<2, > + REBASE_FORCE = 1<<3, > + REBASE_INTERACTIVE_EXPLICIT = 1<<4, > + } flags; > + struct argv_array git_am_opts; > + const char *action; > + int signoff; > + int allow_rerere_autoupdate; > + int keep_empty; > + int autosquash; > + char *gpg_sign_opt; > + int autostash; > + char *cmd; > + int allow_empty_message; > + int rebase_merges, rebase_cousins; > + char *strategy, *strategy_opts; > + struct strbuf git_format_patch_opt; > + int reschedule_failed_exec; > +}; > + > +#define REBASE_OPTIONS_INIT { \ > + .type = REBASE_UNSPECIFIED, \ > + .flags = REBASE_NO_QUIET, \ > + .git_am_opts = ARGV_ARRAY_INIT, \ > + .git_format_patch_opt = STRBUF_INIT \ > + } > + > +static struct replay_opts get_replay_opts(const struct rebase_options *opts) > +{ > + struct replay_opts replay = REPLAY_OPTS_INIT; > + > + sequencer_init_config(&replay); > + > + replay.action = REPLAY_INTERACTIVE_REBASE; > + replay.signoff = opts->signoff; > + replay.allow_ff = !(opts->flags & REBASE_FORCE); > + if (opts->allow_rerere_autoupdate) > + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; > + replay.allow_empty = 1; > + replay.allow_empty_message = opts->allow_empty_message; > + replay.verbose = opts->flags & REBASE_VERBOSE; > + replay.reschedule_failed_exec = opts->reschedule_failed_exec; > + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); > + replay.strategy = opts->strategy; > + > + return replay; > +} > + I wonder if `struct rebase_options` and `struct replay_options` could be merged, or at least have `replay_options` used in `rebase_options`, instead of converting one to the other. I think it would make things simpler and cleaner, but I don’t know how hard it would be, or if my assumption is correct. Cheers, Alban ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args 2019-03-21 21:13 ` Alban Gruin @ 2019-04-10 19:16 ` Phillip Wood 0 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-10 19:16 UTC (permalink / raw) To: Alban Gruin, Phillip Wood, Git Mailing List; +Cc: Johannes Schindelin Hi Alban sorry for the slow reply, I think you're probably off-list for a while by now On 21/03/2019 21:13, Alban Gruin wrote: > Hi Phillip, > > It’s nice to see your work on this on the list. > > Le 19/03/2019 à 20:03, Phillip Wood a écrit : >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> In order to run `rebase -i` without forking `rebase--interactive` it >> will be convenient to use the same structure when parsing the options in >> cmd_rebase() and cmd_rebase__interactive(). >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> builtin/rebase.c | 203 ++++++++++++++++++++++++++--------------------- >> 1 file changed, 112 insertions(+), 91 deletions(-) >> >> diff --git a/builtin/rebase.c b/builtin/rebase.c >> index c93f2aa629..33a2495032 100644 >> --- a/builtin/rebase.c >> +++ b/builtin/rebase.c >> @@ -50,6 +50,73 @@ enum rebase_type { >> REBASE_PRESERVE_MERGES >> }; >> >> +struct rebase_options { >> + enum rebase_type type; >> + const char *state_dir; >> + struct commit *upstream; >> + const char *upstream_name; >> + const char *upstream_arg; >> + char *head_name; >> + struct object_id orig_head; >> + struct commit *onto; >> + const char *onto_name; >> + const char *revisions; >> + const char *switch_to; >> + int root; >> + struct object_id *squash_onto; >> + struct commit *restrict_revision; >> + int dont_finish_rebase; >> + enum { >> + REBASE_NO_QUIET = 1<<0, >> + REBASE_VERBOSE = 1<<1, >> + REBASE_DIFFSTAT = 1<<2, >> + REBASE_FORCE = 1<<3, >> + REBASE_INTERACTIVE_EXPLICIT = 1<<4, >> + } flags; >> + struct argv_array git_am_opts; >> + const char *action; >> + int signoff; >> + int allow_rerere_autoupdate; >> + int keep_empty; >> + int autosquash; >> + char *gpg_sign_opt; >> + int autostash; >> + char *cmd; >> + int allow_empty_message; >> + int rebase_merges, rebase_cousins; >> + char *strategy, *strategy_opts; >> + struct strbuf git_format_patch_opt; >> + int reschedule_failed_exec; >> +}; >> + >> +#define REBASE_OPTIONS_INIT { \ >> + .type = REBASE_UNSPECIFIED, \ >> + .flags = REBASE_NO_QUIET, \ >> + .git_am_opts = ARGV_ARRAY_INIT, \ >> + .git_format_patch_opt = STRBUF_INIT \ >> + } >> + >> +static struct replay_opts get_replay_opts(const struct rebase_options *opts) >> +{ >> + struct replay_opts replay = REPLAY_OPTS_INIT; >> + >> + sequencer_init_config(&replay); >> + >> + replay.action = REPLAY_INTERACTIVE_REBASE; >> + replay.signoff = opts->signoff; >> + replay.allow_ff = !(opts->flags & REBASE_FORCE); >> + if (opts->allow_rerere_autoupdate) >> + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; >> + replay.allow_empty = 1; >> + replay.allow_empty_message = opts->allow_empty_message; >> + replay.verbose = opts->flags & REBASE_VERBOSE; >> + replay.reschedule_failed_exec = opts->reschedule_failed_exec; >> + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); >> + replay.strategy = opts->strategy; >> + >> + return replay; >> +} >> + > > I wonder if `struct rebase_options` and `struct replay_options` could be > merged, or at least have `replay_options` used in `rebase_options`, > instead of converting one to the other. I think it would make things > simpler and cleaner, but I don’t know how hard it would be, or if my > assumption is correct. I did consider doing that, but there are a few subtle differences in the way the options are stored in each struct (eg the gpg option has a leading -S in struct rebase_options but not struct replay_options) and I think it would be a bit of a faff to align them as it would mean messing with the code uses them and the code that reads/writes the various state files (we cannot change the on-disk format without breaking things - sometimes users start a rebase with one version of git bundled with something like tig and then continue on the command-line with a different version). I'd prefer to leave it as a future cleanup once this series has been merged. Best Wishes Phillip > > Cheers, > Alban > ^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH 09/11] rebase -i: use struct rebase_options in do_interactive_rebase() 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (7 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 10/11] rebase: use a common action enum Phillip Wood ` (5 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> All the parameters that are passed to do_interactive_rebase() apart from `flags` are already in `struct rebase_options` so there is no need to pass them separately. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 69 +++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 33a2495032..21ccb6495e 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -113,6 +113,8 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) replay.reschedule_failed_exec = opts->reschedule_failed_exec; replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); replay.strategy = opts->strategy; + if (opts->strategy_opts) + parse_strategy_opts(&replay, opts->strategy_opts); return replay; } @@ -262,44 +264,50 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, return write_basic_state(opts, head_name, onto, orig_head); } -static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, - const char *switch_to, struct commit *upstream, - struct commit *onto, const char *onto_name, - struct object_id *squash_onto, const char *head_name, - struct commit *restrict_revision, char *raw_strategies, - struct string_list *commands, unsigned autosquash) +static void split_exec_commands(const char *cmd, struct string_list *commands) +{ + if (cmd && *cmd) { + string_list_split(commands, cmd, '\n', -1); + + /* rebase.c adds a new line to cmd after every command, + * so here the last command is always empty */ + string_list_remove_empty_items(commands, 0); + } +} + +static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) { int ret; const char *head_hash = NULL; char *revisions = NULL, *shortrevisions = NULL; struct argv_array make_script_args = ARGV_ARRAY_INIT; struct todo_list todo_list = TODO_LIST_INIT; + struct replay_opts replay = get_replay_opts(opts); + struct string_list commands = STRING_LIST_INIT_DUP; - if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) + if (prepare_branch_to_be_rebased(the_repository, &replay, + opts->switch_to)) return -1; - if (get_revision_ranges(upstream, onto, &head_hash, + if (get_revision_ranges(opts->upstream, opts->onto, &head_hash, &revisions, &shortrevisions)) return -1; - if (raw_strategies) - parse_strategy_opts(opts, raw_strategies); - - if (init_basic_state(opts, head_name, onto, head_hash)) { + if (init_basic_state(&replay, opts->head_name, opts->onto, head_hash)) { free(revisions); free(shortrevisions); return -1; } - if (!upstream && squash_onto) + if (!opts->upstream && opts->squash_onto) write_file(path_squash_onto(), "%s\n", - oid_to_hex(squash_onto)); + oid_to_hex(opts->squash_onto)); argv_array_pushl(&make_script_args, "", revisions, NULL); - if (restrict_revision) + if (opts->restrict_revision) argv_array_push(&make_script_args, - oid_to_hex(&restrict_revision->object.oid)); + oid_to_hex(&opts->restrict_revision->object.oid)); ret = sequencer_make_script(the_repository, &todo_list.buf, make_script_args.argc, make_script_args.argv, @@ -313,10 +321,13 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, &todo_list)) BUG("unusable todo list"); - ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name, - onto, head_hash, commands, autosquash, &todo_list); + split_exec_commands(opts->cmd, &commands); + ret = complete_action(the_repository, &replay, flags, + shortrevisions, opts->onto_name, opts->onto, head_hash, + &commands, opts->autosquash, &todo_list); } + string_list_clear(&commands, 0); free(revisions); free(shortrevisions); todo_list_release(&todo_list); @@ -336,7 +347,6 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) unsigned flags = 0; int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; - struct string_list commands = STRING_LIST_INIT_DUP; enum { NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC @@ -424,23 +434,12 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - if (opts.cmd && *opts.cmd) { - string_list_split(&commands, opts.cmd, '\n', -1); - - /* rebase.c adds a new line to cmd after every command, - * so here the last command is always empty */ - string_list_remove_empty_items(&commands, 0); - } - switch (command) { case NONE: { - struct replay_opts replay_opts = get_replay_opts(&opts); if (!opts.onto && !opts.upstream) die(_("a base commit must be provided with --upstream or --onto")); - ret = do_interactive_rebase(&replay_opts, flags, opts.switch_to, opts.upstream, opts.onto, - opts.onto_name, opts.squash_onto, opts.head_name, opts.restrict_revision, - opts.strategy_opts, &commands, opts.autosquash); + ret = do_interactive_rebase(&opts, flags); break; } case SKIP: { @@ -477,14 +476,18 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) case REARRANGE_SQUASH: ret = rearrange_squash_in_todo_file(); break; - case ADD_EXEC: + case ADD_EXEC: { + struct string_list commands = STRING_LIST_INIT_DUP; + + split_exec_commands(opts.cmd, &commands); ret = add_exec_commands(&commands); + string_list_clear(&commands, 0); break; + } default: BUG("invalid command '%d'", command); } - string_list_clear(&commands, 0); return !!ret; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 10/11] rebase: use a common action enum 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (8 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 09/11] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-19 20:24 ` Ævar Arnfjörð Bjarmason 2019-03-19 19:03 ` [RFC PATCH 11/11] rebase -i: run without forking rebase--interactive Phillip Wood ` (4 subsequent siblings) 14 siblings, 1 reply; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> cmd_rebase() and cmd_rebase__interactive() used different enums to hold the current action. Change to using a common enum so the values are the same when we change `rebase -i` to avoid forking `rebase--interactive`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- Notes: I'm not sure what to do with action_names, I moved it to keep it near the definition of the enum. The extra commands in the enum are not traced so they don't need to be in action_names but it looks odd to leave them out. Also I don't understand why action_names in NULL terminated or the names are marked for translation as it is used in trace2_cmd_mode(action_names[action]); so we don't use the NULL termination or translate the names (I'd be surprised if we did for tracing) builtin/rebase.c | 93 +++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 44 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 21ccb6495e..26d25a5242 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -119,6 +119,30 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) return replay; } +enum action { + ACTION_NONE = 0, + ACTION_CONTINUE, + ACTION_SKIP, + ACTION_ABORT, + ACTION_QUIT, + ACTION_EDIT_TODO, + ACTION_SHOW_CURRENT_PATCH, + ACTION_SHORTEN_OIDS, + ACTION_EXPAND_OIDS, + ACTION_CHECK_TODO_LIST, + ACTION_REARRANGE_SQUASH, + ACTION_ADD_EXEC +}; + +static const char *action_names[] = { N_("undefined"), + N_("continue"), + N_("skip"), + N_("abort"), + N_("quit"), + N_("edit_todo"), + N_("show_current_patch"), + NULL }; + static int add_exec_commands(struct string_list *commands) { const char *todo_file = rebase_path_todo(); @@ -347,10 +371,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) unsigned flags = 0; int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; - enum { - NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, - SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC - } command = 0; + enum action command = ACTION_NONE; struct option options[] = { OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), REBASE_FORCE), @@ -367,22 +388,22 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) N_("display a diffstat of what changed upstream"), REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), - CONTINUE), - OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), + ACTION_CONTINUE), + OPT_CMDMODE(0, "skip", &command, N_("skip commit"), ACTION_SKIP), OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), - EDIT_TODO), + ACTION_EDIT_TODO), OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), - SHOW_CURRENT_PATCH), + ACTION_SHOW_CURRENT_PATCH), OPT_CMDMODE(0, "shorten-ids", &command, - N_("shorten commit ids in the todo list"), SHORTEN_OIDS), + N_("shorten commit ids in the todo list"), ACTION_SHORTEN_OIDS), OPT_CMDMODE(0, "expand-ids", &command, - N_("expand commit ids in the todo list"), EXPAND_OIDS), + N_("expand commit ids in the todo list"), ACTION_EXPAND_OIDS), OPT_CMDMODE(0, "check-todo-list", &command, - N_("check the todo list"), CHECK_TODO_LIST), + N_("check the todo list"), ACTION_CHECK_TODO_LIST), OPT_CMDMODE(0, "rearrange-squash", &command, - N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), + N_("rearrange fixup/squash lines"), ACTION_REARRANGE_SQUASH), OPT_CMDMODE(0, "add-exec-commands", &command, - N_("insert exec commands in todo list"), ADD_EXEC), + N_("insert exec commands in todo list"), ACTION_ADD_EXEC), { OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, { OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision, @@ -428,36 +449,36 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; - flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; + flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; if (opts.rebase_cousins >= 0 && !opts.rebase_merges) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); switch (command) { - case NONE: { + case ACTION_NONE: { if (!opts.onto && !opts.upstream) die(_("a base commit must be provided with --upstream or --onto")); ret = do_interactive_rebase(&opts, flags); break; } - case SKIP: { + case ACTION_SKIP: { struct string_list merge_rr = STRING_LIST_INIT_DUP; rerere_clear(the_repository, &merge_rr); } /* fallthrough */ - case CONTINUE: { + case ACTION_CONTINUE: { struct replay_opts replay_opts = get_replay_opts(&opts); ret = sequencer_continue(the_repository, &replay_opts); break; } - case EDIT_TODO: + case ACTION_EDIT_TODO: ret = edit_todo_file(flags); break; - case SHOW_CURRENT_PATCH: { + case ACTION_SHOW_CURRENT_PATCH: { struct child_process cmd = CHILD_PROCESS_INIT; cmd.git_cmd = 1; @@ -466,17 +487,17 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) break; } - case SHORTEN_OIDS: - case EXPAND_OIDS: + case ACTION_SHORTEN_OIDS: + case ACTION_EXPAND_OIDS: ret = transform_todo_file(flags); break; - case CHECK_TODO_LIST: + case ACTION_CHECK_TODO_LIST: ret = check_todo_list_from_file(the_repository); break; - case REARRANGE_SQUASH: + case ACTION_REARRANGE_SQUASH: ret = rearrange_squash_in_todo_file(); break; - case ADD_EXEC: { + case ACTION_ADD_EXEC: { struct string_list commands = STRING_LIST_INIT_DUP; split_exec_commands(opts.cmd, &commands); @@ -1417,23 +1438,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) struct strbuf revisions = STRBUF_INIT; struct strbuf buf = STRBUF_INIT; struct object_id merge_base; - enum { - NO_ACTION, - ACTION_CONTINUE, - ACTION_SKIP, - ACTION_ABORT, - ACTION_QUIT, - ACTION_EDIT_TODO, - ACTION_SHOW_CURRENT_PATCH, - } action = NO_ACTION; - static const char *action_names[] = { N_("undefined"), - N_("continue"), - N_("skip"), - N_("abort"), - N_("quit"), - N_("edit_todo"), - N_("show_current_patch"), - NULL }; + enum action action = ACTION_NONE; const char *gpg_sign = NULL; struct string_list exec = STRING_LIST_INIT_NODUP; const char *rebase_merges = NULL; @@ -1600,7 +1605,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) builtin_rebase_options, builtin_rebase_usage, 0); - if (action != NO_ACTION && total_argc != 2) { + if (action != ACTION_NONE && total_argc != 2) { usage_with_options(builtin_rebase_usage, builtin_rebase_options); } @@ -1609,7 +1614,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) usage_with_options(builtin_rebase_usage, builtin_rebase_options); - if (action != NO_ACTION && !in_progress) + if (action != ACTION_NONE && !in_progress) die(_("No rebase in progress?")); setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0); @@ -1709,7 +1714,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.action = "show-current-patch"; options.dont_finish_rebase = 1; goto run_rebase; - case NO_ACTION: + case ACTION_NONE: break; default: BUG("action: %d", action); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 10/11] rebase: use a common action enum 2019-03-19 19:03 ` [RFC PATCH 10/11] rebase: use a common action enum Phillip Wood @ 2019-03-19 20:24 ` Ævar Arnfjörð Bjarmason 2019-03-21 14:43 ` Phillip Wood 0 siblings, 1 reply; 40+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-03-19 20:24 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin On Tue, Mar 19 2019, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > cmd_rebase() and cmd_rebase__interactive() used different enums to hold > the current action. Change to using a common enum so the values are the > same when we change `rebase -i` to avoid forking `rebase--interactive`. > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > > Notes: > I'm not sure what to do with action_names, I moved it to keep it near > the definition of the enum. The extra commands in the enum are not > traced so they don't need to be in action_names but it looks odd to > leave them out. Also I don't understand why action_names in NULL > terminated or the names are marked for translation as it is used in > trace2_cmd_mode(action_names[action]); > so we don't use the NULL termination or translate the names (I'd be > surprised if we did for tracing) Making them for translation is a bug in the recent b3a5d5a80c ("trace2:data: add subverb for rebase", 2019-02-22) and just looks like copy/paste cargo-culting from somewhere else, and likewise with the trailing NULL which doesn't make sense in this case. > builtin/rebase.c | 93 +++++++++++++++++++++++++----------------------- > 1 file changed, 49 insertions(+), 44 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 21ccb6495e..26d25a5242 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -119,6 +119,30 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) > return replay; > } > > +enum action { > + ACTION_NONE = 0, > + ACTION_CONTINUE, > + ACTION_SKIP, > + ACTION_ABORT, > + ACTION_QUIT, > + ACTION_EDIT_TODO, > + ACTION_SHOW_CURRENT_PATCH, > + ACTION_SHORTEN_OIDS, > + ACTION_EXPAND_OIDS, > + ACTION_CHECK_TODO_LIST, > + ACTION_REARRANGE_SQUASH, > + ACTION_ADD_EXEC > +}; > + > +static const char *action_names[] = { N_("undefined"), > + N_("continue"), > + N_("skip"), > + N_("abort"), > + N_("quit"), > + N_("edit_todo"), > + N_("show_current_patch"), > + NULL }; > + > static int add_exec_commands(struct string_list *commands) > { > const char *todo_file = rebase_path_todo(); > @@ -347,10 +371,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) > unsigned flags = 0; > int abbreviate_commands = 0, ret = 0; > struct object_id squash_onto = null_oid; > - enum { > - NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, > - SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC > - } command = 0; > + enum action command = ACTION_NONE; > struct option options[] = { > OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), > REBASE_FORCE), > @@ -367,22 +388,22 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) > N_("display a diffstat of what changed upstream"), > REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), > OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), > - CONTINUE), > - OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), > + ACTION_CONTINUE), > + OPT_CMDMODE(0, "skip", &command, N_("skip commit"), ACTION_SKIP), > OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), > - EDIT_TODO), > + ACTION_EDIT_TODO), > OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), > - SHOW_CURRENT_PATCH), > + ACTION_SHOW_CURRENT_PATCH), > OPT_CMDMODE(0, "shorten-ids", &command, > - N_("shorten commit ids in the todo list"), SHORTEN_OIDS), > + N_("shorten commit ids in the todo list"), ACTION_SHORTEN_OIDS), > OPT_CMDMODE(0, "expand-ids", &command, > - N_("expand commit ids in the todo list"), EXPAND_OIDS), > + N_("expand commit ids in the todo list"), ACTION_EXPAND_OIDS), > OPT_CMDMODE(0, "check-todo-list", &command, > - N_("check the todo list"), CHECK_TODO_LIST), > + N_("check the todo list"), ACTION_CHECK_TODO_LIST), > OPT_CMDMODE(0, "rearrange-squash", &command, > - N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), > + N_("rearrange fixup/squash lines"), ACTION_REARRANGE_SQUASH), > OPT_CMDMODE(0, "add-exec-commands", &command, > - N_("insert exec commands in todo list"), ADD_EXEC), > + N_("insert exec commands in todo list"), ACTION_ADD_EXEC), > { OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"), > PARSE_OPT_NONEG, parse_opt_commit, 0 }, > { OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision, > @@ -428,36 +449,36 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) > flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; > flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; > flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; > - flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; > + flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; > > if (opts.rebase_cousins >= 0 && !opts.rebase_merges) > warning(_("--[no-]rebase-cousins has no effect without " > "--rebase-merges")); > > switch (command) { > - case NONE: { > + case ACTION_NONE: { > if (!opts.onto && !opts.upstream) > die(_("a base commit must be provided with --upstream or --onto")); > > ret = do_interactive_rebase(&opts, flags); > break; > } > - case SKIP: { > + case ACTION_SKIP: { > struct string_list merge_rr = STRING_LIST_INIT_DUP; > > rerere_clear(the_repository, &merge_rr); > } > /* fallthrough */ > - case CONTINUE: { > + case ACTION_CONTINUE: { > struct replay_opts replay_opts = get_replay_opts(&opts); > > ret = sequencer_continue(the_repository, &replay_opts); > break; > } > - case EDIT_TODO: > + case ACTION_EDIT_TODO: > ret = edit_todo_file(flags); > break; > - case SHOW_CURRENT_PATCH: { > + case ACTION_SHOW_CURRENT_PATCH: { > struct child_process cmd = CHILD_PROCESS_INIT; > > cmd.git_cmd = 1; > @@ -466,17 +487,17 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) > > break; > } > - case SHORTEN_OIDS: > - case EXPAND_OIDS: > + case ACTION_SHORTEN_OIDS: > + case ACTION_EXPAND_OIDS: > ret = transform_todo_file(flags); > break; > - case CHECK_TODO_LIST: > + case ACTION_CHECK_TODO_LIST: > ret = check_todo_list_from_file(the_repository); > break; > - case REARRANGE_SQUASH: > + case ACTION_REARRANGE_SQUASH: > ret = rearrange_squash_in_todo_file(); > break; > - case ADD_EXEC: { > + case ACTION_ADD_EXEC: { > struct string_list commands = STRING_LIST_INIT_DUP; > > split_exec_commands(opts.cmd, &commands); > @@ -1417,23 +1438,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > struct strbuf revisions = STRBUF_INIT; > struct strbuf buf = STRBUF_INIT; > struct object_id merge_base; > - enum { > - NO_ACTION, > - ACTION_CONTINUE, > - ACTION_SKIP, > - ACTION_ABORT, > - ACTION_QUIT, > - ACTION_EDIT_TODO, > - ACTION_SHOW_CURRENT_PATCH, > - } action = NO_ACTION; > - static const char *action_names[] = { N_("undefined"), > - N_("continue"), > - N_("skip"), > - N_("abort"), > - N_("quit"), > - N_("edit_todo"), > - N_("show_current_patch"), > - NULL }; > + enum action action = ACTION_NONE; > const char *gpg_sign = NULL; > struct string_list exec = STRING_LIST_INIT_NODUP; > const char *rebase_merges = NULL; > @@ -1600,7 +1605,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > builtin_rebase_options, > builtin_rebase_usage, 0); > > - if (action != NO_ACTION && total_argc != 2) { > + if (action != ACTION_NONE && total_argc != 2) { > usage_with_options(builtin_rebase_usage, > builtin_rebase_options); > } > @@ -1609,7 +1614,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > usage_with_options(builtin_rebase_usage, > builtin_rebase_options); > > - if (action != NO_ACTION && !in_progress) > + if (action != ACTION_NONE && !in_progress) > die(_("No rebase in progress?")); > setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0); > > @@ -1709,7 +1714,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > options.action = "show-current-patch"; > options.dont_finish_rebase = 1; > goto run_rebase; > - case NO_ACTION: > + case ACTION_NONE: > break; > default: > BUG("action: %d", action); ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 10/11] rebase: use a common action enum 2019-03-19 20:24 ` Ævar Arnfjörð Bjarmason @ 2019-03-21 14:43 ` Phillip Wood 0 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-21 14:43 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Phillip Wood Cc: Git Mailing List, Johannes Schindelin On 19/03/2019 20:24, Ævar Arnfjörð Bjarmason wrote: > > On Tue, Mar 19 2019, Phillip Wood wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> cmd_rebase() and cmd_rebase__interactive() used different enums to hold >> the current action. Change to using a common enum so the values are the >> same when we change `rebase -i` to avoid forking `rebase--interactive`. >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> >> Notes: >> I'm not sure what to do with action_names, I moved it to keep it near >> the definition of the enum. The extra commands in the enum are not >> traced so they don't need to be in action_names but it looks odd to >> leave them out. Also I don't understand why action_names in NULL >> terminated or the names are marked for translation as it is used in >> trace2_cmd_mode(action_names[action]); >> so we don't use the NULL termination or translate the names (I'd be >> surprised if we did for tracing) > > Making them for translation is a bug in the recent b3a5d5a80c > ("trace2:data: add subverb for rebase", 2019-02-22) and just looks like > copy/paste cargo-culting from somewhere else, and likewise with the > trailing NULL which doesn't make sense in this case. Thanks for clarifying that, I'll add a preparatory patch at the beginning of the series to fix those issues Best Wishes Phillip >> builtin/rebase.c | 93 +++++++++++++++++++++++++----------------------- >> 1 file changed, 49 insertions(+), 44 deletions(-) >> >> diff --git a/builtin/rebase.c b/builtin/rebase.c >> index 21ccb6495e..26d25a5242 100644 >> --- a/builtin/rebase.c >> +++ b/builtin/rebase.c >> @@ -119,6 +119,30 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) >> return replay; >> } >> >> +enum action { >> + ACTION_NONE = 0, >> + ACTION_CONTINUE, >> + ACTION_SKIP, >> + ACTION_ABORT, >> + ACTION_QUIT, >> + ACTION_EDIT_TODO, >> + ACTION_SHOW_CURRENT_PATCH, >> + ACTION_SHORTEN_OIDS, >> + ACTION_EXPAND_OIDS, >> + ACTION_CHECK_TODO_LIST, >> + ACTION_REARRANGE_SQUASH, >> + ACTION_ADD_EXEC >> +}; >> + >> +static const char *action_names[] = { N_("undefined"), >> + N_("continue"), >> + N_("skip"), >> + N_("abort"), >> + N_("quit"), >> + N_("edit_todo"), >> + N_("show_current_patch"), >> + NULL }; >> + >> static int add_exec_commands(struct string_list *commands) >> { >> const char *todo_file = rebase_path_todo(); >> @@ -347,10 +371,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) >> unsigned flags = 0; >> int abbreviate_commands = 0, ret = 0; >> struct object_id squash_onto = null_oid; >> - enum { >> - NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, >> - SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC >> - } command = 0; >> + enum action command = ACTION_NONE; >> struct option options[] = { >> OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), >> REBASE_FORCE), >> @@ -367,22 +388,22 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) >> N_("display a diffstat of what changed upstream"), >> REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), >> OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), >> - CONTINUE), >> - OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), >> + ACTION_CONTINUE), >> + OPT_CMDMODE(0, "skip", &command, N_("skip commit"), ACTION_SKIP), >> OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), >> - EDIT_TODO), >> + ACTION_EDIT_TODO), >> OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), >> - SHOW_CURRENT_PATCH), >> + ACTION_SHOW_CURRENT_PATCH), >> OPT_CMDMODE(0, "shorten-ids", &command, >> - N_("shorten commit ids in the todo list"), SHORTEN_OIDS), >> + N_("shorten commit ids in the todo list"), ACTION_SHORTEN_OIDS), >> OPT_CMDMODE(0, "expand-ids", &command, >> - N_("expand commit ids in the todo list"), EXPAND_OIDS), >> + N_("expand commit ids in the todo list"), ACTION_EXPAND_OIDS), >> OPT_CMDMODE(0, "check-todo-list", &command, >> - N_("check the todo list"), CHECK_TODO_LIST), >> + N_("check the todo list"), ACTION_CHECK_TODO_LIST), >> OPT_CMDMODE(0, "rearrange-squash", &command, >> - N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), >> + N_("rearrange fixup/squash lines"), ACTION_REARRANGE_SQUASH), >> OPT_CMDMODE(0, "add-exec-commands", &command, >> - N_("insert exec commands in todo list"), ADD_EXEC), >> + N_("insert exec commands in todo list"), ACTION_ADD_EXEC), >> { OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"), >> PARSE_OPT_NONEG, parse_opt_commit, 0 }, >> { OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision, >> @@ -428,36 +449,36 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) >> flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; >> flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; >> flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; >> - flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; >> + flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; >> >> if (opts.rebase_cousins >= 0 && !opts.rebase_merges) >> warning(_("--[no-]rebase-cousins has no effect without " >> "--rebase-merges")); >> >> switch (command) { >> - case NONE: { >> + case ACTION_NONE: { >> if (!opts.onto && !opts.upstream) >> die(_("a base commit must be provided with --upstream or --onto")); >> >> ret = do_interactive_rebase(&opts, flags); >> break; >> } >> - case SKIP: { >> + case ACTION_SKIP: { >> struct string_list merge_rr = STRING_LIST_INIT_DUP; >> >> rerere_clear(the_repository, &merge_rr); >> } >> /* fallthrough */ >> - case CONTINUE: { >> + case ACTION_CONTINUE: { >> struct replay_opts replay_opts = get_replay_opts(&opts); >> >> ret = sequencer_continue(the_repository, &replay_opts); >> break; >> } >> - case EDIT_TODO: >> + case ACTION_EDIT_TODO: >> ret = edit_todo_file(flags); >> break; >> - case SHOW_CURRENT_PATCH: { >> + case ACTION_SHOW_CURRENT_PATCH: { >> struct child_process cmd = CHILD_PROCESS_INIT; >> >> cmd.git_cmd = 1; >> @@ -466,17 +487,17 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) >> >> break; >> } >> - case SHORTEN_OIDS: >> - case EXPAND_OIDS: >> + case ACTION_SHORTEN_OIDS: >> + case ACTION_EXPAND_OIDS: >> ret = transform_todo_file(flags); >> break; >> - case CHECK_TODO_LIST: >> + case ACTION_CHECK_TODO_LIST: >> ret = check_todo_list_from_file(the_repository); >> break; >> - case REARRANGE_SQUASH: >> + case ACTION_REARRANGE_SQUASH: >> ret = rearrange_squash_in_todo_file(); >> break; >> - case ADD_EXEC: { >> + case ACTION_ADD_EXEC: { >> struct string_list commands = STRING_LIST_INIT_DUP; >> >> split_exec_commands(opts.cmd, &commands); >> @@ -1417,23 +1438,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) >> struct strbuf revisions = STRBUF_INIT; >> struct strbuf buf = STRBUF_INIT; >> struct object_id merge_base; >> - enum { >> - NO_ACTION, >> - ACTION_CONTINUE, >> - ACTION_SKIP, >> - ACTION_ABORT, >> - ACTION_QUIT, >> - ACTION_EDIT_TODO, >> - ACTION_SHOW_CURRENT_PATCH, >> - } action = NO_ACTION; >> - static const char *action_names[] = { N_("undefined"), >> - N_("continue"), >> - N_("skip"), >> - N_("abort"), >> - N_("quit"), >> - N_("edit_todo"), >> - N_("show_current_patch"), >> - NULL }; >> + enum action action = ACTION_NONE; >> const char *gpg_sign = NULL; >> struct string_list exec = STRING_LIST_INIT_NODUP; >> const char *rebase_merges = NULL; >> @@ -1600,7 +1605,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) >> builtin_rebase_options, >> builtin_rebase_usage, 0); >> >> - if (action != NO_ACTION && total_argc != 2) { >> + if (action != ACTION_NONE && total_argc != 2) { >> usage_with_options(builtin_rebase_usage, >> builtin_rebase_options); >> } >> @@ -1609,7 +1614,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) >> usage_with_options(builtin_rebase_usage, >> builtin_rebase_options); >> >> - if (action != NO_ACTION && !in_progress) >> + if (action != ACTION_NONE && !in_progress) >> die(_("No rebase in progress?")); >> setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0); >> >> @@ -1709,7 +1714,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) >> options.action = "show-current-patch"; >> options.dont_finish_rebase = 1; >> goto run_rebase; >> - case NO_ACTION: >> + case ACTION_NONE: >> break; >> default: >> BUG("action: %d", action); ^ permalink raw reply [flat|nested] 40+ messages in thread
* [RFC PATCH 11/11] rebase -i: run without forking rebase--interactive 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (9 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 10/11] rebase: use a common action enum Phillip Wood @ 2019-03-19 19:03 ` Phillip Wood 2019-03-20 20:50 ` [RFC PATCH 00/11] rebase -i " Josh Steadmon ` (3 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-19 19:03 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> When the builtin rebase starts an interactive rebase it parses the options and then repackages them and forks `rebase--interactive`. Separate the option parsing in cmd_rebase__interactive() from the business logic to allow interactive rebases can be run without forking `rebase__interactive` by calling run_rebase_interactive() directly. This makes it easy to debug the sequencer without worrying about attaching to child processes. It also makes it easy to remove cmd_rebase__interactive() in the future when git-legacy-rebase.sh and git-rebase--preserve-merges.sh are retired. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 221 ++++++++++++++++++----------------------------- 1 file changed, 86 insertions(+), 135 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 26d25a5242..2e26d350f0 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -317,7 +317,9 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) &revisions, &shortrevisions)) return -1; - if (init_basic_state(&replay, opts->head_name, opts->onto, head_hash)) { + if (init_basic_state(&replay, + opts->head_name ? opts->head_name : "detached HEAD", + opts->onto, head_hash)) { free(revisions); free(shortrevisions); @@ -360,6 +362,77 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) return ret; } +static int run_rebase_interactive(struct rebase_options *opts, + enum action command) +{ + unsigned flags = 0; + int abbreviate_commands = 0, ret = 0; + + git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); + + flags |= opts->keep_empty ? TODO_LIST_KEEP_EMPTY : 0; + flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; + flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0; + flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; + + switch (command) { + case ACTION_NONE: { + if (!opts->onto && !opts->upstream) + die(_("a base commit must be provided with --upstream or --onto")); + + ret = do_interactive_rebase(opts, flags); + break; + } + case ACTION_SKIP: { + struct string_list merge_rr = STRING_LIST_INIT_DUP; + + rerere_clear(the_repository, &merge_rr); + } + /* fallthrough */ + case ACTION_CONTINUE: { + struct replay_opts replay_opts = get_replay_opts(opts); + + ret = sequencer_continue(the_repository, &replay_opts); + break; + } + case ACTION_EDIT_TODO: + ret = edit_todo_file(flags); + break; + case ACTION_SHOW_CURRENT_PATCH: { + struct child_process cmd = CHILD_PROCESS_INIT; + + cmd.git_cmd = 1; + argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); + ret = run_command(&cmd); + + break; + } + case ACTION_SHORTEN_OIDS: + case ACTION_EXPAND_OIDS: + ret = transform_todo_file(flags); + break; + case ACTION_CHECK_TODO_LIST: + ret = check_todo_list_from_file(the_repository); + break; + case ACTION_REARRANGE_SQUASH: + ret = rearrange_squash_in_todo_file(); + break; + case ACTION_ADD_EXEC: { + struct string_list commands = STRING_LIST_INIT_DUP; + + split_exec_commands(opts->cmd, &commands); + ret = add_exec_commands(&commands); + string_list_clear(&commands, 0); + break; + } + default: + BUG("invalid command '%d'", command); + } + + return ret; +} + static const char * const builtin_rebase_interactive_usage[] = { N_("git rebase--interactive [<options>]"), NULL @@ -368,8 +441,6 @@ static const char * const builtin_rebase_interactive_usage[] = { int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) { struct rebase_options opts = REBASE_OPTIONS_INIT; - unsigned flags = 0; - int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; enum action command = ACTION_NONE; struct option options[] = { @@ -434,8 +505,6 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) opts.rebase_cousins = -1; - git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); - if (argc == 1) usage_with_options(builtin_rebase_interactive_usage, options); @@ -445,71 +514,11 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) if (!is_null_oid(&squash_onto)) opts.squash_onto = &squash_onto; - flags |= opts.keep_empty ? TODO_LIST_KEEP_EMPTY : 0; - flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; - flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; - flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; - flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; - if (opts.rebase_cousins >= 0 && !opts.rebase_merges) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - switch (command) { - case ACTION_NONE: { - if (!opts.onto && !opts.upstream) - die(_("a base commit must be provided with --upstream or --onto")); - - ret = do_interactive_rebase(&opts, flags); - break; - } - case ACTION_SKIP: { - struct string_list merge_rr = STRING_LIST_INIT_DUP; - - rerere_clear(the_repository, &merge_rr); - } - /* fallthrough */ - case ACTION_CONTINUE: { - struct replay_opts replay_opts = get_replay_opts(&opts); - - ret = sequencer_continue(the_repository, &replay_opts); - break; - } - case ACTION_EDIT_TODO: - ret = edit_todo_file(flags); - break; - case ACTION_SHOW_CURRENT_PATCH: { - struct child_process cmd = CHILD_PROCESS_INIT; - - cmd.git_cmd = 1; - argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); - ret = run_command(&cmd); - - break; - } - case ACTION_SHORTEN_OIDS: - case ACTION_EXPAND_OIDS: - ret = transform_todo_file(flags); - break; - case ACTION_CHECK_TODO_LIST: - ret = check_todo_list_from_file(the_repository); - break; - case ACTION_REARRANGE_SQUASH: - ret = rearrange_squash_in_todo_file(); - break; - case ACTION_ADD_EXEC: { - struct string_list commands = STRING_LIST_INIT_DUP; - - split_exec_commands(opts.cmd, &commands); - ret = add_exec_commands(&commands); - string_list_clear(&commands, 0); - break; - } - default: - BUG("invalid command '%d'", command); - } - - return !!ret; + return !!run_rebase_interactive(&opts, command); } static int use_builtin_rebase(void) @@ -1072,7 +1081,7 @@ static int run_am(struct rebase_options *opts) return status; } -static int run_specific_rebase(struct rebase_options *opts) +static int run_specific_rebase(struct rebase_options *opts, enum action action) { const char *argv[] = { NULL, NULL }; struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT; @@ -1081,77 +1090,19 @@ static int run_specific_rebase(struct rebase_options *opts) if (opts->type == REBASE_INTERACTIVE) { /* Run builtin interactive rebase */ - struct child_process child = CHILD_PROCESS_INIT; - - argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s", - resolvemsg); + setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1); if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) { - argv_array_push(&child.env_array, - "GIT_SEQUENCE_EDITOR=:"); + setenv("GIT_SEQUENCE_EDITOR", ":", 1); opts->autosquash = 0; } + if (opts->gpg_sign_opt) { + /* remove the leading "-S" */ + char *tmp = xstrdup(opts->gpg_sign_opt + 2); + free(opts->gpg_sign_opt); + opts->gpg_sign_opt = tmp; + } - child.git_cmd = 1; - argv_array_push(&child.args, "rebase--interactive"); - - if (opts->action) - argv_array_pushf(&child.args, "--%s", opts->action); - if (opts->keep_empty) - argv_array_push(&child.args, "--keep-empty"); - if (opts->rebase_merges) - argv_array_push(&child.args, "--rebase-merges"); - if (opts->rebase_cousins) - argv_array_push(&child.args, "--rebase-cousins"); - if (opts->autosquash) - argv_array_push(&child.args, "--autosquash"); - if (opts->flags & REBASE_VERBOSE) - argv_array_push(&child.args, "--verbose"); - if (opts->flags & REBASE_FORCE) - argv_array_push(&child.args, "--no-ff"); - if (opts->restrict_revision) - argv_array_pushf(&child.args, - "--restrict-revision=^%s", - oid_to_hex(&opts->restrict_revision->object.oid)); - if (opts->upstream) - argv_array_pushf(&child.args, "--upstream=%s", - oid_to_hex(&opts->upstream->object.oid)); - if (opts->onto) - argv_array_pushf(&child.args, "--onto=%s", - oid_to_hex(&opts->onto->object.oid)); - if (opts->squash_onto) - argv_array_pushf(&child.args, "--squash-onto=%s", - oid_to_hex(opts->squash_onto)); - if (opts->onto_name) - argv_array_pushf(&child.args, "--onto-name=%s", - opts->onto_name); - argv_array_pushf(&child.args, "--head-name=%s", - opts->head_name ? - opts->head_name : "detached HEAD"); - if (opts->strategy) - argv_array_pushf(&child.args, "--strategy=%s", - opts->strategy); - if (opts->strategy_opts) - argv_array_pushf(&child.args, "--strategy-opts=%s", - opts->strategy_opts); - if (opts->switch_to) - argv_array_pushf(&child.args, "--switch-to=%s", - opts->switch_to); - if (opts->cmd) - argv_array_pushf(&child.args, "--cmd=%s", opts->cmd); - if (opts->allow_empty_message) - argv_array_push(&child.args, "--allow-empty-message"); - if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) - argv_array_push(&child.args, "--rerere-autoupdate"); - else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE) - argv_array_push(&child.args, "--no-rerere-autoupdate"); - if (opts->gpg_sign_opt) - argv_array_push(&child.args, opts->gpg_sign_opt); - if (opts->signoff) - argv_array_push(&child.args, "--signoff"); - if (opts->reschedule_failed_exec) - argv_array_push(&child.args, "--reschedule-failed-exec"); - - status = run_command(&child); + status = run_rebase_interactive(opts, action); goto finished_rebase; } @@ -2212,7 +2163,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.revisions = revisions.buf; run_rebase: - ret = !!run_specific_rebase(&options); + ret = !!run_specific_rebase(&options, action); cleanup: strbuf_release(&revisions); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 00/11] rebase -i run without forking rebase--interactive 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (10 preceding siblings ...) 2019-03-19 19:03 ` [RFC PATCH 11/11] rebase -i: run without forking rebase--interactive Phillip Wood @ 2019-03-20 20:50 ` Josh Steadmon 2019-03-20 23:05 ` Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 14 siblings, 0 replies; 40+ messages in thread From: Josh Steadmon @ 2019-03-20 20:50 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin On 2019.03.19 19:03, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the builtin rebase starts an interactive rebase it parses the > options and then repackages them and forks `rebase--interactive`. This > series refactors rebase--interactive so that interactive rebases can > be started by the builtin rebase without forking. My motivation was to > make it easier to debug the sequencer but this should help future > maintainability. > > This series involves some code movement so viewing the diffs with > --color-moved is recommended. > > These patches are based on a merge of master [e902e9bcae ("The second > batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 > ("rebase--interactive: move transform_todo_file()", 2019-03-05). They > can be fetched from the tag rebase-i-no-fork/rfc at > https://github.com/phillipwood/git.git > > Phillip Wood (11): > sequencer: always discard index after checkout > rebase: rename write_basic_state() > rebase: use OPT_RERERE_AUTOUPDATE() > rebase -i: combine rebase--interactive.c with rebase.c > rebase -i: remove duplication > rebase -i: use struct commit when parsing options > rebase -i: use struct object_id for squash_onto > rebase -i: use struct rebase_options to parse args > rebase -i: use struct rebase_options in do_interactive_rebase() > rebase: use a common action enum > rebase -i: run without forking rebase--interactive Although I'm not very familiar with the rebase code, this series looks good to me. Reviewed-by: Josh Steadmon <steadmon@google.com> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 00/11] rebase -i run without forking rebase--interactive 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (11 preceding siblings ...) 2019-03-20 20:50 ` [RFC PATCH 00/11] rebase -i " Josh Steadmon @ 2019-03-20 23:05 ` Ævar Arnfjörð Bjarmason 2019-03-21 14:40 ` Phillip Wood 2019-03-21 1:44 ` Junio C Hamano 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood 14 siblings, 1 reply; 40+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-03-20 23:05 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin On Tue, Mar 19 2019, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the builtin rebase starts an interactive rebase it parses the > options and then repackages them and forks `rebase--interactive`. This > series refactors rebase--interactive so that interactive rebases can > be started by the builtin rebase without forking. My motivation was to > make it easier to debug the sequencer but this should help future > maintainability. > > This series involves some code movement so viewing the diffs with > --color-moved is recommended. > > These patches are based on a merge of master [e902e9bcae ("The second > batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 > ("rebase--interactive: move transform_todo_file()", 2019-03-05). They > can be fetched from the tag rebase-i-no-fork/rfc at > https://github.com/phillipwood/git.git Just a that the t/perf/*rebase* numbers look much better with this. I don't have these in front of me anymore, but over 10 runs with -O3 one of those long-runnings test was 30% faster. Another one (rebase -i) went from 0.02 to 0.01 sec, with that short amount of time I wonder (but didn't dig) if the test itself is broken... ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 00/11] rebase -i run without forking rebase--interactive 2019-03-20 23:05 ` Ævar Arnfjörð Bjarmason @ 2019-03-21 14:40 ` Phillip Wood 0 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-03-21 14:40 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Phillip Wood Cc: Git Mailing List, Johannes Schindelin On 20/03/2019 23:05, Ævar Arnfjörð Bjarmason wrote: > > On Tue, Mar 19 2019, Phillip Wood wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> When the builtin rebase starts an interactive rebase it parses the >> options and then repackages them and forks `rebase--interactive`. This >> series refactors rebase--interactive so that interactive rebases can >> be started by the builtin rebase without forking. My motivation was to >> make it easier to debug the sequencer but this should help future >> maintainability. >> >> This series involves some code movement so viewing the diffs with >> --color-moved is recommended. >> >> These patches are based on a merge of master [e902e9bcae ("The second >> batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 >> ("rebase--interactive: move transform_todo_file()", 2019-03-05). They >> can be fetched from the tag rebase-i-no-fork/rfc at >> https://github.com/phillipwood/git.git > > Just a that the t/perf/*rebase* numbers look much better with this. I > don't have these in front of me anymore, but over 10 runs with -O3 one > of those long-runnings test was 30% faster. That's surprising I wouldn't expect that much change as the time taken to fork shouldn't be that significant compared to the overall running time. I'll make a note to have a look at those tests. > Another one (rebase -i) went from 0.02 to 0.01 sec, with that short > amount of time I wonder (but didn't dig) if the test itself is broken... Yeah, that sounds suspicious Thanks for sharing those results Phillip ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 00/11] rebase -i run without forking rebase--interactive 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (12 preceding siblings ...) 2019-03-20 23:05 ` Ævar Arnfjörð Bjarmason @ 2019-03-21 1:44 ` Junio C Hamano 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood 14 siblings, 0 replies; 40+ messages in thread From: Junio C Hamano @ 2019-03-21 1:44 UTC (permalink / raw) To: Phillip Wood; +Cc: Git Mailing List, Johannes Schindelin, Phillip Wood Phillip Wood <phillip.wood123@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the builtin rebase starts an interactive rebase it parses the > options and then repackages them and forks `rebase--interactive`. This > series refactors rebase--interactive so that interactive rebases can > be started by the builtin rebase without forking. My motivation was to > make it easier to debug the sequencer but this should help future > maintainability. > > This series involves some code movement so viewing the diffs with > --color-moved is recommended. > > These patches are based on a merge of master [e902e9bcae ("The second > batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 > ("rebase--interactive: move transform_todo_file()", 2019-03-05). They > can be fetched from the tag rebase-i-no-fork/rfc at > https://github.com/phillipwood/git.git Exciting. > > Phillip Wood (11): > sequencer: always discard index after checkout > rebase: rename write_basic_state() > rebase: use OPT_RERERE_AUTOUPDATE() > rebase -i: combine rebase--interactive.c with rebase.c > rebase -i: remove duplication > rebase -i: use struct commit when parsing options > rebase -i: use struct object_id for squash_onto > rebase -i: use struct rebase_options to parse args > rebase -i: use struct rebase_options in do_interactive_rebase() > rebase: use a common action enum > rebase -i: run without forking rebase--interactive > > Makefile | 1 - > builtin/rebase--interactive.c | 377 -------------------- > builtin/rebase.c | 625 ++++++++++++++++++++++++++-------- > parse-options-cb.c | 34 ++ > parse-options.h | 4 + > sequencer.c | 42 ++- > sequencer.h | 7 +- > 7 files changed, 556 insertions(+), 534 deletions(-) > delete mode 100644 builtin/rebase--interactive.c ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v1 00/12] Run rebase -i without forking rebase--interactive 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood ` (13 preceding siblings ...) 2019-03-21 1:44 ` Junio C Hamano @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 01/12] sequencer: always discard index after checkout Phillip Wood ` (11 more replies) 14 siblings, 12 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Thanks for the feedback on the last version. I've updated the final commit message to mention the speed up reported by Ævar. I've also removed the translation of the trace strings. Apart from that this series is unchanged. There shouldn't be any merge conflicts with the updated dl/merge-cleanup-scissors-fix I posted earlier. (There are conflicts with other series but they are already resolved in pu) This series still based on a merge of master [e902e9bcae ("The second batch", 2019-03-11)] and ag/sequencer-reduce-rewriting-todo ed35d18841 ("rebase--interactive: move transform_todo_file()", 2019-03-05). It can be fetched from the tag rebase-i-no-fork/v1 at https://github.com/phillipwood/git.git Phillip Wood (12): sequencer: always discard index after checkout rebase: don't translate trace strings rebase: rename write_basic_state() rebase: use OPT_RERERE_AUTOUPDATE() rebase -i: combine rebase--interactive.c with rebase.c rebase -i: remove duplication rebase -i: use struct commit when parsing options rebase -i: use struct object_id for squash_onto rebase -i: use struct rebase_options to parse args rebase -i: use struct rebase_options in do_interactive_rebase() rebase: use a common action enum rebase -i: run without forking rebase--interactive Makefile | 1 - builtin/rebase--interactive.c | 377 -------------------- builtin/rebase.c | 624 ++++++++++++++++++++++++++-------- parse-options-cb.c | 34 ++ parse-options.h | 4 + sequencer.c | 42 ++- sequencer.h | 7 +- 7 files changed, 555 insertions(+), 534 deletions(-) delete mode 100644 builtin/rebase--interactive.c Range-diff to rfc 1: 549d45dc63 = 1: fe51202dfb sequencer: always discard index after checkout -: ---------- > 2: 2598165c0f rebase: don't translate trace strings 2: d896a3c76f = 3: 48801ccdd5 rebase: rename write_basic_state() 3: 253de33871 = 4: 66972f0f7d rebase: use OPT_RERERE_AUTOUPDATE() 4: 0aaeaf7c09 = 5: c18aa58305 rebase -i: combine rebase--interactive.c with rebase.c 5: ff539c37d1 = 6: 27982630d1 rebase -i: remove duplication 6: 71559a38db = 7: f28e8df52a rebase -i: use struct commit when parsing options 7: 34c9e4ae96 = 8: 1ae13b5a7d rebase -i: use struct object_id for squash_onto 8: 4fe81c9e81 ! 9: efcc32d8e3 rebase -i: use struct rebase_options to parse args @@ -65,9 +65,9 @@ +{ + struct replay_opts replay = REPLAY_OPTS_INIT; + -+ sequencer_init_config(&replay); -+ + replay.action = REPLAY_INTERACTIVE_REBASE; ++ sequencer_init_config(&replay); ++ + replay.signoff = opts->signoff; + replay.allow_ff = !(opts->flags & REBASE_FORCE); + if (opts->allow_rerere_autoupdate) 9: 09343e540f = 10: c89d815ad4 rebase -i: use struct rebase_options in do_interactive_rebase() 10: 9ad1482d7f ! 11: fda591f006 rebase: use a common action enum @@ -38,14 +30,13 @@ + ACTION_ADD_EXEC +}; + -+static const char *action_names[] = { N_("undefined"), -+ N_("continue"), -+ N_("skip"), -+ N_("abort"), -+ N_("quit"), -+ N_("edit_todo"), -+ N_("show_current_patch"), -+ NULL }; ++static const char *action_names[] = { "undefined", ++ "continue", ++ "skip", ++ "abort", ++ "quit", ++ "edit_todo", ++ "show_current_patch" }; + static int add_exec_commands(struct string_list *commands) { @@ -173,14 +164,13 @@ - ACTION_EDIT_TODO, - ACTION_SHOW_CURRENT_PATCH, - } action = NO_ACTION; -- static const char *action_names[] = { N_("undefined"), -- N_("continue"), -- N_("skip"), -- N_("abort"), -- N_("quit"), -- N_("edit_todo"), -- N_("show_current_patch"), -- NULL }; +- static const char *action_names[] = { "undefined", +- "continue", +- "skip", +- "abort", +- "quit", +- "edit_todo", +- "show_current_patch" }; + enum action action = ACTION_NONE; const char *gpg_sign = NULL; struct string_list exec = STRING_LIST_INIT_NODUP; 11: ebfd7f80d9 ! 12: d921226d32 rebase -i: run without forking rebase--interactive @@ -7,10 +7,18 @@ `rebase--interactive`. Separate the option parsing in cmd_rebase__interactive() from the business logic to allow interactive rebases can be run without forking `rebase__interactive` by calling - run_rebase_interactive() directly. This makes it easy to debug the - sequencer without worrying about attaching to child processes. It also - makes it easy to remove cmd_rebase__interactive() in the future when - git-legacy-rebase.sh and git-rebase--preserve-merges.sh are retired. + run_rebase_interactive() directly. + + Starting interactive rebases without forking makes it easy to debug + the sequencer without worrying about attaching to child + processes. Ævar has also reported that some of the rebase perf tests + are 30% faster [1]. + + This patch also makes it easy to remove cmd_rebase__interactive() in + the future when git-legacy-rebase.sh and + git-rebase--preserve-merges.sh are retired. + + [1] https://public-inbox.org/git/87y359cfjj.fsf@evledraar.gmail.com/ Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> -- 2.21.0 ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v1 01/12] sequencer: always discard index after checkout 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 02/12] rebase: don't translate trace strings Phillip Wood ` (10 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> As the checkout runs in a separate process our index will be out of date so it should be discarded. The existing callers are not doing this consistently so do it here to avoid the callers having to worry about it. This fixes some test failures that happen if do_interactive_rebase() is called without forking rebase--interactive which we will implement shortly. Running git rebase -i master topic starting on master created empty todo lists because all the commits in topic were marked as cherry-picks. After topic was checked out in prepare_branch_to_be_rebased() the working tree contained the contents from topic but the index contained master and the cache entries were still valid. This meant that diff_populate_filespec() which loads the blobs when calculating patch-id's ended up reading the contents for master from the working tree which actually contained topic. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase--interactive.c | 2 +- sequencer.c | 27 +++++++++++++++++---------- sequencer.h | 3 ++- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c index 4535523bf5..d1a4ac1b84 100644 --- a/builtin/rebase--interactive.c +++ b/builtin/rebase--interactive.c @@ -171,7 +171,7 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, struct argv_array make_script_args = ARGV_ARRAY_INIT; struct todo_list todo_list = TODO_LIST_INIT; - if (prepare_branch_to_be_rebased(opts, switch_to)) + if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) return -1; if (get_revision_ranges(upstream, onto, &head_hash, diff --git a/sequencer.c b/sequencer.c index 281a8ade19..ccc0160800 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3418,10 +3418,11 @@ static const char *reflog_message(struct replay_opts *opts, return buf.buf; } -static int run_git_checkout(struct replay_opts *opts, const char *commit, - const char *action) +static int run_git_checkout(struct repository *r, struct replay_opts *opts, + const char *commit, const char *action) { struct child_process cmd = CHILD_PROCESS_INIT; + int ret; cmd.git_cmd = 1; @@ -3430,25 +3431,31 @@ static int run_git_checkout(struct replay_opts *opts, const char *commit, argv_array_pushf(&cmd.env_array, GIT_REFLOG_ACTION "=%s", action); if (opts->verbose) - return run_command(&cmd); + ret = run_command(&cmd); else - return run_command_silent_on_success(&cmd); + ret = run_command_silent_on_success(&cmd); + + if (!ret) + discard_index(r->index); + + return ret; } -int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit) +int prepare_branch_to_be_rebased(struct repository *r, struct replay_opts *opts, + const char *commit) { const char *action; if (commit && *commit) { action = reflog_message(opts, "start", "checkout %s", commit); - if (run_git_checkout(opts, commit, action)) + if (run_git_checkout(r, opts, commit, action)) return error(_("could not checkout %s"), commit); } return 0; } -static int checkout_onto(struct replay_opts *opts, +static int checkout_onto(struct repository *r, struct replay_opts *opts, const char *onto_name, const char *onto, const char *orig_head) { @@ -3458,7 +3465,7 @@ static int checkout_onto(struct replay_opts *opts, if (get_oid(orig_head, &oid)) return error(_("%s: not a valid OID"), orig_head); - if (run_git_checkout(opts, onto, action)) { + if (run_git_checkout(r, opts, onto, action)) { apply_autostash(opts); sequencer_remove_state(opts); return error(_("could not detach HEAD")); @@ -4786,7 +4793,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) || todo_list_check(todo_list, &new_todo)) { fprintf(stderr, _(edit_todo_list_advice)); - checkout_onto(opts, onto_name, onto, orig_head); + checkout_onto(r, opts, onto_name, onto, orig_head); todo_list_release(&new_todo); return -1; @@ -4805,7 +4812,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla todo_list_release(&new_todo); - if (checkout_onto(opts, onto_name, oid_to_hex(&oid), orig_head)) + if (checkout_onto(r, opts, onto_name, oid_to_hex(&oid), orig_head)) return -1; if (require_clean_work_tree(r, "rebase", "", 1, 1)) diff --git a/sequencer.h b/sequencer.h index a515ee4457..6c55aa4200 100644 --- a/sequencer.h +++ b/sequencer.h @@ -175,7 +175,8 @@ void commit_post_rewrite(struct repository *r, const struct commit *current_head, const struct object_id *new_head); -int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit); +int prepare_branch_to_be_rebased(struct repository *r, struct replay_opts *opts, + const char *commit); #define SUMMARY_INITIAL_COMMIT (1 << 0) #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1) -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 02/12] rebase: don't translate trace strings 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood 2019-04-17 14:30 ` [PATCH v1 01/12] sequencer: always discard index after checkout Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-19 5:53 ` Junio C Hamano 2019-04-17 14:30 ` [PATCH v1 03/12] rebase: rename write_basic_state() Phillip Wood ` (9 subsequent siblings) 11 siblings, 1 reply; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood, Jeff Hostetler From: Phillip Wood <phillip.wood@dunelm.org.uk> commit b3a5d5a80c ("trace2:data: add subverb for rebase", 2019-02-22) mistakenly marked the subverb names for translation and unnecessarily NULL terminated the array. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 52114cbf0d..239a54ecfe 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1027,14 +1027,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) ACTION_EDIT_TODO, ACTION_SHOW_CURRENT_PATCH, } action = NO_ACTION; - static const char *action_names[] = { N_("undefined"), - N_("continue"), - N_("skip"), - N_("abort"), - N_("quit"), - N_("edit_todo"), - N_("show_current_patch"), - NULL }; + static const char *action_names[] = { "undefined", + "continue", + "skip", + "abort", + "quit", + "edit_todo", + "show_current_patch" }; const char *gpg_sign = NULL; struct string_list exec = STRING_LIST_INIT_NODUP; const char *rebase_merges = NULL; -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v1 02/12] rebase: don't translate trace strings 2019-04-17 14:30 ` [PATCH v1 02/12] rebase: don't translate trace strings Phillip Wood @ 2019-04-19 5:53 ` Junio C Hamano 2019-04-25 17:47 ` Phillip Wood 0 siblings, 1 reply; 40+ messages in thread From: Junio C Hamano @ 2019-04-19 5:53 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood, Jeff Hostetler Phillip Wood <phillip.wood123@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > commit b3a5d5a80c ("trace2:data: add subverb for rebase", 2019-02-22) > mistakenly marked the subverb names for translation and unnecessarily > NULL terminated the array. > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > builtin/rebase.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 52114cbf0d..239a54ecfe 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -1027,14 +1027,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > ACTION_EDIT_TODO, > ACTION_SHOW_CURRENT_PATCH, > } action = NO_ACTION; > - static const char *action_names[] = { N_("undefined"), > - N_("continue"), > - N_("skip"), > - N_("abort"), > - N_("quit"), > - N_("edit_todo"), > - N_("show_current_patch"), > - NULL }; > + static const char *action_names[] = { "undefined", > + "continue", > + "skip", > + "abort", > + "quit", > + "edit_todo", > + "show_current_patch" }; That's an improvement independent from the rest of the patches. Now we've had the C99 designated initialisers weather balloon changes for some time in our codebase, perhaps we can ensure that these entries match the intended & corresponding "enum action" constants? If we can also ensure that the array is large enough so that the trace2 call done like so trace2_cmd_mode(action_names[action]) is safe, that would be good, but that is secondary. Thanks. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v1 02/12] rebase: don't translate trace strings 2019-04-19 5:53 ` Junio C Hamano @ 2019-04-25 17:47 ` Phillip Wood 0 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-25 17:47 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Johannes Schindelin, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood, Jeff Hostetler On 19/04/2019 06:53, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> commit b3a5d5a80c ("trace2:data: add subverb for rebase", 2019-02-22) >> mistakenly marked the subverb names for translation and unnecessarily >> NULL terminated the array. >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> builtin/rebase.c | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/builtin/rebase.c b/builtin/rebase.c >> index 52114cbf0d..239a54ecfe 100644 >> --- a/builtin/rebase.c >> +++ b/builtin/rebase.c >> @@ -1027,14 +1027,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) >> ACTION_EDIT_TODO, >> ACTION_SHOW_CURRENT_PATCH, >> } action = NO_ACTION; >> - static const char *action_names[] = { N_("undefined"), >> - N_("continue"), >> - N_("skip"), >> - N_("abort"), >> - N_("quit"), >> - N_("edit_todo"), >> - N_("show_current_patch"), >> - NULL }; >> + static const char *action_names[] = { "undefined", >> + "continue", >> + "skip", >> + "abort", >> + "quit", >> + "edit_todo", >> + "show_current_patch" }; > > That's an improvement independent from the rest of the patches. Yes I only included it as I move the definition later in the series > Now we've had the C99 designated initialisers weather balloon > changes for some time in our codebase, perhaps we can ensure that > these entries match the intended & corresponding "enum action" > constants? If we can also ensure that the array is large enough so > that the trace2 call done like so > > trace2_cmd_mode(action_names[action]) > > is safe, that would be good, but that is secondary. > > Thanks. If what's below is ok, I'll send a re-roll, I wasn't sure if it was best to die if action is larger than the array of names or just use a default. My worrying with dying is that it wont be caught by tests and will cause a problem for users who enable tracing. At least with what's below they can still rebase and hopefully report a bug about unknown action in their trace output. Best Wishes Phillip diff --git a/builtin/rebase.c b/builtin/rebase.c index 52114cbf0d..3f56be230e 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1027,14 +1027,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) ACTION_EDIT_TODO, ACTION_SHOW_CURRENT_PATCH, } action = NO_ACTION; - static const char *action_names[] = { N_("undefined"), - N_("continue"), - N_("skip"), - N_("abort"), - N_("quit"), - N_("edit_todo"), - N_("show_current_patch"), - NULL }; + static const char *action_names[] = { + [NO_ACTION] = "undefined", + [ACTION_CONTINUE] = "continue", + [ACTION_SKIP] = "skip", + [ACTION_ABORT] = "abort", + [ACTION_QUIT] = "quit", + [ACTION_EDIT_TODO] = "edit_todo", + [ACTION_SHOW_CURRENT_PATCH] = "show_current_patch" + }; const char *gpg_sign = NULL; struct string_list exec = STRING_LIST_INIT_NODUP; const char *rebase_merges = NULL; @@ -1225,8 +1226,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) trace2_cmd_mode("interactive"); else if (exec.nr) trace2_cmd_mode("interactive-exec"); - else + else if (action < ARRAY_SIZE(action_names)) trace2_cmd_mode(action_names[action]); + else + trace2_cmd_mode("unknown rebase action"); } switch (action) { ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 03/12] rebase: rename write_basic_state() 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood 2019-04-17 14:30 ` [PATCH v1 01/12] sequencer: always discard index after checkout Phillip Wood 2019-04-17 14:30 ` [PATCH v1 02/12] rebase: don't translate trace strings Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 04/12] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood ` (8 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> This clashes with a function in sequencer.c Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 239a54ecfe..0b1a193a8e 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -245,7 +245,7 @@ static int read_basic_state(struct rebase_options *opts) return 0; } -static int write_basic_state(struct rebase_options *opts) +static int rebase_write_basic_state(struct rebase_options *opts) { write_file(state_dir_path("head-name", opts), "%s", opts->head_name ? opts->head_name : "detached HEAD"); @@ -640,7 +640,7 @@ static int run_am(struct rebase_options *opts) } if (is_directory(opts->state_dir)) - write_basic_state(opts); + rebase_write_basic_state(opts); return status; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 04/12] rebase: use OPT_RERERE_AUTOUPDATE() 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (2 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 03/12] rebase: rename write_basic_state() Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 05/12] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood ` (7 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> As we have a macro for this it makes sense to use it. Having cmd_rebase() and cmd_rebase__interactive() use the same values for this option will be helpful when we start running interactive rebases without forking rebase--interactive. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 0b1a193a8e..e7b8122003 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -206,14 +206,13 @@ static int read_basic_state(struct rebase_options *opts) &buf)) return -1; if (!strcmp(buf.buf, "--rerere-autoupdate")) - opts->allow_rerere_autoupdate = 1; + opts->allow_rerere_autoupdate = RERERE_AUTOUPDATE; else if (!strcmp(buf.buf, "--no-rerere-autoupdate")) - opts->allow_rerere_autoupdate = 0; + opts->allow_rerere_autoupdate = RERERE_NOAUTOUPDATE; else warning(_("ignoring invalid allow_rerere_autoupdate: " "'%s'"), buf.buf); - } else - opts->allow_rerere_autoupdate = -1; + } if (file_exists(state_dir_path("gpg_sign_opt", opts))) { strbuf_reset(&buf); @@ -263,10 +262,11 @@ static int rebase_write_basic_state(struct rebase_options *opts) if (opts->strategy_opts) write_file(state_dir_path("strategy_opts", opts), "%s", opts->strategy_opts); - if (opts->allow_rerere_autoupdate >= 0) + if (opts->allow_rerere_autoupdate > 0) write_file(state_dir_path("allow_rerere_autoupdate", opts), "-%s-rerere-autoupdate", - opts->allow_rerere_autoupdate ? "" : "-no"); + opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE ? + "" : "-no"); if (opts->gpg_sign_opt) write_file(state_dir_path("gpg_sign_opt", opts), "%s", opts->gpg_sign_opt); @@ -625,9 +625,9 @@ static int run_am(struct rebase_options *opts) argv_array_push(&am.args, "--rebasing"); argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg); argv_array_push(&am.args, "--patch-format=mboxrd"); - if (opts->allow_rerere_autoupdate > 0) + if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) argv_array_push(&am.args, "--rerere-autoupdate"); - else if (opts->allow_rerere_autoupdate == 0) + else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE) argv_array_push(&am.args, "--no-rerere-autoupdate"); if (opts->gpg_sign_opt) argv_array_push(&am.args, opts->gpg_sign_opt); @@ -713,9 +713,9 @@ static int run_specific_rebase(struct rebase_options *opts) argv_array_pushf(&child.args, "--cmd=%s", opts->cmd); if (opts->allow_empty_message) argv_array_push(&child.args, "--allow-empty-message"); - if (opts->allow_rerere_autoupdate > 0) + if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) argv_array_push(&child.args, "--rerere-autoupdate"); - else if (opts->allow_rerere_autoupdate == 0) + else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE) argv_array_push(&child.args, "--no-rerere-autoupdate"); if (opts->gpg_sign_opt) argv_array_push(&child.args, opts->gpg_sign_opt); @@ -764,9 +764,9 @@ static int run_specific_rebase(struct rebase_options *opts) add_var(&script_snippet, "action", opts->action ? opts->action : ""); add_var(&script_snippet, "signoff", opts->signoff ? "--signoff" : ""); add_var(&script_snippet, "allow_rerere_autoupdate", - opts->allow_rerere_autoupdate < 0 ? "" : opts->allow_rerere_autoupdate ? - "--rerere-autoupdate" : "--no-rerere-autoupdate"); + opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE ? + "--rerere-autoupdate" : "--no-rerere-autoupdate" : ""); add_var(&script_snippet, "keep_empty", opts->keep_empty ? "yes" : ""); add_var(&script_snippet, "autosquash", opts->autosquash ? "t" : ""); add_var(&script_snippet, "gpg_sign_opt", opts->gpg_sign_opt); @@ -1007,7 +1007,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) .type = REBASE_UNSPECIFIED, .flags = REBASE_NO_QUIET, .git_am_opts = ARGV_ARRAY_INIT, - .allow_rerere_autoupdate = -1, .allow_empty_message = 1, .git_format_patch_opt = STRBUF_INIT, }; @@ -1101,10 +1100,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) OPT_SET_INT('p', "preserve-merges", &options.type, N_("try to recreate merges instead of ignoring " "them"), REBASE_PRESERVE_MERGES), - OPT_BOOL(0, "rerere-autoupdate", - &options.allow_rerere_autoupdate, - N_("allow rerere to update index with resolved " - "conflict")), + OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate), OPT_BOOL('k', "keep-empty", &options.keep_empty, N_("preserve empty commits during rebase")), OPT_BOOL(0, "autosquash", &options.autosquash, -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 05/12] rebase -i: combine rebase--interactive.c with rebase.c 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (3 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 04/12] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 06/12] rebase -i: remove duplication Phillip Wood ` (6 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> In order to run `rebase -i` without forking `rebase--interactive` it will be convenient to have all the code from rebase--interactive.c in rebase.c. This is a straight forward copy of the code from rebase--interactive.c, it will be simplified slightly in the next commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- Makefile | 1 - builtin/rebase--interactive.c | 377 ---------------------------------- builtin/rebase.c | 367 +++++++++++++++++++++++++++++++++ 3 files changed, 367 insertions(+), 378 deletions(-) delete mode 100644 builtin/rebase--interactive.c diff --git a/Makefile b/Makefile index 537493822b..c644089244 100644 --- a/Makefile +++ b/Makefile @@ -1130,7 +1130,6 @@ BUILTIN_OBJS += builtin/push.o BUILTIN_OBJS += builtin/range-diff.o BUILTIN_OBJS += builtin/read-tree.o BUILTIN_OBJS += builtin/rebase.o -BUILTIN_OBJS += builtin/rebase--interactive.o BUILTIN_OBJS += builtin/receive-pack.o BUILTIN_OBJS += builtin/reflog.o BUILTIN_OBJS += builtin/remote.o diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c deleted file mode 100644 index d1a4ac1b84..0000000000 --- a/builtin/rebase--interactive.c +++ /dev/null @@ -1,377 +0,0 @@ -#define USE_THE_INDEX_COMPATIBILITY_MACROS -#include "builtin.h" -#include "cache.h" -#include "config.h" -#include "parse-options.h" -#include "sequencer.h" -#include "rebase-interactive.h" -#include "argv-array.h" -#include "refs.h" -#include "rerere.h" -#include "run-command.h" - -static GIT_PATH_FUNC(path_state_dir, "rebase-merge/") -static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto") -static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive") - -static int add_exec_commands(struct string_list *commands) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT; - int res; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) { - todo_list_release(&todo_list); - return error(_("unusable todo list: '%s'"), todo_file); - } - - todo_list_add_exec_commands(&todo_list, commands); - res = todo_list_write_to_file(the_repository, &todo_list, - todo_file, NULL, NULL, -1, 0); - todo_list_release(&todo_list); - - if (res) - return error_errno(_("could not write '%s'."), todo_file); - return 0; -} - -static int rearrange_squash_in_todo_file(void) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT; - int res = 0; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) { - todo_list_release(&todo_list); - return error(_("unusable todo list: '%s'"), todo_file); - } - - res = todo_list_rearrange_squash(&todo_list); - if (!res) - res = todo_list_write_to_file(the_repository, &todo_list, - todo_file, NULL, NULL, -1, 0); - - todo_list_release(&todo_list); - - if (res) - return error_errno(_("could not write '%s'."), todo_file); - return 0; -} - -static int transform_todo_file(unsigned flags) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT; - int res; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) { - todo_list_release(&todo_list); - return error(_("unusable todo list: '%s'"), todo_file); - } - - res = todo_list_write_to_file(the_repository, &todo_list, todo_file, - NULL, NULL, -1, flags); - todo_list_release(&todo_list); - - if (res) - return error_errno(_("could not write '%s'."), todo_file); - return 0; -} - -static int edit_todo_file(unsigned flags) -{ - const char *todo_file = rebase_path_todo(); - struct todo_list todo_list = TODO_LIST_INIT, - new_todo = TODO_LIST_INIT; - int res = 0; - - if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) - return error_errno(_("could not read '%s'."), todo_file); - - strbuf_stripspace(&todo_list.buf, 1); - res = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags); - if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file, - NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS))) - res = error_errno(_("could not write '%s'"), todo_file); - - todo_list_release(&todo_list); - todo_list_release(&new_todo); - - return res; -} - -static int get_revision_ranges(const char *upstream, const char *onto, - const char **head_hash, - char **revisions, char **shortrevisions) -{ - const char *base_rev = upstream ? upstream : onto, *shorthead; - struct object_id orig_head; - - if (get_oid("HEAD", &orig_head)) - return error(_("no HEAD?")); - - *head_hash = find_unique_abbrev(&orig_head, GIT_MAX_HEXSZ); - *revisions = xstrfmt("%s...%s", base_rev, *head_hash); - - shorthead = find_unique_abbrev(&orig_head, DEFAULT_ABBREV); - - if (upstream) { - const char *shortrev; - struct object_id rev_oid; - - get_oid(base_rev, &rev_oid); - shortrev = find_unique_abbrev(&rev_oid, DEFAULT_ABBREV); - - *shortrevisions = xstrfmt("%s..%s", shortrev, shorthead); - } else - *shortrevisions = xstrdup(shorthead); - - return 0; -} - -static int init_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head) -{ - FILE *interactive; - - if (!is_directory(path_state_dir()) && mkdir_in_gitdir(path_state_dir())) - return error_errno(_("could not create temporary %s"), path_state_dir()); - - delete_reflog("REBASE_HEAD"); - - interactive = fopen(path_interactive(), "w"); - if (!interactive) - return error_errno(_("could not mark as interactive")); - fclose(interactive); - - return write_basic_state(opts, head_name, onto, orig_head); -} - -static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, - const char *switch_to, const char *upstream, - const char *onto, const char *onto_name, - const char *squash_onto, const char *head_name, - const char *restrict_revision, char *raw_strategies, - struct string_list *commands, unsigned autosquash) -{ - int ret; - const char *head_hash = NULL; - char *revisions = NULL, *shortrevisions = NULL; - struct argv_array make_script_args = ARGV_ARRAY_INIT; - struct todo_list todo_list = TODO_LIST_INIT; - - if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) - return -1; - - if (get_revision_ranges(upstream, onto, &head_hash, - &revisions, &shortrevisions)) - return -1; - - if (raw_strategies) - parse_strategy_opts(opts, raw_strategies); - - if (init_basic_state(opts, head_name, onto, head_hash)) { - free(revisions); - free(shortrevisions); - - return -1; - } - - if (!upstream && squash_onto) - write_file(path_squash_onto(), "%s\n", squash_onto); - - argv_array_pushl(&make_script_args, "", revisions, NULL); - if (restrict_revision) - argv_array_push(&make_script_args, restrict_revision); - - ret = sequencer_make_script(the_repository, &todo_list.buf, - make_script_args.argc, make_script_args.argv, - flags); - - if (ret) - error(_("could not generate todo list")); - else { - discard_cache(); - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) - BUG("unusable todo list"); - - ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name, - onto, head_hash, commands, autosquash, &todo_list); - } - - free(revisions); - free(shortrevisions); - todo_list_release(&todo_list); - argv_array_clear(&make_script_args); - - return ret; -} - -static const char * const builtin_rebase_interactive_usage[] = { - N_("git rebase--interactive [<options>]"), - NULL -}; - -int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) -{ - struct replay_opts opts = REPLAY_OPTS_INIT; - unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; - int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL, - *squash_onto = NULL, *upstream = NULL, *head_name = NULL, - *switch_to = NULL, *cmd = NULL; - struct string_list commands = STRING_LIST_INIT_DUP; - char *raw_strategies = NULL; - enum { - NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, - SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC - } command = 0; - struct option options[] = { - OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), - OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")), - OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, - N_("allow commits with empty messages")), - OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")), - OPT_BOOL(0, "rebase-cousins", &rebase_cousins, - N_("keep original branch points of cousins")), - OPT_BOOL(0, "autosquash", &autosquash, - N_("move commits that begin with squash!/fixup!")), - OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")), - OPT__VERBOSE(&opts.verbose, N_("be verbose")), - OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), - CONTINUE), - OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), - OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), - EDIT_TODO), - OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), - SHOW_CURRENT_PATCH), - OPT_CMDMODE(0, "shorten-ids", &command, - N_("shorten commit ids in the todo list"), SHORTEN_OIDS), - OPT_CMDMODE(0, "expand-ids", &command, - N_("expand commit ids in the todo list"), EXPAND_OIDS), - OPT_CMDMODE(0, "check-todo-list", &command, - N_("check the todo list"), CHECK_TODO_LIST), - OPT_CMDMODE(0, "rearrange-squash", &command, - N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), - OPT_CMDMODE(0, "add-exec-commands", &command, - N_("insert exec commands in todo list"), ADD_EXEC), - OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")), - OPT_STRING(0, "restrict-revision", &restrict_revision, - N_("restrict-revision"), N_("restrict revision")), - OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), - N_("squash onto")), - OPT_STRING(0, "upstream", &upstream, N_("upstream"), - N_("the upstream commit")), - OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), - { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), - N_("GPG-sign commits"), - PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, - OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"), - N_("rebase strategy")), - OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"), - N_("strategy options")), - OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"), - N_("the branch or commit to checkout")), - OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")), - OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")), - OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto), - OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec, - N_("automatically re-schedule any `exec` that fails")), - OPT_END() - }; - - sequencer_init_config(&opts); - git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); - - opts.action = REPLAY_INTERACTIVE_REBASE; - opts.allow_ff = 1; - opts.allow_empty = 1; - - if (argc == 1) - usage_with_options(builtin_rebase_interactive_usage, options); - - argc = parse_options(argc, argv, NULL, options, - builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0); - - opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); - - flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; - flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; - flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; - flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; - flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; - - if (rebase_cousins >= 0 && !rebase_merges) - warning(_("--[no-]rebase-cousins has no effect without " - "--rebase-merges")); - - if (cmd && *cmd) { - string_list_split(&commands, cmd, '\n', -1); - - /* rebase.c adds a new line to cmd after every command, - * so here the last command is always empty */ - string_list_remove_empty_items(&commands, 0); - } - - switch (command) { - case NONE: - if (!onto && !upstream) - die(_("a base commit must be provided with --upstream or --onto")); - - ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, - onto_name, squash_onto, head_name, restrict_revision, - raw_strategies, &commands, autosquash); - break; - case SKIP: { - struct string_list merge_rr = STRING_LIST_INIT_DUP; - - rerere_clear(the_repository, &merge_rr); - /* fallthrough */ - case CONTINUE: - ret = sequencer_continue(the_repository, &opts); - break; - } - case EDIT_TODO: - ret = edit_todo_file(flags); - break; - case SHOW_CURRENT_PATCH: { - struct child_process cmd = CHILD_PROCESS_INIT; - - cmd.git_cmd = 1; - argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); - ret = run_command(&cmd); - - break; - } - case SHORTEN_OIDS: - case EXPAND_OIDS: - ret = transform_todo_file(flags); - break; - case CHECK_TODO_LIST: - ret = check_todo_list_from_file(the_repository); - break; - case REARRANGE_SQUASH: - ret = rearrange_squash_in_todo_file(); - break; - case ADD_EXEC: - ret = add_exec_commands(&commands); - break; - default: - BUG("invalid command '%d'", command); - } - - string_list_clear(&commands, 0); - return !!ret; -} diff --git a/builtin/rebase.c b/builtin/rebase.c index e7b8122003..57a3c9caf1 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -25,6 +25,8 @@ #include "commit-reach.h" #include "rerere.h" #include "branch.h" +#include "sequencer.h" +#include "rebase-interactive.h" static char const * const builtin_rebase_usage[] = { N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] " @@ -35,6 +37,9 @@ static char const * const builtin_rebase_usage[] = { NULL }; +static GIT_PATH_FUNC(path_state_dir, "rebase-merge/") +static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto") +static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive") static GIT_PATH_FUNC(apply_dir, "rebase-apply") static GIT_PATH_FUNC(merge_dir, "rebase-merge") @@ -46,6 +51,368 @@ enum rebase_type { REBASE_PRESERVE_MERGES }; +static int add_exec_commands(struct string_list *commands) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT; + int res; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) { + todo_list_release(&todo_list); + return error(_("unusable todo list: '%s'"), todo_file); + } + + todo_list_add_exec_commands(&todo_list, commands); + res = todo_list_write_to_file(the_repository, &todo_list, + todo_file, NULL, NULL, -1, 0); + todo_list_release(&todo_list); + + if (res) + return error_errno(_("could not write '%s'."), todo_file); + return 0; +} + +static int rearrange_squash_in_todo_file(void) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT; + int res = 0; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) { + todo_list_release(&todo_list); + return error(_("unusable todo list: '%s'"), todo_file); + } + + res = todo_list_rearrange_squash(&todo_list); + if (!res) + res = todo_list_write_to_file(the_repository, &todo_list, + todo_file, NULL, NULL, -1, 0); + + todo_list_release(&todo_list); + + if (res) + return error_errno(_("could not write '%s'."), todo_file); + return 0; +} + +static int transform_todo_file(unsigned flags) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT; + int res; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) { + todo_list_release(&todo_list); + return error(_("unusable todo list: '%s'"), todo_file); + } + + res = todo_list_write_to_file(the_repository, &todo_list, todo_file, + NULL, NULL, -1, flags); + todo_list_release(&todo_list); + + if (res) + return error_errno(_("could not write '%s'."), todo_file); + return 0; +} + +static int edit_todo_file(unsigned flags) +{ + const char *todo_file = rebase_path_todo(); + struct todo_list todo_list = TODO_LIST_INIT, + new_todo = TODO_LIST_INIT; + int res = 0; + + if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) + return error_errno(_("could not read '%s'."), todo_file); + + strbuf_stripspace(&todo_list.buf, 1); + res = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags); + if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file, + NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS))) + res = error_errno(_("could not write '%s'"), todo_file); + + todo_list_release(&todo_list); + todo_list_release(&new_todo); + + return res; +} + +static int get_revision_ranges(const char *upstream, const char *onto, + const char **head_hash, + char **revisions, char **shortrevisions) +{ + const char *base_rev = upstream ? upstream : onto, *shorthead; + struct object_id orig_head; + + if (get_oid("HEAD", &orig_head)) + return error(_("no HEAD?")); + + *head_hash = find_unique_abbrev(&orig_head, GIT_MAX_HEXSZ); + *revisions = xstrfmt("%s...%s", base_rev, *head_hash); + + shorthead = find_unique_abbrev(&orig_head, DEFAULT_ABBREV); + + if (upstream) { + const char *shortrev; + struct object_id rev_oid; + + get_oid(base_rev, &rev_oid); + shortrev = find_unique_abbrev(&rev_oid, DEFAULT_ABBREV); + + *shortrevisions = xstrfmt("%s..%s", shortrev, shorthead); + } else + *shortrevisions = xstrdup(shorthead); + + return 0; +} + +static int init_basic_state(struct replay_opts *opts, const char *head_name, + const char *onto, const char *orig_head) +{ + FILE *interactive; + + if (!is_directory(path_state_dir()) && mkdir_in_gitdir(path_state_dir())) + return error_errno(_("could not create temporary %s"), path_state_dir()); + + delete_reflog("REBASE_HEAD"); + + interactive = fopen(path_interactive(), "w"); + if (!interactive) + return error_errno(_("could not mark as interactive")); + fclose(interactive); + + return write_basic_state(opts, head_name, onto, orig_head); +} + +static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, + const char *switch_to, const char *upstream, + const char *onto, const char *onto_name, + const char *squash_onto, const char *head_name, + const char *restrict_revision, char *raw_strategies, + struct string_list *commands, unsigned autosquash) +{ + int ret; + const char *head_hash = NULL; + char *revisions = NULL, *shortrevisions = NULL; + struct argv_array make_script_args = ARGV_ARRAY_INIT; + struct todo_list todo_list = TODO_LIST_INIT; + + if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) + return -1; + + if (get_revision_ranges(upstream, onto, &head_hash, + &revisions, &shortrevisions)) + return -1; + + if (raw_strategies) + parse_strategy_opts(opts, raw_strategies); + + if (init_basic_state(opts, head_name, onto, head_hash)) { + free(revisions); + free(shortrevisions); + + return -1; + } + + if (!upstream && squash_onto) + write_file(path_squash_onto(), "%s\n", squash_onto); + + argv_array_pushl(&make_script_args, "", revisions, NULL); + if (restrict_revision) + argv_array_push(&make_script_args, restrict_revision); + + ret = sequencer_make_script(the_repository, &todo_list.buf, + make_script_args.argc, make_script_args.argv, + flags); + + if (ret) + error(_("could not generate todo list")); + else { + discard_cache(); + if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, + &todo_list)) + BUG("unusable todo list"); + + ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name, + onto, head_hash, commands, autosquash, &todo_list); + } + + free(revisions); + free(shortrevisions); + todo_list_release(&todo_list); + argv_array_clear(&make_script_args); + + return ret; +} + +static const char * const builtin_rebase_interactive_usage[] = { + N_("git rebase--interactive [<options>]"), + NULL +}; + +int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) +{ + struct replay_opts opts = REPLAY_OPTS_INIT; + unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; + int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; + const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL, + *squash_onto = NULL, *upstream = NULL, *head_name = NULL, + *switch_to = NULL, *cmd = NULL; + struct string_list commands = STRING_LIST_INIT_DUP; + char *raw_strategies = NULL; + enum { + NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, + SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC + } command = 0; + struct option options[] = { + OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), + OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")), + OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, + N_("allow commits with empty messages")), + OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")), + OPT_BOOL(0, "rebase-cousins", &rebase_cousins, + N_("keep original branch points of cousins")), + OPT_BOOL(0, "autosquash", &autosquash, + N_("move commits that begin with squash!/fixup!")), + OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")), + OPT__VERBOSE(&opts.verbose, N_("be verbose")), + OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), + CONTINUE), + OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), + OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), + EDIT_TODO), + OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), + SHOW_CURRENT_PATCH), + OPT_CMDMODE(0, "shorten-ids", &command, + N_("shorten commit ids in the todo list"), SHORTEN_OIDS), + OPT_CMDMODE(0, "expand-ids", &command, + N_("expand commit ids in the todo list"), EXPAND_OIDS), + OPT_CMDMODE(0, "check-todo-list", &command, + N_("check the todo list"), CHECK_TODO_LIST), + OPT_CMDMODE(0, "rearrange-squash", &command, + N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), + OPT_CMDMODE(0, "add-exec-commands", &command, + N_("insert exec commands in todo list"), ADD_EXEC), + OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")), + OPT_STRING(0, "restrict-revision", &restrict_revision, + N_("restrict-revision"), N_("restrict revision")), + OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), + N_("squash onto")), + OPT_STRING(0, "upstream", &upstream, N_("upstream"), + N_("the upstream commit")), + OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), + { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), + N_("GPG-sign commits"), + PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"), + N_("rebase strategy")), + OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"), + N_("strategy options")), + OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"), + N_("the branch or commit to checkout")), + OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")), + OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")), + OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto), + OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec, + N_("automatically re-schedule any `exec` that fails")), + OPT_END() + }; + + sequencer_init_config(&opts); + git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); + + opts.action = REPLAY_INTERACTIVE_REBASE; + opts.allow_ff = 1; + opts.allow_empty = 1; + + if (argc == 1) + usage_with_options(builtin_rebase_interactive_usage, options); + + argc = parse_options(argc, argv, NULL, options, + builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0); + + opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); + + flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; + flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; + flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; + flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; + + if (rebase_cousins >= 0 && !rebase_merges) + warning(_("--[no-]rebase-cousins has no effect without " + "--rebase-merges")); + + if (cmd && *cmd) { + string_list_split(&commands, cmd, '\n', -1); + + /* rebase.c adds a new line to cmd after every command, + * so here the last command is always empty */ + string_list_remove_empty_items(&commands, 0); + } + + switch (command) { + case NONE: + if (!onto && !upstream) + die(_("a base commit must be provided with --upstream or --onto")); + + ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, + onto_name, squash_onto, head_name, restrict_revision, + raw_strategies, &commands, autosquash); + break; + case SKIP: { + struct string_list merge_rr = STRING_LIST_INIT_DUP; + + rerere_clear(the_repository, &merge_rr); + /* fallthrough */ + case CONTINUE: + ret = sequencer_continue(the_repository, &opts); + break; + } + case EDIT_TODO: + ret = edit_todo_file(flags); + break; + case SHOW_CURRENT_PATCH: { + struct child_process cmd = CHILD_PROCESS_INIT; + + cmd.git_cmd = 1; + argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); + ret = run_command(&cmd); + + break; + } + case SHORTEN_OIDS: + case EXPAND_OIDS: + ret = transform_todo_file(flags); + break; + case CHECK_TODO_LIST: + ret = check_todo_list_from_file(the_repository); + break; + case REARRANGE_SQUASH: + ret = rearrange_squash_in_todo_file(); + break; + case ADD_EXEC: + ret = add_exec_commands(&commands); + break; + default: + BUG("invalid command '%d'", command); + } + + string_list_clear(&commands, 0); + return !!ret; +} + static int use_builtin_rebase(void) { struct child_process cp = CHILD_PROCESS_INIT; -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 06/12] rebase -i: remove duplication 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (4 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 05/12] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 07/12] rebase -i: use struct commit when parsing options Phillip Wood ` (5 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> path_state_dir() and merge_dir() refer to the same path so remove one of them. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 57a3c9caf1..610b67827b 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -37,7 +37,6 @@ static char const * const builtin_rebase_usage[] = { NULL }; -static GIT_PATH_FUNC(path_state_dir, "rebase-merge/") static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto") static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive") static GIT_PATH_FUNC(apply_dir, "rebase-apply") @@ -182,8 +181,8 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, { FILE *interactive; - if (!is_directory(path_state_dir()) && mkdir_in_gitdir(path_state_dir())) - return error_errno(_("could not create temporary %s"), path_state_dir()); + if (!is_directory(merge_dir()) && mkdir_in_gitdir(merge_dir())) + return error_errno(_("could not create temporary %s"), merge_dir()); delete_reflog("REBASE_HEAD"); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 07/12] rebase -i: use struct commit when parsing options 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (5 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 06/12] rebase -i: remove duplication Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 08/12] rebase -i: use struct object_id for squash_onto Phillip Wood ` (4 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> This is in preparation for using `struct rebase_options` when parsing options in cmd_rebase__interactive(). Using a string for onto, restrict_revision and upstream, was a hangover from the scripted version of rebase. The functions that use these variables are updated to take a `struct commit`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 42 ++++++++++++++++++++++++------------------ parse-options-cb.c | 17 +++++++++++++++++ parse-options.h | 1 + sequencer.c | 21 +++++++++++---------- sequencer.h | 4 ++-- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 610b67827b..06f6490ca3 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -147,27 +147,28 @@ static int edit_todo_file(unsigned flags) return res; } -static int get_revision_ranges(const char *upstream, const char *onto, +static int get_revision_ranges(struct commit *upstream, struct commit *onto, const char **head_hash, char **revisions, char **shortrevisions) { - const char *base_rev = upstream ? upstream : onto, *shorthead; + struct commit *base_rev = upstream ? upstream : onto; + const char *shorthead; struct object_id orig_head; if (get_oid("HEAD", &orig_head)) return error(_("no HEAD?")); *head_hash = find_unique_abbrev(&orig_head, GIT_MAX_HEXSZ); - *revisions = xstrfmt("%s...%s", base_rev, *head_hash); + *revisions = xstrfmt("%s...%s", oid_to_hex(&base_rev->object.oid), + *head_hash); shorthead = find_unique_abbrev(&orig_head, DEFAULT_ABBREV); if (upstream) { const char *shortrev; - struct object_id rev_oid; - get_oid(base_rev, &rev_oid); - shortrev = find_unique_abbrev(&rev_oid, DEFAULT_ABBREV); + shortrev = find_unique_abbrev(&base_rev->object.oid, + DEFAULT_ABBREV); *shortrevisions = xstrfmt("%s..%s", shortrev, shorthead); } else @@ -177,7 +178,7 @@ static int get_revision_ranges(const char *upstream, const char *onto, } static int init_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head) + struct commit *onto, const char *orig_head) { FILE *interactive; @@ -195,10 +196,10 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, } static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, - const char *switch_to, const char *upstream, - const char *onto, const char *onto_name, + const char *switch_to, struct commit *upstream, + struct commit *onto, const char *onto_name, const char *squash_onto, const char *head_name, - const char *restrict_revision, char *raw_strategies, + struct commit *restrict_revision, char *raw_strategies, struct string_list *commands, unsigned autosquash) { int ret; @@ -229,7 +230,8 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, argv_array_pushl(&make_script_args, "", revisions, NULL); if (restrict_revision) - argv_array_push(&make_script_args, restrict_revision); + argv_array_push(&make_script_args, + oid_to_hex(&restrict_revision->object.oid)); ret = sequencer_make_script(the_repository, &todo_list.buf, make_script_args.argc, make_script_args.argv, @@ -265,9 +267,10 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) struct replay_opts opts = REPLAY_OPTS_INIT; unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL, - *squash_onto = NULL, *upstream = NULL, *head_name = NULL, + const char *onto_name = NULL, + *squash_onto = NULL, *head_name = NULL, *switch_to = NULL, *cmd = NULL; + struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL; struct string_list commands = STRING_LIST_INIT_DUP; char *raw_strategies = NULL; enum { @@ -303,13 +306,16 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), OPT_CMDMODE(0, "add-exec-commands", &command, N_("insert exec commands in todo list"), ADD_EXEC), - OPT_STRING(0, "onto", &onto, N_("onto"), N_("onto")), - OPT_STRING(0, "restrict-revision", &restrict_revision, - N_("restrict-revision"), N_("restrict revision")), + { OPTION_CALLBACK, 0, "onto", &onto, N_("onto"), N_("onto"), + PARSE_OPT_NONEG, parse_opt_commit, 0 }, + { OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision, + N_("restrict-revision"), N_("restrict revision"), + PARSE_OPT_NONEG, parse_opt_commit, 0 }, OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), N_("squash onto")), - OPT_STRING(0, "upstream", &upstream, N_("upstream"), - N_("the upstream commit")), + { OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"), + N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit, + 0 }, OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), N_("GPG-sign commits"), diff --git a/parse-options-cb.c b/parse-options-cb.c index 2733393546..2206eb763c 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -96,6 +96,23 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_commit(const struct option *opt, const char *arg, int unset) +{ + struct object_id oid; + struct commit *commit; + struct commit **target = opt->value; + + if (!arg) + return -1; + if (get_oid(arg, &oid)) + return error("malformed object name %s", arg); + commit = lookup_commit_reference(the_repository, &oid); + if (!commit) + return error("no such commit %s", arg); + *target = commit; + return 0; +} + int parse_opt_object_name(const struct option *opt, const char *arg, int unset) { struct object_id oid; diff --git a/parse-options.h b/parse-options.h index 7d83e2971d..5a75646618 100644 --- a/parse-options.h +++ b/parse-options.h @@ -266,6 +266,7 @@ int parse_opt_color_flag_cb(const struct option *, const char *, int); int parse_opt_verbosity_cb(const struct option *, const char *, int); int parse_opt_object_name(const struct option *, const char *, int); int parse_opt_commits(const struct option *, const char *, int); +int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); diff --git a/sequencer.c b/sequencer.c index ccc0160800..610b7ece14 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2418,14 +2418,15 @@ static void write_strategy_opts(struct replay_opts *opts) } int write_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head) + struct commit *onto, const char *orig_head) { const char *quiet = getenv("GIT_QUIET"); if (head_name) write_file(rebase_path_head_name(), "%s\n", head_name); if (onto) - write_file(rebase_path_onto(), "%s\n", onto); + write_file(rebase_path_onto(), "%s\n", + oid_to_hex(&onto->object.oid)); if (orig_head) write_file(rebase_path_orig_head(), "%s\n", orig_head); @@ -3456,7 +3457,7 @@ int prepare_branch_to_be_rebased(struct repository *r, struct replay_opts *opts, } static int checkout_onto(struct repository *r, struct replay_opts *opts, - const char *onto_name, const char *onto, + const char *onto_name, const struct object_id *onto, const char *orig_head) { struct object_id oid; @@ -3465,7 +3466,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts, if (get_oid(orig_head, &oid)) return error(_("%s: not a valid OID"), orig_head); - if (run_git_checkout(r, opts, onto, action)) { + if (run_git_checkout(r, opts, oid_to_hex(onto), action)) { apply_autostash(opts); sequencer_remove_state(opts); return error(_("could not detach HEAD")); @@ -4741,16 +4742,16 @@ static int skip_unnecessary_picks(struct repository *r, int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags, const char *shortrevisions, const char *onto_name, - const char *onto, const char *orig_head, struct string_list *commands, - unsigned autosquash, struct todo_list *todo_list) + struct commit *onto, const char *orig_head, + struct string_list *commands, unsigned autosquash, + struct todo_list *todo_list) { const char *shortonto, *todo_file = rebase_path_todo(); struct todo_list new_todo = TODO_LIST_INIT; struct strbuf *buf = &todo_list->buf; - struct object_id oid; + struct object_id oid = onto->object.oid; int res; - get_oid(onto, &oid); shortonto = find_unique_abbrev(&oid, DEFAULT_ABBREV); if (buf->len == 0) { @@ -4793,7 +4794,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) || todo_list_check(todo_list, &new_todo)) { fprintf(stderr, _(edit_todo_list_advice)); - checkout_onto(r, opts, onto_name, onto, orig_head); + checkout_onto(r, opts, onto_name, &onto->object.oid, orig_head); todo_list_release(&new_todo); return -1; @@ -4812,7 +4813,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla todo_list_release(&new_todo); - if (checkout_onto(r, opts, onto_name, oid_to_hex(&oid), orig_head)) + if (checkout_onto(r, opts, onto_name, &oid, orig_head)) return -1; if (require_clean_work_tree(r, "rebase", "", 1, 1)) diff --git a/sequencer.h b/sequencer.h index 6c55aa4200..e640ca21f2 100644 --- a/sequencer.h +++ b/sequencer.h @@ -150,7 +150,7 @@ void todo_list_add_exec_commands(struct todo_list *todo_list, int check_todo_list_from_file(struct repository *r); int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags, const char *shortrevisions, const char *onto_name, - const char *onto, const char *orig_head, struct string_list *commands, + struct commit *onto, const char *orig_head, struct string_list *commands, unsigned autosquash, struct todo_list *todo_list); int todo_list_rearrange_squash(struct todo_list *todo_list); @@ -191,4 +191,4 @@ int read_author_script(const char *path, char **name, char **email, char **date, void parse_strategy_opts(struct replay_opts *opts, char *raw_opts); int write_basic_state(struct replay_opts *opts, const char *head_name, - const char *onto, const char *orig_head); + struct commit *onto, const char *orig_head); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 08/12] rebase -i: use struct object_id for squash_onto 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (6 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 07/12] rebase -i: use struct commit when parsing options Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 09/12] rebase -i: use struct rebase_options to parse args Phillip Wood ` (3 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> More preparation for using `struct rebase_options` in cmd_rebase__interactive(). Using a string was a hangover from the scripted version of rebase, update the functions that use `squash_onto` to take a `sturct object_id`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 21 +++++++++++++-------- parse-options-cb.c | 17 +++++++++++++++++ parse-options.h | 3 +++ 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 06f6490ca3..3b9da69473 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -198,7 +198,7 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, const char *switch_to, struct commit *upstream, struct commit *onto, const char *onto_name, - const char *squash_onto, const char *head_name, + struct object_id *squash_onto, const char *head_name, struct commit *restrict_revision, char *raw_strategies, struct string_list *commands, unsigned autosquash) { @@ -226,7 +226,8 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, } if (!upstream && squash_onto) - write_file(path_squash_onto(), "%s\n", squash_onto); + write_file(path_squash_onto(), "%s\n", + oid_to_hex(squash_onto)); argv_array_pushl(&make_script_args, "", revisions, NULL); if (restrict_revision) @@ -267,10 +268,11 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) struct replay_opts opts = REPLAY_OPTS_INIT; unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto_name = NULL, - *squash_onto = NULL, *head_name = NULL, - *switch_to = NULL, *cmd = NULL; + const char *onto_name = NULL, *head_name = NULL, *switch_to = NULL, + *cmd = NULL; struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL; + struct object_id squash_onto = null_oid; + struct object_id *squash_onto_opt = NULL; struct string_list commands = STRING_LIST_INIT_DUP; char *raw_strategies = NULL; enum { @@ -311,8 +313,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) { OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision, N_("restrict-revision"), N_("restrict revision"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, - OPT_STRING(0, "squash-onto", &squash_onto, N_("squash-onto"), - N_("squash onto")), + { OPTION_CALLBACK, 0, "squash-onto", &squash_onto, N_("squash-onto"), + N_("squash onto"), PARSE_OPT_NONEG, parse_opt_object_id, 0 }, { OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"), N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, @@ -349,6 +351,9 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); + if (!is_null_oid(&squash_onto)) + squash_onto_opt = &squash_onto; + flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; @@ -373,7 +378,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) die(_("a base commit must be provided with --upstream or --onto")); ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, - onto_name, squash_onto, head_name, restrict_revision, + onto_name, squash_onto_opt, head_name, restrict_revision, raw_strategies, &commands, autosquash); break; case SKIP: { diff --git a/parse-options-cb.c b/parse-options-cb.c index 2206eb763c..28ad5cd94b 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -129,6 +129,23 @@ int parse_opt_object_name(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_object_id(const struct option *opt, const char *arg, int unset) +{ + struct object_id oid; + struct object_id *target = opt->value; + + if (unset) { + *target = null_oid; + return 0; + } + if (!arg) + return -1; + if (get_oid(arg, &oid)) + return error(_("malformed object name '%s'"), arg); + *target = oid; + return 0; +} + int parse_opt_tertiary(const struct option *opt, const char *arg, int unset) { int *target = opt->value; diff --git a/parse-options.h b/parse-options.h index 5a75646618..0ab1103bc7 100644 --- a/parse-options.h +++ b/parse-options.h @@ -264,7 +264,10 @@ int parse_opt_abbrev_cb(const struct option *, const char *, int); int parse_opt_expiry_date_cb(const struct option *, const char *, int); int parse_opt_color_flag_cb(const struct option *, const char *, int); int parse_opt_verbosity_cb(const struct option *, const char *, int); +/* value is struct oid_array* */ int parse_opt_object_name(const struct option *, const char *, int); +/* value is struct object_id* */ +int parse_opt_object_id(const struct option *, const char *, int); int parse_opt_commits(const struct option *, const char *, int); int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 09/12] rebase -i: use struct rebase_options to parse args 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (7 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 08/12] rebase -i: use struct object_id for squash_onto Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 10/12] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood ` (2 subsequent siblings) 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> In order to run `rebase -i` without forking `rebase--interactive` it will be convenient to use the same structure when parsing the options in cmd_rebase() and cmd_rebase__interactive(). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 203 ++++++++++++++++++++++++++--------------------- 1 file changed, 112 insertions(+), 91 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 3b9da69473..aa97d81ab8 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -50,6 +50,73 @@ enum rebase_type { REBASE_PRESERVE_MERGES }; +struct rebase_options { + enum rebase_type type; + const char *state_dir; + struct commit *upstream; + const char *upstream_name; + const char *upstream_arg; + char *head_name; + struct object_id orig_head; + struct commit *onto; + const char *onto_name; + const char *revisions; + const char *switch_to; + int root; + struct object_id *squash_onto; + struct commit *restrict_revision; + int dont_finish_rebase; + enum { + REBASE_NO_QUIET = 1<<0, + REBASE_VERBOSE = 1<<1, + REBASE_DIFFSTAT = 1<<2, + REBASE_FORCE = 1<<3, + REBASE_INTERACTIVE_EXPLICIT = 1<<4, + } flags; + struct argv_array git_am_opts; + const char *action; + int signoff; + int allow_rerere_autoupdate; + int keep_empty; + int autosquash; + char *gpg_sign_opt; + int autostash; + char *cmd; + int allow_empty_message; + int rebase_merges, rebase_cousins; + char *strategy, *strategy_opts; + struct strbuf git_format_patch_opt; + int reschedule_failed_exec; +}; + +#define REBASE_OPTIONS_INIT { \ + .type = REBASE_UNSPECIFIED, \ + .flags = REBASE_NO_QUIET, \ + .git_am_opts = ARGV_ARRAY_INIT, \ + .git_format_patch_opt = STRBUF_INIT \ + } + +static struct replay_opts get_replay_opts(const struct rebase_options *opts) +{ + struct replay_opts replay = REPLAY_OPTS_INIT; + + replay.action = REPLAY_INTERACTIVE_REBASE; + sequencer_init_config(&replay); + + replay.signoff = opts->signoff; + replay.allow_ff = !(opts->flags & REBASE_FORCE); + if (opts->allow_rerere_autoupdate) + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; + replay.allow_empty = 1; + replay.allow_empty_message = opts->allow_empty_message; + replay.verbose = opts->flags & REBASE_VERBOSE; + replay.reschedule_failed_exec = opts->reschedule_failed_exec; + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); + replay.strategy = opts->strategy; + + return replay; +} + static int add_exec_commands(struct string_list *commands) { const char *todo_file = rebase_path_todo(); @@ -265,32 +332,30 @@ static const char * const builtin_rebase_interactive_usage[] = { int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) { - struct replay_opts opts = REPLAY_OPTS_INIT; - unsigned flags = 0, keep_empty = 0, rebase_merges = 0, autosquash = 0; - int abbreviate_commands = 0, rebase_cousins = -1, ret = 0; - const char *onto_name = NULL, *head_name = NULL, *switch_to = NULL, - *cmd = NULL; - struct commit *onto = NULL, *upstream = NULL, *restrict_revision = NULL; + struct rebase_options opts = REBASE_OPTIONS_INIT; + unsigned flags = 0; + int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; - struct object_id *squash_onto_opt = NULL; struct string_list commands = STRING_LIST_INIT_DUP; - char *raw_strategies = NULL; enum { NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC } command = 0; struct option options[] = { - OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), - OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")), + OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), + REBASE_FORCE), + OPT_BOOL(0, "keep-empty", &opts.keep_empty, N_("keep empty commits")), OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message, N_("allow commits with empty messages")), - OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")), - OPT_BOOL(0, "rebase-cousins", &rebase_cousins, + OPT_BOOL(0, "rebase-merges", &opts.rebase_merges, N_("rebase merge commits")), + OPT_BOOL(0, "rebase-cousins", &opts.rebase_cousins, N_("keep original branch points of cousins")), - OPT_BOOL(0, "autosquash", &autosquash, + OPT_BOOL(0, "autosquash", &opts.autosquash, N_("move commits that begin with squash!/fixup!")), OPT_BOOL(0, "signoff", &opts.signoff, N_("sign commits")), - OPT__VERBOSE(&opts.verbose, N_("be verbose")), + OPT_BIT('v', "verbose", &opts.flags, + N_("display a diffstat of what changed upstream"), + REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), CONTINUE), OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), @@ -308,86 +373,86 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), OPT_CMDMODE(0, "add-exec-commands", &command, N_("insert exec commands in todo list"), ADD_EXEC), - { OPTION_CALLBACK, 0, "onto", &onto, N_("onto"), N_("onto"), + { OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, - { OPTION_CALLBACK, 0, "restrict-revision", &restrict_revision, + { OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision, N_("restrict-revision"), N_("restrict revision"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, { OPTION_CALLBACK, 0, "squash-onto", &squash_onto, N_("squash-onto"), N_("squash onto"), PARSE_OPT_NONEG, parse_opt_object_id, 0 }, - { OPTION_CALLBACK, 0, "upstream", &upstream, N_("upstream"), + { OPTION_CALLBACK, 0, "upstream", &opts.upstream, N_("upstream"), N_("the upstream commit"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, - OPT_STRING(0, "head-name", &head_name, N_("head-name"), N_("head name")), - { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign, N_("key-id"), + OPT_STRING(0, "head-name", &opts.head_name, N_("head-name"), N_("head name")), + { OPTION_STRING, 'S', "gpg-sign", &opts.gpg_sign_opt, N_("key-id"), N_("GPG-sign commits"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, OPT_STRING(0, "strategy", &opts.strategy, N_("strategy"), N_("rebase strategy")), - OPT_STRING(0, "strategy-opts", &raw_strategies, N_("strategy-opts"), + OPT_STRING(0, "strategy-opts", &opts.strategy_opts, N_("strategy-opts"), N_("strategy options")), - OPT_STRING(0, "switch-to", &switch_to, N_("switch-to"), + OPT_STRING(0, "switch-to", &opts.switch_to, N_("switch-to"), N_("the branch or commit to checkout")), - OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")), - OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")), - OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto), + OPT_STRING(0, "onto-name", &opts.onto_name, N_("onto-name"), N_("onto name")), + OPT_STRING(0, "cmd", &opts.cmd, N_("cmd"), N_("the command to run")), + OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_autoupdate), OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec, N_("automatically re-schedule any `exec` that fails")), OPT_END() }; - sequencer_init_config(&opts); + opts.rebase_cousins = -1; + git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); - opts.action = REPLAY_INTERACTIVE_REBASE; - opts.allow_ff = 1; - opts.allow_empty = 1; - if (argc == 1) usage_with_options(builtin_rebase_interactive_usage, options); argc = parse_options(argc, argv, NULL, options, builtin_rebase_interactive_usage, PARSE_OPT_KEEP_ARGV0); - opts.gpg_sign = xstrdup_or_null(opts.gpg_sign); - if (!is_null_oid(&squash_onto)) - squash_onto_opt = &squash_onto; + opts.squash_onto = &squash_onto; - flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0; + flags |= opts.keep_empty ? TODO_LIST_KEEP_EMPTY : 0; flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; - flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0; - flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; + flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; - if (rebase_cousins >= 0 && !rebase_merges) + if (opts.rebase_cousins >= 0 && !opts.rebase_merges) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - if (cmd && *cmd) { - string_list_split(&commands, cmd, '\n', -1); + if (opts.cmd && *opts.cmd) { + string_list_split(&commands, opts.cmd, '\n', -1); /* rebase.c adds a new line to cmd after every command, * so here the last command is always empty */ string_list_remove_empty_items(&commands, 0); } switch (command) { - case NONE: - if (!onto && !upstream) + case NONE: { + struct replay_opts replay_opts = get_replay_opts(&opts); + if (!opts.onto && !opts.upstream) die(_("a base commit must be provided with --upstream or --onto")); - ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto, - onto_name, squash_onto_opt, head_name, restrict_revision, - raw_strategies, &commands, autosquash); + ret = do_interactive_rebase(&replay_opts, flags, opts.switch_to, opts.upstream, opts.onto, + opts.onto_name, opts.squash_onto, opts.head_name, opts.restrict_revision, + opts.strategy_opts, &commands, opts.autosquash); break; + } case SKIP: { struct string_list merge_rr = STRING_LIST_INIT_DUP; rerere_clear(the_repository, &merge_rr); + } /* fallthrough */ - case CONTINUE: - ret = sequencer_continue(the_repository, &opts); + case CONTINUE: { + struct replay_opts replay_opts = get_replay_opts(&opts); + + ret = sequencer_continue(the_repository, &replay_opts); break; } case EDIT_TODO: @@ -446,45 +511,6 @@ static int use_builtin_rebase(void) return ret; } -struct rebase_options { - enum rebase_type type; - const char *state_dir; - struct commit *upstream; - const char *upstream_name; - const char *upstream_arg; - char *head_name; - struct object_id orig_head; - struct commit *onto; - const char *onto_name; - const char *revisions; - const char *switch_to; - int root; - struct object_id *squash_onto; - struct commit *restrict_revision; - int dont_finish_rebase; - enum { - REBASE_NO_QUIET = 1<<0, - REBASE_VERBOSE = 1<<1, - REBASE_DIFFSTAT = 1<<2, - REBASE_FORCE = 1<<3, - REBASE_INTERACTIVE_EXPLICIT = 1<<4, - } flags; - struct argv_array git_am_opts; - const char *action; - int signoff; - int allow_rerere_autoupdate; - int keep_empty; - int autosquash; - char *gpg_sign_opt; - int autostash; - char *cmd; - int allow_empty_message; - int rebase_merges, rebase_cousins; - char *strategy, *strategy_opts; - struct strbuf git_format_patch_opt; - int reschedule_failed_exec; -}; - static int is_interactive(struct rebase_options *opts) { return opts->type == REBASE_INTERACTIVE || @@ -1380,13 +1406,7 @@ static int check_exec_cmd(const char *cmd) int cmd_rebase(int argc, const char **argv, const char *prefix) { - struct rebase_options options = { - .type = REBASE_UNSPECIFIED, - .flags = REBASE_NO_QUIET, - .git_am_opts = ARGV_ARRAY_INIT, - .allow_empty_message = 1, - .git_format_patch_opt = STRBUF_INIT, - }; + struct rebase_options options = REBASE_OPTIONS_INIT; const char *branch_name; int ret, flags, total_argc, in_progress = 0; int ok_to_skip_pre_rebase = 0; @@ -1539,6 +1559,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) trace_repo_setup(prefix); setup_work_tree(); + options.allow_empty_message = 1; git_config(rebase_config, &options); strbuf_reset(&buf); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 10/12] rebase -i: use struct rebase_options in do_interactive_rebase() 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (8 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 09/12] rebase -i: use struct rebase_options to parse args Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 11/12] rebase: use a common action enum Phillip Wood 2019-04-17 14:30 ` [PATCH v1 12/12] rebase -i: run without forking rebase--interactive Phillip Wood 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> All the parameters that are passed to do_interactive_rebase() apart from `flags` are already in `struct rebase_options` so there is no need to pass them separately. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 69 +++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index aa97d81ab8..277dbaadf4 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -113,6 +113,8 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) replay.reschedule_failed_exec = opts->reschedule_failed_exec; replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); replay.strategy = opts->strategy; + if (opts->strategy_opts) + parse_strategy_opts(&replay, opts->strategy_opts); return replay; } @@ -262,44 +264,50 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name, return write_basic_state(opts, head_name, onto, orig_head); } -static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, - const char *switch_to, struct commit *upstream, - struct commit *onto, const char *onto_name, - struct object_id *squash_onto, const char *head_name, - struct commit *restrict_revision, char *raw_strategies, - struct string_list *commands, unsigned autosquash) +static void split_exec_commands(const char *cmd, struct string_list *commands) +{ + if (cmd && *cmd) { + string_list_split(commands, cmd, '\n', -1); + + /* rebase.c adds a new line to cmd after every command, + * so here the last command is always empty */ + string_list_remove_empty_items(commands, 0); + } +} + +static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) { int ret; const char *head_hash = NULL; char *revisions = NULL, *shortrevisions = NULL; struct argv_array make_script_args = ARGV_ARRAY_INIT; struct todo_list todo_list = TODO_LIST_INIT; + struct replay_opts replay = get_replay_opts(opts); + struct string_list commands = STRING_LIST_INIT_DUP; - if (prepare_branch_to_be_rebased(the_repository, opts, switch_to)) + if (prepare_branch_to_be_rebased(the_repository, &replay, + opts->switch_to)) return -1; - if (get_revision_ranges(upstream, onto, &head_hash, + if (get_revision_ranges(opts->upstream, opts->onto, &head_hash, &revisions, &shortrevisions)) return -1; - if (raw_strategies) - parse_strategy_opts(opts, raw_strategies); - - if (init_basic_state(opts, head_name, onto, head_hash)) { + if (init_basic_state(&replay, opts->head_name, opts->onto, head_hash)) { free(revisions); free(shortrevisions); return -1; } - if (!upstream && squash_onto) + if (!opts->upstream && opts->squash_onto) write_file(path_squash_onto(), "%s\n", - oid_to_hex(squash_onto)); + oid_to_hex(opts->squash_onto)); argv_array_pushl(&make_script_args, "", revisions, NULL); - if (restrict_revision) + if (opts->restrict_revision) argv_array_push(&make_script_args, - oid_to_hex(&restrict_revision->object.oid)); + oid_to_hex(&opts->restrict_revision->object.oid)); ret = sequencer_make_script(the_repository, &todo_list.buf, make_script_args.argc, make_script_args.argv, @@ -313,10 +321,13 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags, &todo_list)) BUG("unusable todo list"); - ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name, - onto, head_hash, commands, autosquash, &todo_list); + split_exec_commands(opts->cmd, &commands); + ret = complete_action(the_repository, &replay, flags, + shortrevisions, opts->onto_name, opts->onto, head_hash, + &commands, opts->autosquash, &todo_list); } + string_list_clear(&commands, 0); free(revisions); free(shortrevisions); todo_list_release(&todo_list); @@ -336,7 +347,6 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) unsigned flags = 0; int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; - struct string_list commands = STRING_LIST_INIT_DUP; enum { NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC @@ -424,23 +434,12 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - if (opts.cmd && *opts.cmd) { - string_list_split(&commands, opts.cmd, '\n', -1); - - /* rebase.c adds a new line to cmd after every command, - * so here the last command is always empty */ - string_list_remove_empty_items(&commands, 0); - } - switch (command) { case NONE: { - struct replay_opts replay_opts = get_replay_opts(&opts); if (!opts.onto && !opts.upstream) die(_("a base commit must be provided with --upstream or --onto")); - ret = do_interactive_rebase(&replay_opts, flags, opts.switch_to, opts.upstream, opts.onto, - opts.onto_name, opts.squash_onto, opts.head_name, opts.restrict_revision, - opts.strategy_opts, &commands, opts.autosquash); + ret = do_interactive_rebase(&opts, flags); break; } case SKIP: { @@ -477,14 +476,18 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) case REARRANGE_SQUASH: ret = rearrange_squash_in_todo_file(); break; - case ADD_EXEC: + case ADD_EXEC: { + struct string_list commands = STRING_LIST_INIT_DUP; + + split_exec_commands(opts.cmd, &commands); ret = add_exec_commands(&commands); + string_list_clear(&commands, 0); break; + } default: BUG("invalid command '%d'", command); } - string_list_clear(&commands, 0); return !!ret; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 11/12] rebase: use a common action enum 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (9 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 10/12] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 12/12] rebase -i: run without forking rebase--interactive Phillip Wood 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> cmd_rebase() and cmd_rebase__interactive() used different enums to hold the current action. Change to using a common enum so the values are the same when we change `rebase -i` to avoid forking `rebase--interactive`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 91 +++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 277dbaadf4..0020e1ecf6 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -119,6 +119,29 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) return replay; } +enum action { + ACTION_NONE = 0, + ACTION_CONTINUE, + ACTION_SKIP, + ACTION_ABORT, + ACTION_QUIT, + ACTION_EDIT_TODO, + ACTION_SHOW_CURRENT_PATCH, + ACTION_SHORTEN_OIDS, + ACTION_EXPAND_OIDS, + ACTION_CHECK_TODO_LIST, + ACTION_REARRANGE_SQUASH, + ACTION_ADD_EXEC +}; + +static const char *action_names[] = { "undefined", + "continue", + "skip", + "abort", + "quit", + "edit_todo", + "show_current_patch" }; + static int add_exec_commands(struct string_list *commands) { const char *todo_file = rebase_path_todo(); @@ -347,10 +370,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) unsigned flags = 0; int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; - enum { - NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH, - SHORTEN_OIDS, EXPAND_OIDS, CHECK_TODO_LIST, REARRANGE_SQUASH, ADD_EXEC - } command = 0; + enum action command = ACTION_NONE; struct option options[] = { OPT_NEGBIT(0, "ff", &opts.flags, N_("allow fast-forward"), REBASE_FORCE), @@ -367,22 +387,22 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) N_("display a diffstat of what changed upstream"), REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), - CONTINUE), - OPT_CMDMODE(0, "skip", &command, N_("skip commit"), SKIP), + ACTION_CONTINUE), + OPT_CMDMODE(0, "skip", &command, N_("skip commit"), ACTION_SKIP), OPT_CMDMODE(0, "edit-todo", &command, N_("edit the todo list"), - EDIT_TODO), + ACTION_EDIT_TODO), OPT_CMDMODE(0, "show-current-patch", &command, N_("show the current patch"), - SHOW_CURRENT_PATCH), + ACTION_SHOW_CURRENT_PATCH), OPT_CMDMODE(0, "shorten-ids", &command, - N_("shorten commit ids in the todo list"), SHORTEN_OIDS), + N_("shorten commit ids in the todo list"), ACTION_SHORTEN_OIDS), OPT_CMDMODE(0, "expand-ids", &command, - N_("expand commit ids in the todo list"), EXPAND_OIDS), + N_("expand commit ids in the todo list"), ACTION_EXPAND_OIDS), OPT_CMDMODE(0, "check-todo-list", &command, - N_("check the todo list"), CHECK_TODO_LIST), + N_("check the todo list"), ACTION_CHECK_TODO_LIST), OPT_CMDMODE(0, "rearrange-squash", &command, - N_("rearrange fixup/squash lines"), REARRANGE_SQUASH), + N_("rearrange fixup/squash lines"), ACTION_REARRANGE_SQUASH), OPT_CMDMODE(0, "add-exec-commands", &command, - N_("insert exec commands in todo list"), ADD_EXEC), + N_("insert exec commands in todo list"), ACTION_ADD_EXEC), { OPTION_CALLBACK, 0, "onto", &opts.onto, N_("onto"), N_("onto"), PARSE_OPT_NONEG, parse_opt_commit, 0 }, { OPTION_CALLBACK, 0, "restrict-revision", &opts.restrict_revision, @@ -428,36 +448,36 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; - flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; + flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; if (opts.rebase_cousins >= 0 && !opts.rebase_merges) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); switch (command) { - case NONE: { + case ACTION_NONE: { if (!opts.onto && !opts.upstream) die(_("a base commit must be provided with --upstream or --onto")); ret = do_interactive_rebase(&opts, flags); break; } - case SKIP: { + case ACTION_SKIP: { struct string_list merge_rr = STRING_LIST_INIT_DUP; rerere_clear(the_repository, &merge_rr); } /* fallthrough */ - case CONTINUE: { + case ACTION_CONTINUE: { struct replay_opts replay_opts = get_replay_opts(&opts); ret = sequencer_continue(the_repository, &replay_opts); break; } - case EDIT_TODO: + case ACTION_EDIT_TODO: ret = edit_todo_file(flags); break; - case SHOW_CURRENT_PATCH: { + case ACTION_SHOW_CURRENT_PATCH: { struct child_process cmd = CHILD_PROCESS_INIT; cmd.git_cmd = 1; @@ -466,17 +486,17 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) break; } - case SHORTEN_OIDS: - case EXPAND_OIDS: + case ACTION_SHORTEN_OIDS: + case ACTION_EXPAND_OIDS: ret = transform_todo_file(flags); break; - case CHECK_TODO_LIST: + case ACTION_CHECK_TODO_LIST: ret = check_todo_list_from_file(the_repository); break; - case REARRANGE_SQUASH: + case ACTION_REARRANGE_SQUASH: ret = rearrange_squash_in_todo_file(); break; - case ADD_EXEC: { + case ACTION_ADD_EXEC: { struct string_list commands = STRING_LIST_INIT_DUP; split_exec_commands(opts.cmd, &commands); @@ -1417,22 +1437,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) struct strbuf revisions = STRBUF_INIT; struct strbuf buf = STRBUF_INIT; struct object_id merge_base; - enum { - NO_ACTION, - ACTION_CONTINUE, - ACTION_SKIP, - ACTION_ABORT, - ACTION_QUIT, - ACTION_EDIT_TODO, - ACTION_SHOW_CURRENT_PATCH, - } action = NO_ACTION; - static const char *action_names[] = { "undefined", - "continue", - "skip", - "abort", - "quit", - "edit_todo", - "show_current_patch" }; + enum action action = ACTION_NONE; const char *gpg_sign = NULL; struct string_list exec = STRING_LIST_INIT_NODUP; const char *rebase_merges = NULL; @@ -1599,7 +1604,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) builtin_rebase_options, builtin_rebase_usage, 0); - if (action != NO_ACTION && total_argc != 2) { + if (action != ACTION_NONE && total_argc != 2) { usage_with_options(builtin_rebase_usage, builtin_rebase_options); } @@ -1608,7 +1613,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) usage_with_options(builtin_rebase_usage, builtin_rebase_options); - if (action != NO_ACTION && !in_progress) + if (action != ACTION_NONE && !in_progress) die(_("No rebase in progress?")); setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0); @@ -1708,7 +1713,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.action = "show-current-patch"; options.dont_finish_rebase = 1; goto run_rebase; - case NO_ACTION: + case ACTION_NONE: break; default: BUG("action: %d", action); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v1 12/12] rebase -i: run without forking rebase--interactive 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood ` (10 preceding siblings ...) 2019-04-17 14:30 ` [PATCH v1 11/12] rebase: use a common action enum Phillip Wood @ 2019-04-17 14:30 ` Phillip Wood 11 siblings, 0 replies; 40+ messages in thread From: Phillip Wood @ 2019-04-17 14:30 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Junio C Hamano, Elijah Newren, Duy Nguyen, Alban Gruin, Josh Steadmon, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> When the builtin rebase starts an interactive rebase it parses the options and then repackages them and forks `rebase--interactive`. Separate the option parsing in cmd_rebase__interactive() from the business logic to allow interactive rebases can be run without forking `rebase__interactive` by calling run_rebase_interactive() directly. Starting interactive rebases without forking makes it easy to debug the sequencer without worrying about attaching to child processes. Ævar has also reported that some of the rebase perf tests are 30% faster [1]. This patch also makes it easy to remove cmd_rebase__interactive() in the future when git-legacy-rebase.sh and git-rebase--preserve-merges.sh are retired. [1] https://public-inbox.org/git/87y359cfjj.fsf@evledraar.gmail.com/ Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- builtin/rebase.c | 221 ++++++++++++++++++----------------------------- 1 file changed, 86 insertions(+), 135 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 0020e1ecf6..cd233c6118 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -316,7 +316,9 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) &revisions, &shortrevisions)) return -1; - if (init_basic_state(&replay, opts->head_name, opts->onto, head_hash)) { + if (init_basic_state(&replay, + opts->head_name ? opts->head_name : "detached HEAD", + opts->onto, head_hash)) { free(revisions); free(shortrevisions); @@ -359,6 +361,77 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) return ret; } +static int run_rebase_interactive(struct rebase_options *opts, + enum action command) +{ + unsigned flags = 0; + int abbreviate_commands = 0, ret = 0; + + git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); + + flags |= opts->keep_empty ? TODO_LIST_KEEP_EMPTY : 0; + flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; + flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0; + flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; + + switch (command) { + case ACTION_NONE: { + if (!opts->onto && !opts->upstream) + die(_("a base commit must be provided with --upstream or --onto")); + + ret = do_interactive_rebase(opts, flags); + break; + } + case ACTION_SKIP: { + struct string_list merge_rr = STRING_LIST_INIT_DUP; + + rerere_clear(the_repository, &merge_rr); + } + /* fallthrough */ + case ACTION_CONTINUE: { + struct replay_opts replay_opts = get_replay_opts(opts); + + ret = sequencer_continue(the_repository, &replay_opts); + break; + } + case ACTION_EDIT_TODO: + ret = edit_todo_file(flags); + break; + case ACTION_SHOW_CURRENT_PATCH: { + struct child_process cmd = CHILD_PROCESS_INIT; + + cmd.git_cmd = 1; + argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); + ret = run_command(&cmd); + + break; + } + case ACTION_SHORTEN_OIDS: + case ACTION_EXPAND_OIDS: + ret = transform_todo_file(flags); + break; + case ACTION_CHECK_TODO_LIST: + ret = check_todo_list_from_file(the_repository); + break; + case ACTION_REARRANGE_SQUASH: + ret = rearrange_squash_in_todo_file(); + break; + case ACTION_ADD_EXEC: { + struct string_list commands = STRING_LIST_INIT_DUP; + + split_exec_commands(opts->cmd, &commands); + ret = add_exec_commands(&commands); + string_list_clear(&commands, 0); + break; + } + default: + BUG("invalid command '%d'", command); + } + + return ret; +} + static const char * const builtin_rebase_interactive_usage[] = { N_("git rebase--interactive [<options>]"), NULL @@ -367,8 +440,6 @@ static const char * const builtin_rebase_interactive_usage[] = { int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) { struct rebase_options opts = REBASE_OPTIONS_INIT; - unsigned flags = 0; - int abbreviate_commands = 0, ret = 0; struct object_id squash_onto = null_oid; enum action command = ACTION_NONE; struct option options[] = { @@ -433,8 +504,6 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) opts.rebase_cousins = -1; - git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands); - if (argc == 1) usage_with_options(builtin_rebase_interactive_usage, options); @@ -444,71 +513,11 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) if (!is_null_oid(&squash_onto)) opts.squash_onto = &squash_onto; - flags |= opts.keep_empty ? TODO_LIST_KEEP_EMPTY : 0; - flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; - flags |= opts.rebase_merges ? TODO_LIST_REBASE_MERGES : 0; - flags |= opts.rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; - flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; - if (opts.rebase_cousins >= 0 && !opts.rebase_merges) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - switch (command) { - case ACTION_NONE: { - if (!opts.onto && !opts.upstream) - die(_("a base commit must be provided with --upstream or --onto")); - - ret = do_interactive_rebase(&opts, flags); - break; - } - case ACTION_SKIP: { - struct string_list merge_rr = STRING_LIST_INIT_DUP; - - rerere_clear(the_repository, &merge_rr); - } - /* fallthrough */ - case ACTION_CONTINUE: { - struct replay_opts replay_opts = get_replay_opts(&opts); - - ret = sequencer_continue(the_repository, &replay_opts); - break; - } - case ACTION_EDIT_TODO: - ret = edit_todo_file(flags); - break; - case ACTION_SHOW_CURRENT_PATCH: { - struct child_process cmd = CHILD_PROCESS_INIT; - - cmd.git_cmd = 1; - argv_array_pushl(&cmd.args, "show", "REBASE_HEAD", "--", NULL); - ret = run_command(&cmd); - - break; - } - case ACTION_SHORTEN_OIDS: - case ACTION_EXPAND_OIDS: - ret = transform_todo_file(flags); - break; - case ACTION_CHECK_TODO_LIST: - ret = check_todo_list_from_file(the_repository); - break; - case ACTION_REARRANGE_SQUASH: - ret = rearrange_squash_in_todo_file(); - break; - case ACTION_ADD_EXEC: { - struct string_list commands = STRING_LIST_INIT_DUP; - - split_exec_commands(opts.cmd, &commands); - ret = add_exec_commands(&commands); - string_list_clear(&commands, 0); - break; - } - default: - BUG("invalid command '%d'", command); - } - - return !!ret; + return !!run_rebase_interactive(&opts, command); } static int use_builtin_rebase(void) @@ -1071,7 +1080,7 @@ static int run_am(struct rebase_options *opts) return status; } -static int run_specific_rebase(struct rebase_options *opts) +static int run_specific_rebase(struct rebase_options *opts, enum action action) { const char *argv[] = { NULL, NULL }; struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT; @@ -1080,77 +1089,19 @@ static int run_specific_rebase(struct rebase_options *opts) if (opts->type == REBASE_INTERACTIVE) { /* Run builtin interactive rebase */ - struct child_process child = CHILD_PROCESS_INIT; - - argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s", - resolvemsg); + setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1); if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) { - argv_array_push(&child.env_array, - "GIT_SEQUENCE_EDITOR=:"); + setenv("GIT_SEQUENCE_EDITOR", ":", 1); opts->autosquash = 0; } + if (opts->gpg_sign_opt) { + /* remove the leading "-S" */ + char *tmp = xstrdup(opts->gpg_sign_opt + 2); + free(opts->gpg_sign_opt); + opts->gpg_sign_opt = tmp; + } - child.git_cmd = 1; - argv_array_push(&child.args, "rebase--interactive"); - - if (opts->action) - argv_array_pushf(&child.args, "--%s", opts->action); - if (opts->keep_empty) - argv_array_push(&child.args, "--keep-empty"); - if (opts->rebase_merges) - argv_array_push(&child.args, "--rebase-merges"); - if (opts->rebase_cousins) - argv_array_push(&child.args, "--rebase-cousins"); - if (opts->autosquash) - argv_array_push(&child.args, "--autosquash"); - if (opts->flags & REBASE_VERBOSE) - argv_array_push(&child.args, "--verbose"); - if (opts->flags & REBASE_FORCE) - argv_array_push(&child.args, "--no-ff"); - if (opts->restrict_revision) - argv_array_pushf(&child.args, - "--restrict-revision=^%s", - oid_to_hex(&opts->restrict_revision->object.oid)); - if (opts->upstream) - argv_array_pushf(&child.args, "--upstream=%s", - oid_to_hex(&opts->upstream->object.oid)); - if (opts->onto) - argv_array_pushf(&child.args, "--onto=%s", - oid_to_hex(&opts->onto->object.oid)); - if (opts->squash_onto) - argv_array_pushf(&child.args, "--squash-onto=%s", - oid_to_hex(opts->squash_onto)); - if (opts->onto_name) - argv_array_pushf(&child.args, "--onto-name=%s", - opts->onto_name); - argv_array_pushf(&child.args, "--head-name=%s", - opts->head_name ? - opts->head_name : "detached HEAD"); - if (opts->strategy) - argv_array_pushf(&child.args, "--strategy=%s", - opts->strategy); - if (opts->strategy_opts) - argv_array_pushf(&child.args, "--strategy-opts=%s", - opts->strategy_opts); - if (opts->switch_to) - argv_array_pushf(&child.args, "--switch-to=%s", - opts->switch_to); - if (opts->cmd) - argv_array_pushf(&child.args, "--cmd=%s", opts->cmd); - if (opts->allow_empty_message) - argv_array_push(&child.args, "--allow-empty-message"); - if (opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE) - argv_array_push(&child.args, "--rerere-autoupdate"); - else if (opts->allow_rerere_autoupdate == RERERE_NOAUTOUPDATE) - argv_array_push(&child.args, "--no-rerere-autoupdate"); - if (opts->gpg_sign_opt) - argv_array_push(&child.args, opts->gpg_sign_opt); - if (opts->signoff) - argv_array_push(&child.args, "--signoff"); - if (opts->reschedule_failed_exec) - argv_array_push(&child.args, "--reschedule-failed-exec"); - - status = run_command(&child); + status = run_rebase_interactive(opts, action); goto finished_rebase; } @@ -2211,7 +2162,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.revisions = revisions.buf; run_rebase: - ret = !!run_specific_rebase(&options); + ret = !!run_specific_rebase(&options, action); cleanup: strbuf_release(&revisions); -- 2.21.0 ^ permalink raw reply related [flat|nested] 40+ messages in thread
end of thread, other threads:[~2019-04-25 17:47 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-19 19:03 [RFC PATCH 00/11] rebase -i run without forking rebase--interactive Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 01/11] sequencer: always discard index after checkout Phillip Wood 2019-03-20 1:50 ` Duy Nguyen 2019-03-21 14:35 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 02/11] rebase: rename write_basic_state() Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 03/11] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 04/11] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 05/11] rebase -i: remove duplication Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 06/11] rebase -i: use struct commit when parsing options Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 07/11] rebase -i: use struct object_id for squash_onto Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 08/11] rebase -i: use struct rebase_options to parse args Phillip Wood 2019-03-21 4:21 ` Junio C Hamano 2019-03-21 14:59 ` Phillip Wood 2019-03-22 3:34 ` Junio C Hamano 2019-03-21 21:13 ` Alban Gruin 2019-04-10 19:16 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 09/11] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 10/11] rebase: use a common action enum Phillip Wood 2019-03-19 20:24 ` Ævar Arnfjörð Bjarmason 2019-03-21 14:43 ` Phillip Wood 2019-03-19 19:03 ` [RFC PATCH 11/11] rebase -i: run without forking rebase--interactive Phillip Wood 2019-03-20 20:50 ` [RFC PATCH 00/11] rebase -i " Josh Steadmon 2019-03-20 23:05 ` Ævar Arnfjörð Bjarmason 2019-03-21 14:40 ` Phillip Wood 2019-03-21 1:44 ` Junio C Hamano 2019-04-17 14:30 ` [PATCH v1 00/12] Run rebase -i " Phillip Wood 2019-04-17 14:30 ` [PATCH v1 01/12] sequencer: always discard index after checkout Phillip Wood 2019-04-17 14:30 ` [PATCH v1 02/12] rebase: don't translate trace strings Phillip Wood 2019-04-19 5:53 ` Junio C Hamano 2019-04-25 17:47 ` Phillip Wood 2019-04-17 14:30 ` [PATCH v1 03/12] rebase: rename write_basic_state() Phillip Wood 2019-04-17 14:30 ` [PATCH v1 04/12] rebase: use OPT_RERERE_AUTOUPDATE() Phillip Wood 2019-04-17 14:30 ` [PATCH v1 05/12] rebase -i: combine rebase--interactive.c with rebase.c Phillip Wood 2019-04-17 14:30 ` [PATCH v1 06/12] rebase -i: remove duplication Phillip Wood 2019-04-17 14:30 ` [PATCH v1 07/12] rebase -i: use struct commit when parsing options Phillip Wood 2019-04-17 14:30 ` [PATCH v1 08/12] rebase -i: use struct object_id for squash_onto Phillip Wood 2019-04-17 14:30 ` [PATCH v1 09/12] rebase -i: use struct rebase_options to parse args Phillip Wood 2019-04-17 14:30 ` [PATCH v1 10/12] rebase -i: use struct rebase_options in do_interactive_rebase() Phillip Wood 2019-04-17 14:30 ` [PATCH v1 11/12] rebase: use a common action enum Phillip Wood 2019-04-17 14:30 ` [PATCH v1 12/12] rebase -i: run without forking rebase--interactive Phillip Wood
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).