reset_head(): take struct rebase_head_opts

This function takes a confusingly large number of parameters which
makes it difficult to remember which order to pass them in. The
following commits will add a couple more parameters which makes the
problem worse. To address this change the function to take a struct of
options. Using a struct means that it is no longer necessary to
remember which order to pass the parameters in and anyone reading the
code can easily see which value is passed to each parameter.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Phillip Wood 2022-01-26 13:05:46 +00:00 committed by Junio C Hamano
parent ee464c4e37
commit 6ae8086161
4 changed files with 92 additions and 48 deletions

View File

@ -571,6 +571,7 @@ static int finish_rebase(struct rebase_options *opts)
static int move_to_original_branch(struct rebase_options *opts) static int move_to_original_branch(struct rebase_options *opts)
{ {
struct strbuf orig_head_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT; struct strbuf orig_head_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
struct reset_head_opts ropts = { 0 };
int ret; int ret;
if (!opts->head_name) if (!opts->head_name)
@ -583,9 +584,11 @@ static int move_to_original_branch(struct rebase_options *opts)
opts->head_name, oid_to_hex(&opts->onto->object.oid)); opts->head_name, oid_to_hex(&opts->onto->object.oid));
strbuf_addf(&head_reflog, "rebase finished: returning to %s", strbuf_addf(&head_reflog, "rebase finished: returning to %s",
opts->head_name); opts->head_name);
ret = reset_head(the_repository, NULL, opts->head_name, ropts.branch = opts->head_name;
RESET_HEAD_REFS_ONLY, ropts.flags = RESET_HEAD_REFS_ONLY;
orig_head_reflog.buf, head_reflog.buf, NULL); ropts.orig_head_msg = orig_head_reflog.buf;
ropts.head_msg = head_reflog.buf;
ret = reset_head(the_repository, &ropts);
strbuf_release(&orig_head_reflog); strbuf_release(&orig_head_reflog);
strbuf_release(&head_reflog); strbuf_release(&head_reflog);
@ -669,13 +672,15 @@ static int run_am(struct rebase_options *opts)
status = run_command(&format_patch); status = run_command(&format_patch);
if (status) { if (status) {
struct reset_head_opts ropts = { 0 };
unlink(rebased_patches); unlink(rebased_patches);
free(rebased_patches); free(rebased_patches);
strvec_clear(&am.args); strvec_clear(&am.args);
reset_head(the_repository, &opts->orig_head, ropts.oid = &opts->orig_head;
opts->head_name, 0, ropts.branch = opts->head_name;
NULL, NULL, DEFAULT_REFLOG_ACTION); ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
reset_head(the_repository, &ropts);
error(_("\ngit encountered an error while preparing the " error(_("\ngit encountered an error while preparing the "
"patches to replay\n" "patches to replay\n"
"these revisions:\n" "these revisions:\n"
@ -814,14 +819,17 @@ static int rebase_config(const char *var, const char *value, void *data)
static int checkout_up_to_date(struct rebase_options *options) static int checkout_up_to_date(struct rebase_options *options)
{ {
struct strbuf buf = STRBUF_INIT; struct strbuf buf = STRBUF_INIT;
struct reset_head_opts ropts = { 0 };
int ret = 0; int ret = 0;
strbuf_addf(&buf, "%s: checkout %s", strbuf_addf(&buf, "%s: checkout %s",
getenv(GIT_REFLOG_ACTION_ENVIRONMENT), getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
options->switch_to); options->switch_to);
if (reset_head(the_repository, &options->orig_head, ropts.oid = &options->orig_head;
options->head_name, RESET_HEAD_RUN_POST_CHECKOUT_HOOK, ropts.branch = options->head_name;
NULL, buf.buf, NULL) < 0) ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = buf.buf;
if (reset_head(the_repository, &ropts) < 0)
ret = error(_("could not switch to %s"), options->switch_to); ret = error(_("could not switch to %s"), options->switch_to);
strbuf_release(&buf); strbuf_release(&buf);
@ -1033,6 +1041,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
int reschedule_failed_exec = -1; int reschedule_failed_exec = -1;
int allow_preemptive_ff = 1; int allow_preemptive_ff = 1;
int preserve_merges_selected = 0; int preserve_merges_selected = 0;
struct reset_head_opts ropts = { 0 };
struct option builtin_rebase_options[] = { struct option builtin_rebase_options[] = {
OPT_STRING(0, "onto", &options.onto_name, OPT_STRING(0, "onto", &options.onto_name,
N_("revision"), N_("revision"),
@ -1270,9 +1279,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
rerere_clear(the_repository, &merge_rr); rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1); string_list_clear(&merge_rr, 1);
ropts.flags = RESET_HEAD_HARD;
if (reset_head(the_repository, NULL, NULL, RESET_HEAD_HARD, if (reset_head(the_repository, &ropts) < 0)
NULL, NULL, NULL) < 0)
die(_("could not discard worktree changes")); die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0); remove_branch_state(the_repository, 0);
if (read_basic_state(&options)) if (read_basic_state(&options))
@ -1289,9 +1297,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (read_basic_state(&options)) if (read_basic_state(&options))
exit(1); exit(1);
if (reset_head(the_repository, &options.orig_head, ropts.oid = &options.orig_head;
options.head_name, RESET_HEAD_HARD, ropts.branch = options.head_name;
NULL, NULL, DEFAULT_REFLOG_ACTION) < 0) ropts.flags = RESET_HEAD_HARD;
ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
if (reset_head(the_repository, &ropts) < 0)
die(_("could not move back to %s"), die(_("could not move back to %s"),
oid_to_hex(&options.orig_head)); oid_to_hex(&options.orig_head));
remove_branch_state(the_repository, 0); remove_branch_state(the_repository, 0);
@ -1758,10 +1768,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
strbuf_addf(&msg, "%s: checkout %s", strbuf_addf(&msg, "%s: checkout %s",
getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name); getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
if (reset_head(the_repository, &options.onto->object.oid, NULL, ropts.oid = &options.onto->object.oid;
RESET_HEAD_DETACH | RESET_ORIG_HEAD | ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
RESET_HEAD_RUN_POST_CHECKOUT_HOOK, RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
NULL, msg.buf, DEFAULT_REFLOG_ACTION)) ropts.head_msg = msg.buf;
ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
if (reset_head(the_repository, &ropts))
die(_("Could not detach HEAD")); die(_("Could not detach HEAD"));
strbuf_release(&msg); strbuf_release(&msg);
@ -1776,8 +1788,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
strbuf_addf(&msg, "rebase finished: %s onto %s", strbuf_addf(&msg, "rebase finished: %s onto %s",
options.head_name ? options.head_name : "detached HEAD", options.head_name ? options.head_name : "detached HEAD",
oid_to_hex(&options.onto->object.oid)); oid_to_hex(&options.onto->object.oid));
reset_head(the_repository, NULL, options.head_name, memset(&ropts, 0, sizeof(ropts));
RESET_HEAD_REFS_ONLY, NULL, msg.buf, NULL); ropts.branch = options.head_name;
ropts.flags = RESET_HEAD_REFS_ONLY;
ropts.head_msg = msg.buf;
reset_head(the_repository, &ropts);
strbuf_release(&msg); strbuf_release(&msg);
ret = finish_rebase(&options); ret = finish_rebase(&options);
goto cleanup; goto cleanup;

38
reset.c
View File

@ -8,14 +8,17 @@
#include "tree.h" #include "tree.h"
#include "unpack-trees.h" #include "unpack-trees.h"
static int update_refs(const struct object_id *oid, const char *switch_to_branch, static int update_refs(const struct reset_head_opts *opts,
const struct object_id *head, const char *reflog_head, const struct object_id *oid,
const char *reflog_orig_head, const struct object_id *head)
const char *default_reflog_action, unsigned flags)
{ {
unsigned detach_head = flags & RESET_HEAD_DETACH; unsigned detach_head = opts->flags & RESET_HEAD_DETACH;
unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK; unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
unsigned update_orig_head = flags & RESET_ORIG_HEAD; unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
const char *switch_to_branch = opts->branch;
const char *reflog_head = opts->head_msg;
const char *reflog_orig_head = opts->orig_head_msg;
const char *default_reflog_action = opts->default_reflog_action;
struct object_id *old_orig = NULL, oid_old_orig; struct object_id *old_orig = NULL, oid_old_orig;
struct strbuf msg = STRBUF_INIT; struct strbuf msg = STRBUF_INIT;
const char *reflog_action; const char *reflog_action;
@ -69,14 +72,13 @@ static int update_refs(const struct object_id *oid, const char *switch_to_branch
return ret; return ret;
} }
int reset_head(struct repository *r, struct object_id *oid, int reset_head(struct repository *r, const struct reset_head_opts *opts)
const char *switch_to_branch, unsigned flags,
const char *reflog_orig_head, const char *reflog_head,
const char *default_reflog_action)
{ {
unsigned reset_hard = flags & RESET_HEAD_HARD; const struct object_id *oid = opts->oid;
unsigned refs_only = flags & RESET_HEAD_REFS_ONLY; const char *switch_to_branch = opts->branch;
unsigned update_orig_head = flags & RESET_ORIG_HEAD; unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
struct object_id *head = NULL, head_oid; struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } }; struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT; struct lock_file lock = LOCK_INIT;
@ -104,9 +106,7 @@ int reset_head(struct repository *r, struct object_id *oid,
oid = &head_oid; oid = &head_oid;
if (refs_only) if (refs_only)
return update_refs(oid, switch_to_branch, head, reflog_head, return update_refs(opts, oid, head);
reflog_orig_head, default_reflog_action,
flags);
action = reset_hard ? "reset" : "checkout"; action = reset_hard ? "reset" : "checkout";
setup_unpack_trees_porcelain(&unpack_tree_opts, action); setup_unpack_trees_porcelain(&unpack_tree_opts, action);
@ -151,9 +151,7 @@ int reset_head(struct repository *r, struct object_id *oid,
} }
if (oid != &head_oid || update_orig_head || switch_to_branch) if (oid != &head_oid || update_orig_head || switch_to_branch)
ret = update_refs(oid, switch_to_branch, head, reflog_head, ret = update_refs(opts, oid, head);
reflog_orig_head, default_reflog_action,
flags);
leave_reset_head: leave_reset_head:
rollback_lock_file(&lock); rollback_lock_file(&lock);

40
reset.h
View File

@ -6,15 +6,47 @@
#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION" #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
/* Request a detached checkout */
#define RESET_HEAD_DETACH (1<<0) #define RESET_HEAD_DETACH (1<<0)
/* Request a reset rather than a checkout */
#define RESET_HEAD_HARD (1<<1) #define RESET_HEAD_HARD (1<<1)
/* Run the post-checkout hook */
#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2) #define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
/* Only update refs, do not touch the worktree */
#define RESET_HEAD_REFS_ONLY (1<<3) #define RESET_HEAD_REFS_ONLY (1<<3)
/* Update ORIG_HEAD as well as HEAD */
#define RESET_ORIG_HEAD (1<<4) #define RESET_ORIG_HEAD (1<<4)
int reset_head(struct repository *r, struct object_id *oid, struct reset_head_opts {
const char *switch_to_branch, unsigned flags, /*
const char *reflog_orig_head, const char *reflog_head, * The commit to checkout/reset to. Defaults to HEAD.
const char *default_reflog_action); */
const struct object_id *oid;
/*
* Optional branch to switch to.
*/
const char *branch;
/*
* Flags defined above.
*/
unsigned flags;
/*
* Optional reflog message for HEAD, if this omitted but oid or branch
* are given then default_reflog_action must be given.
*/
const char *head_msg;
/*
* Optional reflog message for ORIG_HEAD, if this omitted and flags
* contains RESET_ORIG_HEAD then default_reflog_action must be given.
*/
const char *orig_head_msg;
/*
* Action to use in default reflog messages, only required if a ref is
* being updated and the reflog messages above are omitted.
*/
const char *default_reflog_action;
};
int reset_head(struct repository *r, const struct reset_head_opts *opts);
#endif #endif

View File

@ -4115,6 +4115,7 @@ void create_autostash(struct repository *r, const char *path)
if (has_unstaged_changes(r, 1) || if (has_unstaged_changes(r, 1) ||
has_uncommitted_changes(r, 1)) { has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT; struct child_process stash = CHILD_PROCESS_INIT;
struct reset_head_opts ropts = { .flags = RESET_HEAD_HARD };
struct object_id oid; struct object_id oid;
strvec_pushl(&stash.args, strvec_pushl(&stash.args,
@ -4136,10 +4137,8 @@ void create_autostash(struct repository *r, const char *path)
path); path);
write_file(path, "%s", oid_to_hex(&oid)); write_file(path, "%s", oid_to_hex(&oid));
printf(_("Created autostash: %s\n"), buf.buf); printf(_("Created autostash: %s\n"), buf.buf);
if (reset_head(r, NULL, NULL, RESET_HEAD_HARD, NULL, NULL, if (reset_head(r, &ropts) < 0)
NULL) < 0)
die(_("could not reset --hard")); die(_("could not reset --hard"));
if (discard_index(r->index) < 0 || if (discard_index(r->index) < 0 ||
repo_read_index(r) < 0) repo_read_index(r) < 0)
die(_("could not read index")); die(_("could not read index"));