merge: learn --autostash
Subsystems (2)
PATCH v4 17/23 sequencer: extract perform_autostash() from rebase
In reply to Denton LiuLib-ify the autostash code by extracting perform_autostash() from rebase into sequencer. In a future commit, this will be used to implement --autostash in other builtins.
This patch is best viewed with --color-moved.
--- builtin/rebase.c | 49 ----------------------------------------------- sequencer.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ sequencer.h | 2 ++ 3 files changed, 52 insertions(+), 49 deletions(-)
builtin/rebase.c +0 -49
@@ -1269,55 +1269,6 @@ static int check_exec_cmd(const char *cmd)return 0 ; } -static void create_autostash (struct repository *r,const char *path, -const char *default_reflog_action) -{ -struct strbuf buf =STRBUF_INIT ; -struct lock_file lock_file =LOCK_INIT ; -int fd; - - fd =repo_hold_locked_index (r, &lock_file,0 ); -refresh_index (r->index,REFRESH_QUIET ,NULL ,NULL ,NULL ); -if (0 <= fd) -repo_update_index_if_able (r, &lock_file); -rollback_lock_file (&lock_file); - -if (has_unstaged_changes (r,1 ) || -has_uncommitted_changes (r,1 )) { -struct child_process stash =CHILD_PROCESS_INIT ; -struct object_id oid; - -argv_array_pushl (&stash.args, -"stash" ,"create" ,"autostash" ,NULL ); - stash.git_cmd =1 ; - stash.no_stdin =1 ; -strbuf_reset (&buf); -if (capture_command (&stash, &buf,GIT_MAX_HEXSZ )) -die (_ ("Cannot autostash" )); -strbuf_trim_trailing_newline (&buf); -if (get_oid (buf.buf, &oid)) -die (_ ("Unexpected stash response: '%s'" ), - buf.buf); -strbuf_reset (&buf); -strbuf_add_unique_abbrev (&buf, &oid,DEFAULT_ABBREV ); - -if (safe_create_leading_directories_const (path)) -die (_ ("Could not create directory for '%s'" ), - path); -write_file (path,"%s" ,oid_to_hex (&oid)); -printf (_ ("Created autostash: %s\n" ), buf.buf); -if (reset_head (r,NULL ,"reset --hard" , -NULL ,RESET_HEAD_HARD ,NULL ,NULL , - default_reflog_action) <0 ) -die (_ ("could not reset --hard" )); - -if (discard_index (r->index) <0 || -repo_read_index (r) <0 ) -die (_ ("could not read index" )); - } -strbuf_release (&buf); -} -int cmd_rebase (int argc,const char **argv,const char *prefix) {struct rebase_options options =REBASE_OPTIONS_INIT ;
sequencer.c +50 -0
62 lines not on this page · raw message
sequencer.h +2 -0
@@ -191,6 +191,8 @@ void commit_post_rewrite(struct repository *r,const struct commit *current_head, const struct object_id *new_head); +void create_autostash (struct repository *r,const char *path, +const char *default_reflog_action);int apply_autostash (const char *path);#define SUMMARY_INITIAL_COMMIT (1 << 0)
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v4 18/23 sequencer: unlink autostash in apply_autostash()
In reply to Denton LiuExplicitly remove autostash file in apply_autostash() once it has been applied successfully.
This is currently a no-op because the only users of this function will unlink the state (including the autostash file) after this function runs. However, in the future, we will introduce a user of the function that does not explicitly remove the state so we do it here.
--- sequencer.c | 1 + 1 file changed, 1 insertion(+)
sequencer.c +1 -0
@@ -3740,6 +3740,7 @@ int apply_autostash(const char *path) " \"git stash drop\" at any time.\n")); } +unlink (path);strbuf_release (&stash_oid);return ret; }
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v4 19/23 sequencer: implement save_autostash()
In reply to Denton LiuExtract common functionality of apply_autostash() into apply_save_autostash() and use it to implement save_autostash(). This function will be used in a future commit.
The difference between save_autostash() and apply_autostash() is that the latter does not try to apply the stash. It skips that step and just stores the created entry in the stash reflog.
--- sequencer.c | 37 +++++++++++++++++++++++++++---------- sequencer.h | 1 + 2 files changed, 28 insertions(+), 10 deletions(-)
sequencer.c +27 -10
@@ -3699,7 +3699,7 @@ void create_autostash(struct repository *r, const char *path,strbuf_release (&buf); } -int apply_autostash (const char *path) +static int apply_save_autostash (const char *path,int attempt_apply) {struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;
@@ -3712,13 +3712,17 @@ int apply_autostash(const char *path) }strbuf_trim (&stash_oid); - child.git_cmd =1 ; - child.no_stdout =1 ; - child.no_stderr =1 ; -argv_array_push (&child.args,"stash" ); -argv_array_push (&child.args,"apply" ); -argv_array_push (&child.args, stash_oid.buf); -if (!run_command (&child)) +if (attempt_apply) { + child.git_cmd =1 ; + child.no_stdout =1 ; + child.no_stderr =1 ; +argv_array_push (&child.args,"stash" ); +argv_array_push (&child.args,"apply" ); +argv_array_push (&child.args, stash_oid.buf); + ret =run_command (&child); + } + +if (attempt_apply && !ret)fprintf (stderr,_ ("Applied autostash.\n" ));else {struct child_process store =CHILD_PROCESS_INIT ;
@@ -3734,10 +3738,13 @@ int apply_autostash(const char *path) ret =error (_ ("cannot store %s" ), stash_oid.buf); else fprintf(stderr, - _("Applying autostash resulted in conflicts.\n" + _("%s\n""Your changes are safe in the stash.\n" "You can run \"git stash pop\" or" - " \"git stash drop\" at any time.\n")); + " \"git stash drop\" at any time.\n"), + attempt_apply ? +_ ("Applying autostash resulted in conflicts." ) : + _("Autostash exists; creating a new stash entry.")); }unlink (path);
@@ -3745,6 +3752,16 @@ int apply_autostash(const char *path)return ret; } +int save_autostash (const char *path) +{ +return apply_save_autostash (path,0 ); +} + +int apply_autostash (const char *path) +{ +return apply_save_autostash (path,1 ); +} +static const char *reflog_message (struct replay_opts *opts,const char *sub_action,const char *fmt, ...) {
sequencer.h +1 -0
@@ -193,6 +193,7 @@ void commit_post_rewrite(struct repository *r,void create_autostash (struct repository *r,const char *path,const char *default_reflog_action); +int save_autostash (const char *path);int apply_autostash (const char *path);#define SUMMARY_INITIAL_COMMIT (1 << 0)
Signature · 1 line
2.26.0.159.g23e2136ad0
Re: [PATCH v4 19/23] sequencer: implement save_autostash()
Hi Denton
Quoted 6 lines · Denton Liu
Extract common functionality of
apply_autostash()intoapply_save_autostash()and use it to implementsave_autostash(). This function will be used in a future commit.The difference between
save_autostash()andapply_autostash()is that the latter does not try to apply the stash.
the latter is apply_autostash() - surely that tries to apply the stash?
Quoted 2 lines
It skips that step and
just stores the created entry in the stash reflog.
I think we want save_autostash() so we can call it to save the stash when running a reset - perhaps we should mention that in the commit message.
Best Wishes
Phillip
Quoted 90 lines
Signed-off-by Denton Liu <[email protected]>--- sequencer.c | 37 +++++++++++++++++++++++++++---------- sequencer.h | 1 + 2 files changed, 28 insertions(+), 10 deletions(-)sequencer.c +27
-10@@ -3699,7 +3699,7 @@ void create_autostash(struct repository *r, const char *path,strbuf_release (&buf); } -int apply_autostash (const char *path) +static int apply_save_autostash (const char *path,int attempt_apply) {struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;@@ -3712,13 +3712,17 @@ int apply_autostash(const char *path) }strbuf_trim (&stash_oid); - child.git_cmd =1 ; - child.no_stdout =1 ; - child.no_stderr =1 ; -argv_array_push (&child.args,"stash" ); -argv_array_push (&child.args,"apply" ); -argv_array_push (&child.args, stash_oid.buf); -if (!run_command (&child)) +if (attempt_apply) { + child.git_cmd =1 ; + child.no_stdout =1 ; + child.no_stderr =1 ; +argv_array_push (&child.args,"stash" ); +argv_array_push (&child.args,"apply" ); +argv_array_push (&child.args, stash_oid.buf); + ret =run_command (&child); + } + +if (attempt_apply && !ret)fprintf (stderr,_ ("Applied autostash.\n" ));else {struct child_process store =CHILD_PROCESS_INIT ;@@ -3734,10 +3738,13 @@ int apply_autostash(const char *path) ret =error (_ ("cannot store %s" ), stash_oid.buf); else fprintf(stderr, - _("Applying autostash resulted in conflicts.\n" + _("%s\n""Your changes are safe in the stash.\n" "You can run \"git stash pop\" or" - " \"git stash drop\" at any time.\n")); + " \"git stash drop\" at any time.\n"), + attempt_apply ? +_ ("Applying autostash resulted in conflicts." ) : + _("Autostash exists; creating a new stash entry.")); }unlink (path);@@ -3745,6 +3752,16 @@ int apply_autostash(const char *path)return ret; } +int save_autostash (const char *path) +{ +return apply_save_autostash (path,0 ); +} + +int apply_autostash (const char *path) +{ +return apply_save_autostash (path,1 ); +} +static const char *reflog_message (struct replay_opts *opts,const char *sub_action,const char *fmt, ...) {sequencer.h +1
-0@@ -193,6 +193,7 @@ void commit_post_rewrite(struct repository *r,void create_autostash (struct repository *r,const char *path,const char *default_reflog_action); +int save_autostash (const char *path);int apply_autostash (const char *path);#define SUMMARY_INITIAL_COMMIT (1 << 0)
PATCH v4 20/23 sequencer: implement apply_autostash_oid()
In reply to Denton LiuSplit apply_save_autostash() into apply_autostash_oid() and apply_save_autostash() where the former operates on an OID string and the latter reads the OID from a file before passing it into apply_save_autostash_oid().
This function is required for a future commmit which will rely on being able to apply an autostash whose OID is stored as a string.
--- sequencer.c | 38 ++++++++++++++++++++++++++------------ sequencer.h | 1 + 2 files changed, 27 insertions(+), 12 deletions(-)
sequencer.c +26 -12
@@ -3699,26 +3699,18 @@ void create_autostash(struct repository *r, const char *path,strbuf_release (&buf); } -static int apply_save_autostash (const char *path,int attempt_apply) +static int apply_save_autostash_oid (const char *stash_oid,int attempt_apply) { -struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;int ret =0 ; -if (!read_oneliner (&stash_oid, path, -READ_ONELINER_SKIP_IF_EMPTY )) { -strbuf_release (&stash_oid); -return 0 ; - } -strbuf_trim (&stash_oid); -if (attempt_apply) { child.git_cmd =1 ; child.no_stdout =1 ; child.no_stderr =1 ;argv_array_push (&child.args,"stash" );argv_array_push (&child.args,"apply" ); -argv_array_push (&child.args, stash_oid.buf); +argv_array_push (&child.args, stash_oid); ret =run_command (&child); }
@@ -3733,9 +3725,9 @@ static int apply_save_autostash(const char *path, int attempt_apply)argv_array_push (&store.args,"-m" );argv_array_push (&store.args,"autostash" );argv_array_push (&store.args,"-q" ); -argv_array_push (&store.args, stash_oid.buf); +argv_array_push (&store.args, stash_oid);if (run_command (&store)) - ret =error (_ ("cannot store %s" ), stash_oid.buf); + ret =error (_ ("cannot store %s" ), stash_oid);else fprintf(stderr, _("%s\n"
@@ -3747,6 +3739,23 @@ static int apply_save_autostash(const char *path, int attempt_apply) _("Autostash exists; creating a new stash entry.")); } +return ret; +} + +static int apply_save_autostash (const char *path,int attempt_apply) +{ +struct strbuf stash_oid =STRBUF_INIT ; +int ret =0 ; + +if (!read_oneliner (&stash_oid, path, +READ_ONELINER_SKIP_IF_EMPTY )) { +strbuf_release (&stash_oid); +return 0 ; + } +strbuf_trim (&stash_oid); + + ret =apply_save_autostash_oid (stash_oid.buf, attempt_apply); +unlink (path);strbuf_release (&stash_oid);return ret;
@@ -3762,6 +3771,11 @@ int apply_autostash(const char *path)return apply_save_autostash (path,1 ); } +int apply_autostash_oid (const char *stash_oid) +{ +return apply_save_autostash_oid (stash_oid,1 ); +} +static const char *reflog_message (struct replay_opts *opts,const char *sub_action,const char *fmt, ...) {
sequencer.h +1 -0
@@ -195,6 +195,7 @@ void create_autostash(struct repository *r, const char *path, const char *default_reflog_action);int save_autostash (const char *path);int apply_autostash (const char *path); +int apply_autostash_oid (const char *stash_oid);#define SUMMARY_INITIAL_COMMIT (1 << 0)#define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v4 21/23 merge: teach --autostash option
In reply to Denton LiuIn rebase, one can pass the --autostash option to cause the worktree to be automatically stashed before continuing with the rebase. This option is missing in merge, however.
Implement the --autostash option and corresponding merge.autoStash option in merge which stashes before merging and then pops after.
This option is useful when a developer has some local changes on a topic branch but they realize that their work depends on another branch. Previously, they had to run something like
git fetch ...
git stash push
git merge FETCH_HEAD
git stash pop
but now, that is reduced to
git fetch ... git merge --autostash FETCH_HEAD
When an autostash is generated, it is automatically reapplied to the worktree only in three explicit situations:
1. An incomplete merge is commit using git commit.
2. A merge completes successfully.
3. A merge is aborted using git merge --abort.
In all other situations where the merge state is removed using remove_merge_branch_state() such as aborting a merge via git reset --hard, the autostash is saved into the stash reflog instead keeping the worktree clean.
--- Documentation/config/merge.txt | 10 +++ Documentation/git-merge.txt | 6 +- Documentation/merge-options.txt | 8 +++ branch.c | 1 + builtin/commit.c | 2 + builtin/merge.c | 27 ++++++++ builtin/rebase.c | 3 +- builtin/reset.c | 4 +- parse-options.h | 1 + path.c | 1 + path.h | 4 +- t/t3033-merge-toplevel.sh | 22 ++++++ t/t7600-merge.sh | 118 ++++++++++++++++++++++++++++++++ 13 files changed, 201 insertions(+), 6 deletions(-)
Documentation/config/merge.txt +10 -0
@@ -70,6 +70,16 @@ merge.stat:: Whether to print the diffstat between ORIG_HEAD and the merge result at the end of the merge. True by default. +merge.autoStash:: + When set to true, automatically create a temporary stash entry + before the operation begins, and apply it after the operation + ends. This means that you can run merge on a dirty worktree. + However, use with care: the final stash application after a + successful merge might result in non-trivial conflicts. + This option can be overridden by the `--no-autostash` and + `--autostash` options of linkgit:git-merge[1]. + Defaults to false. + merge.tool:: Controls which merge tool is used by linkgit:git-mergetool[1]. The list below shows the valid built-in values.
Documentation/git-merge.txt +4 -2
@@ -94,7 +94,8 @@ will be appended to the specified message. --abort:: Abort the current conflict resolution process, and - try to reconstruct the pre-merge state. + try to reconstruct the pre-merge state. If an autostash entry is + present, apply it back to the worktree. + If there were uncommitted worktree changes present when the merge started, 'git merge --abort' will in some cases be unable to
@@ -102,7 +103,8 @@ reconstruct these changes. It is therefore recommended to always commit or stash your changes before running 'git merge'. + 'git merge --abort' is equivalent to 'git reset --merge' when -`MERGE_HEAD` is present. +`MERGE_HEAD` is present unless `MERGE_AUTOSTASH` is also present where +the stash entry is applied to the worktree. --quit:: Forget about the current merge in progress. Leave the index
Documentation/merge-options.txt +8 -0
@@ -155,6 +155,14 @@ ifndef::git-pull[] Note that not all merge strategies may support progress reporting. +--autostash:: +--no-autostash:: + Automatically create a temporary stash entry before the operation + begins, and apply it after the operation ends. This means + that you can run the operation on a dirty worktree. However, use + with care: the final stash application after a successful + merge might result in non-trivial conflicts. + endif::git-pull[] --allow-unrelated-histories::
branch.c +1 -0
@@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r)unlink (git_path_merge_rr (r));unlink (git_path_merge_msg (r));unlink (git_path_merge_mode (r)); +save_autostash (git_path_merge_autostash (r)); }void remove_branch_state (struct repository *r,int verbose)
builtin/commit.c +2 -0
@@ -1713,6 +1713,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) &oid, flags); } +apply_autostash (git_path_merge_autostash (the_repository)); +UNLEAK (err);UNLEAK (sb);return 0 ;
builtin/merge.c +27 -0
@@ -82,6 +82,7 @@ static int show_progress = -1;static int default_to_upstream =1 ;static int signoff;static const char *sign_commit; +static int autostash;static int no_verify;static struct strategy all_strategy[] = {
@@ -286,6 +287,7 @@ static struct option builtin_merge_options[] = {OPT_SET_INT (0 ,"progress" , &show_progress,N_ ("force progress reporting" ),1 ), { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"), N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, +OPT_AUTOSTASH (&autostash),OPT_BOOL (0 ,"overwrite-ignore" , &overwrite_ignore,N_ ("update ignored files (default)" )),OPT_BOOL (0 ,"signoff" , &signoff,N_ ("add Signed-off-by:" )), OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
@@ -475,6 +477,7 @@ static void finish(struct commit *head_commit,/* Run a post-merge hook */ run_hook_le (NULL ,"post-merge" , squash ?"1" :"0" ,NULL ); +apply_autostash (git_path_merge_autostash (the_repository));strbuf_release (&reflog_message); }
@@ -634,6 +637,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)return 0 ; }else if (!strcmp (k,"gpg.mintrustlevel" )) { check_trust_level =0 ; + }else if (!strcmp (k,"merge.autostash" )) { + autostash =git_config_bool (k, v); +return 0 ; } status =fmt_merge_msg_config (k, v, cb);
@@ -1281,6 +1287,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (abort_current_merge) {int nargc =2 ;const char *nargv[] = {"reset" ,"--merge" ,NULL }; +struct strbuf stash_oid =STRBUF_INIT ;if (orig_argc !=2 ) usage_msg_opt(_("--abort expects no arguments"),
@@ -1289,8 +1296,19 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (!file_exists (git_path_merge_head (the_repository)))die (_ ("There is no merge to abort (MERGE_HEAD missing)." )); +if (read_oneliner (&stash_oid,git_path_merge_autostash (the_repository), +READ_ONELINER_SKIP_IF_EMPTY )) { +strbuf_trim (&stash_oid); +unlink (git_path_merge_autostash (the_repository)); + } +/* Invoke 'git reset --merge' */ ret =cmd_reset (nargc, nargv, prefix); + +if (stash_oid.len) +apply_autostash_oid (stash_oid.buf); + +strbuf_release (&stash_oid); goto done; }
@@ -1513,6 +1531,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix) goto done; } +if (autostash) +create_autostash (the_repository, +git_path_merge_autostash (the_repository), +"merge" ); if (checkout_fast_forward(the_repository, &head_commit->object.oid, &commit->object.oid,
@@ -1579,6 +1601,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (fast_forward ==FF_ONLY )die (_ ("Not possible to fast-forward, aborting." )); +if (autostash) +create_autostash (the_repository, +git_path_merge_autostash (the_repository), +"merge" ); +/* We are going to make a new commit. */ git_committer_info (IDENT_STRICT );
builtin/rebase.c +1 -2
@@ -1371,8 +1371,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) {OPTION_STRING ,'S' ,"gpg-sign" , &gpg_sign,N_ ("key-id" ),N_ ("GPG-sign commits" ), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, -OPT_BOOL (0 ,"autostash" , &options.autostash, -N_ ("automatically stash/stash pop before and after" )), +OPT_AUTOSTASH (&options.autostash),OPT_STRING_LIST ('x' ,"exec" , &exec,N_ ("exec" ),N_ ("add exec lines after each commit of the " "editable list")),
builtin/reset.c +3 -1
@@ -25,6 +25,7 @@#include "cache-tree.h" #include "submodule.h" #include "submodule-config.h" +#include "sequencer.h" #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
@@ -437,8 +438,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)if (reset_type ==HARD && !update_ref_status && !quiet)print_new_head_line (lookup_commit_reference (the_repository, &oid)); } -if (!pathspec.nr) +if (!pathspec.nr) {remove_branch_state (the_repository,0 ); + }return update_ref_status; }
parse-options.h +1 -0
@@ -336,5 +336,6 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);#define OPT_CLEANUP (v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))#define OPT_PATHSPEC_FROM_FILE (v) OPT_FILENAME(0, "pathspec-from-file", v, N_("read pathspec from file"))#define OPT_PATHSPEC_FILE_NUL (v) OPT_BOOL(0, "pathspec-file-nul", v, N_("with --pathspec-from-file, pathspec elements are separated with NUL character")) +#define OPT_AUTOSTASH (v) OPT_BOOL(0, "autostash", v, N_("automatically stash/stash pop before and after"))#endif
path.c +1 -0
@@ -1535,5 +1535,6 @@ REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG") REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR") REPO_GIT_PATH_FUNC(merge_mode, "MERGE_MODE") REPO_GIT_PATH_FUNC(merge_head, "MERGE_HEAD") +REPO_GIT_PATH_FUNC(merge_autostash, "MERGE_AUTOSTASH") REPO_GIT_PATH_FUNC(fetch_head, "FETCH_HEAD")REPO_GIT_PATH_FUNC (shallow,"shallow" )
path.h +3 -1
@@ -177,11 +177,12 @@ struct path_cache {const char *merge_rr;const char *merge_mode;const char *merge_head; +const char *merge_autostash;const char *fetch_head;const char *shallow; }; -#define PATH_CACHE_INIT { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL } +#define PATH_CACHE_INIT { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL }const char *git_path_cherry_pick_head (struct repository *r);const char *git_path_revert_head (struct repository *r);
@@ -190,6 +191,7 @@ const char *git_path_merge_msg(struct repository *r);const char *git_path_merge_rr (struct repository *r);const char *git_path_merge_mode (struct repository *r);const char *git_path_merge_head (struct repository *r); +const char *git_path_merge_autostash (struct repository *r);const char *git_path_fetch_head (struct repository *r);const char *git_path_shallow (struct repository *r);
t/t3033-merge-toplevel.sh +22 -0
@@ -142,6 +142,17 @@ test_expect_success 'refuse two-project merge by default' ' test_must_fail git merge five ' +test_expect_success 'refuse two-project merge by default, quit before --autostash happens' ' + t3033_reset && + git reset --hard four && + echo change >>one.t && + git diff >expect && + test_must_fail git merge --autostash five 2>err && + test_i18ngrep ! "stash" err && + git diff >actual && + test_cmp expect actual +' + test_expect_success 'two-project merge with --allow-unrelated-histories' ' t3033_reset && git reset --hard four &&
@@ -149,4 +160,15 @@ test_expect_success 'two-project merge with --allow-unrelated-histories' ' git diff --exit-code five ' +test_expect_success 'two-project merge with --allow-unrelated-histories with --autostash' ' + t3033_reset && + git reset --hard four && + echo change >>one.t && + git diff one.t >expect && + git merge --allow-unrelated-histories --autostash five 2>err && + test_i18ngrep "Applied autostash." err && + git diff one.t >actual && + test_cmp expect actual +' + test_done
t/t7600-merge.sh +118 -0
137 lines not on this page · raw message
Signature · 1 line
2.26.0.159.g23e2136ad0
Re: [PATCH v4 21/23] merge: teach --autostash option
Hi Denton
Thanks for re-rolling, I think this addresses all of my previous concerns, I've got a few minor comments below and a query about 'git merge --quit'
Quoted 77 lines · Denton Liu
In rebase, one can pass the
--autostashoption to cause the worktree to be automatically stashed before continuing with the rebase. This option is missing in merge, however.Implement the
--autostashoption and correspondingmerge.autoStashoption in merge which stashes before merging and then pops after.This option is useful when a developer has some local changes on a topic branch but they realize that their work depends on another branch. Previously, they had to run something like
git fetch ...
git stash push
git mergeFETCH_HEAD
git stash popbut now, that is reduced to
git fetch ... git merge --autostash FETCH_HEADWhen an autostash is generated, it is automatically reapplied to the worktree only in three explicit situations:
1. An incomplete merge is commit using
git commit.
2. A merge completes successfully.
3. A merge is aborted usinggit merge --abort.In all other situations where the merge state is removed using
remove_merge_branch_state()such as aborting a merge viagit reset --hard, the autostash is saved into the stash reflog instead keeping the worktree clean.Suggested-by Alban Gruin <[email protected]>Signed-off-by Denton Liu <[email protected]>--- Documentation/config/merge.txt | 10 +++ Documentation/git-merge.txt | 6 +- Documentation/merge-options.txt | 8 +++ branch.c | 1 + builtin/commit.c | 2 + builtin/merge.c | 27 ++++++++ builtin/rebase.c | 3 +- builtin/reset.c | 4 +- parse-options.h | 1 + path.c | 1 + path.h | 4 +- t/t3033-merge-toplevel.sh | 22 ++++++ t/t7600-merge.sh | 118 ++++++++++++++++++++++++++++++++ 13 files changed, 201 insertions(+), 6 deletions(-)Documentation/config/merge.txt +10
-0@@ -70,6 +70,16 @@ merge.stat:: Whether to print the diffstat between ORIG_HEAD and the merge result at the end of the merge. True by default. +merge.autoStash:: + When set to true, automatically create a temporary stash entry + before the operation begins, and apply it after the operation + ends. This means that you can run merge on a dirty worktree. + However, use with care: the final stash application after a + successful merge might result in non-trivial conflicts. + This option can be overridden by the `--no-autostash` and + `--autostash` options of linkgit:git-merge[1]. + Defaults to false. + merge.tool:: Controls which merge tool is used by linkgit:git-mergetool[1]. The list below shows the valid built-in values.Documentation/git-merge.txt +0
-0
Documentation/git-merge.txt will be appended to the specified message. · line 98
Quoted 6 lines
Documentation/git-merge.txt +2
-1@@ -94,7 +94,8 @@ will be appended to the specified message. --abort:: Abort the current conflict resolution process, and - try to reconstruct the pre-merge state. + try to reconstruct the pre-merge state. If an autostash entry is + present, apply it back to the worktree.
Being picky I'd suggest deleting 'back'
Documentation/git-merge.txt reconstruct these changes. It is therefore recommended to always · line 107
Quoted 10 lines
Documentation/git-merge.txt +2
-1+ If there were uncommitted worktree changes present when the merge started, 'git merge --abort' will in some cases be unable to@@ -102,7 +103,8 @@ reconstruct these changes. It is therefore recommended to always commit or stash your changes before running 'git merge'. + 'git merge --abort' is equivalent to 'git reset --merge' when -`MERGE_HEAD` is present. +`MERGE_HEAD` is present unless `MERGE_AUTOSTASH` is also present where +the stash entry is applied to the worktree.
Perhaps
+MERGE_HEAD is present unless MERGE_AUTOSTASH is also present in +which case 'git merge --abort' applies the stash entry to the worktree +whereas 'git reset --merge' will save the stashed changes in the stash +reflog.
Documentation/git-merge.txt reconstruct these changes. It is therefore recommended to always · line 110
Quoted 2 lines
Documentation/git-merge.txt +0
-0--quit:: Forget about the current merge in progress. Leave the index
Should we be saving the stash if the user runs 'git merge --quit'?
Quoted 36 lines
Documentation/merge-options.txt +8
-0@@ -155,6 +155,14 @@ ifndef::git-pull[] Note that not all merge strategies may support progress reporting. +--autostash:: +--no-autostash:: + Automatically create a temporary stash entry before the operation + begins, and apply it after the operation ends. This means + that you can run the operation on a dirty worktree. However, use + with care: the final stash application after a successful + merge might result in non-trivial conflicts. + endif::git-pull[] --allow-unrelated-histories::branch.c +1
-0@@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r)unlink (git_path_merge_rr (r));unlink (git_path_merge_msg (r));unlink (git_path_merge_mode (r)); +save_autostash (git_path_merge_autostash (r)); }void remove_branch_state (struct repository *r,int verbose)builtin/commit.c +0
-0
builtin/commit.c int cmd_commit(int argc, const char **argv, const char *prefix) · line 1717
Quoted 5 lines
builtin/commit.c +2
-0@@ -1713,6 +1713,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) &oid, flags); } +apply_autostash (git_path_merge_autostash (the_repository)); +
I think this is the right place to apply the stash. There is a corner case - if commit_index_files() fails then we leave MERGE_AUTOSTASH lying around. I think that is probably the best we can do - if we cannot write the new index file to disk then saving or applying the stash would probably fail as well.
Quoted 24 lines
builtin/commit.c +0
-0UNLEAK (err);UNLEAK (sb);return 0 ;builtin/merge.c +2
-0@@ -82,6 +82,7 @@ static int show_progress = -1;static int default_to_upstream =1 ;static int signoff;static const char *sign_commit; +static int autostash;static int no_verify;static struct strategy all_strategy[] = {@@ -286,6 +287,7 @@ static struct option builtin_merge_options[] = {OPT_SET_INT (0 ,"progress" , &show_progress,N_ ("force progress reporting" ),1 ), { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"), N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, +OPT_AUTOSTASH (&autostash),OPT_BOOL (0 ,"overwrite-ignore" , &overwrite_ignore,N_ ("update ignored files (default)" )),OPT_BOOL (0 ,"signoff" , &signoff,N_ ("add Signed-off-by:" )), OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
builtin/merge.c static void finish(struct commit *head_commit, · line 480
Quoted 4 lines
builtin/merge.c +1
-0@@ -475,6 +477,7 @@ static void finish(struct commit *head_commit,/* Run a post-merge hook */ run_hook_le (NULL ,"post-merge" , squash ?"1" :"0" ,NULL ); +apply_autostash (git_path_merge_autostash (the_repository));
I think this is the right place to apply it
Quoted 22 lines
builtin/merge.c +4
-0strbuf_release (&reflog_message); }@@ -634,6 +637,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)return 0 ; }else if (!strcmp (k,"gpg.mintrustlevel" )) { check_trust_level =0 ; + }else if (!strcmp (k,"merge.autostash" )) { + autostash =git_config_bool (k, v); +return 0 ; } status =fmt_merge_msg_config (k, v, cb);@@ -1281,6 +1287,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (abort_current_merge) {int nargc =2 ;const char *nargv[] = {"reset" ,"--merge" ,NULL }; +struct strbuf stash_oid =STRBUF_INIT ;if (orig_argc !=2 ) usage_msg_opt(_("--abort expects no arguments"),
builtin/merge.c int cmd_merge(int argc, const char **argv, const char *prefix) · line 1300
Quoted 5 lines
builtin/merge.c +2
-0@@ -1289,8 +1296,19 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (!file_exists (git_path_merge_head (the_repository)))die (_ ("There is no merge to abort (MERGE_HEAD missing)." )); +if (read_oneliner (&stash_oid,git_path_merge_autostash (the_repository), +READ_ONELINER_SKIP_IF_EMPTY )) {
dodgy indentation
builtin/merge.c int cmd_merge(int argc, const char **argv, const char *prefix) · line 1301
Quoted 1 line
builtin/merge.c +1
-0+strbuf_trim (&stash_oid);
do we really need to trim the buffer?
Quoted 63 lines
builtin/merge.c +17
-0+unlink (git_path_merge_autostash (the_repository)); + } +/* Invoke 'git reset --merge' */ ret =cmd_reset (nargc, nargv, prefix); + +if (stash_oid.len) +apply_autostash_oid (stash_oid.buf); + +strbuf_release (&stash_oid); goto done; }@@ -1513,6 +1531,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix) goto done; } +if (autostash) +create_autostash (the_repository, +git_path_merge_autostash (the_repository), +"merge" ); if (checkout_fast_forward(the_repository, &head_commit->object.oid, &commit->object.oid,@@ -1579,6 +1601,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (fast_forward ==FF_ONLY )die (_ ("Not possible to fast-forward, aborting." )); +if (autostash) +create_autostash (the_repository, +git_path_merge_autostash (the_repository), +"merge" ); +/* We are going to make a new commit. */ git_committer_info (IDENT_STRICT );builtin/rebase.c +1
-2@@ -1371,8 +1371,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) {OPTION_STRING ,'S' ,"gpg-sign" , &gpg_sign,N_ ("key-id" ),N_ ("GPG-sign commits" ), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, -OPT_BOOL (0 ,"autostash" , &options.autostash, -N_ ("automatically stash/stash pop before and after" )), +OPT_AUTOSTASH (&options.autostash),OPT_STRING_LIST ('x' ,"exec" , &exec,N_ ("exec" ),N_ ("add exec lines after each commit of the " "editable list")),builtin/reset.c +1
-0@@ -25,6 +25,7 @@#include "cache-tree.h" #include "submodule.h" #include "submodule-config.h" +#include "sequencer.h" #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
builtin/reset.c int cmd_reset(int argc, const char **argv, const char *prefix) · line 443
Quoted 7 lines
builtin/reset.c +2
-1@@ -437,8 +438,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)if (reset_type ==HARD && !update_ref_status && !quiet)print_new_head_line (lookup_commit_reference (the_repository, &oid)); } -if (!pathspec.nr) +if (!pathspec.nr) {remove_branch_state (the_repository,0 ); + }
No need to add these braces any longer
Best Wishes
Phillip
Quoted 233 lines
builtin/reset.c +0
-0return update_ref_status; }parse-options.h +1
-0@@ -336,5 +336,6 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);#define OPT_CLEANUP (v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))#define OPT_PATHSPEC_FROM_FILE (v) OPT_FILENAME(0, "pathspec-from-file", v, N_("read pathspec from file"))#define OPT_PATHSPEC_FILE_NUL (v) OPT_BOOL(0, "pathspec-file-nul", v, N_("with --pathspec-from-file, pathspec elements are separated with NUL character")) +#define OPT_AUTOSTASH (v) OPT_BOOL(0, "autostash", v, N_("automatically stash/stash pop before and after"))#endif path.c +1
-0@@ -1535,5 +1535,6 @@ REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG") REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR") REPO_GIT_PATH_FUNC(merge_mode, "MERGE_MODE") REPO_GIT_PATH_FUNC(merge_head, "MERGE_HEAD") +REPO_GIT_PATH_FUNC(merge_autostash, "MERGE_AUTOSTASH") REPO_GIT_PATH_FUNC(fetch_head, "FETCH_HEAD")REPO_GIT_PATH_FUNC (shallow,"shallow" )path.h +3
-1@@ -177,11 +177,12 @@ struct path_cache {const char *merge_rr;const char *merge_mode;const char *merge_head; +const char *merge_autostash;const char *fetch_head;const char *shallow; }; -#define PATH_CACHE_INIT { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL } +#define PATH_CACHE_INIT { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL }const char *git_path_cherry_pick_head (struct repository *r);const char *git_path_revert_head (struct repository *r);@@ -190,6 +191,7 @@ const char *git_path_merge_msg(struct repository *r);const char *git_path_merge_rr (struct repository *r);const char *git_path_merge_mode (struct repository *r);const char *git_path_merge_head (struct repository *r); +const char *git_path_merge_autostash (struct repository *r);const char *git_path_fetch_head (struct repository *r);const char *git_path_shallow (struct repository *r);t/t3033-merge-toplevel.sh +22
-0@@ -142,6 +142,17 @@ test_expect_success 'refuse two-project merge by default' ' test_must_fail git merge five ' +test_expect_success 'refuse two-project merge by default, quit before --autostash happens' ' + t3033_reset && + git reset --hard four && + echo change >>one.t && + git diff >expect && + test_must_fail git merge --autostash five 2>err && + test_i18ngrep ! "stash" err && + git diff >actual && + test_cmp expect actual +' + test_expect_success 'two-project merge with --allow-unrelated-histories' ' t3033_reset && git reset --hard four &&@@ -149,4 +160,15 @@ test_expect_success 'two-project merge with --allow-unrelated-histories' ' git diff --exit-code five ' +test_expect_success 'two-project merge with --allow-unrelated-histories with --autostash' ' + t3033_reset && + git reset --hard four && + echo change >>one.t && + git diff one.t >expect && + git merge --allow-unrelated-histories --autostash five 2>err && + test_i18ngrep "Applied autostash." err && + git diff one.t >actual && + test_cmp expect actual +' + test_donet/t7600-merge.sh +118
-0@@ -30,13 +30,17 @@ Testing basic merge operations/option parsing. . "$TEST_DIRECTORY"/lib-gpg.sh test_write_lines 1 2 3 4 5 6 7 8 9 >file +cp file file.orig test_write_lines '1 X' 2 3 4 5 6 7 8 9 >file.1 +test_write_lines 1 2 '3 X' 4 5 6 7 8 9 >file.3 test_write_lines 1 2 3 4 '5 X' 6 7 8 9 >file.5 test_write_lines 1 2 3 4 5 6 7 8 '9 X' >file.9 test_write_lines 1 2 3 4 5 6 7 8 '9 Y' >file.9y test_write_lines '1 X' 2 3 4 5 6 7 8 9 >result.1 test_write_lines '1 X' 2 3 4 '5 X' 6 7 8 9 >result.1-5 +test_write_lines '1 X' 2 3 4 5 6 7 8 '9 X' >result.1-9 test_write_lines '1 X' 2 3 4 '5 X' 6 7 8 '9 X' >result.1-5-9 +test_write_lines '1 X' 2 '3 X' 4 '5 X' 6 7 8 '9 X' >result.1-3-5-9 test_write_lines 1 2 3 4 5 6 7 8 '9 Z' >result.9z create_merge_msgs () {@@ -675,6 +679,120 @@ test_expect_success 'refresh the index before merging' ' git merge c3 ' +test_expect_success 'merge with --autostash' ' + git reset --hard c1 && + git merge-file file file.orig file.9 && + git merge --autostash c2 2>err && + test_i18ngrep "Applied autostash." err && + git show HEAD:file >merge-result && + test_cmp result.1-5 merge-result && + test_cmp result.1-5-9 file +' + +test_expect_success 'merge with merge.autoStash' ' + test_config merge.autoStash true && + git reset --hard c1 && + git merge-file file file.orig file.9 && + git merge c2 2>err && + test_i18ngrep "Applied autostash." err && + git show HEAD:file >merge-result && + test_cmp result.1-5 merge-result && + test_cmp result.1-5-9 file +' + +test_expect_success 'fast-forward merge with --autostash' ' + git reset --hard c0 && + git merge-file file file.orig file.5 && + git merge --autostash c1 2>err && + test_i18ngrep "Applied autostash." err && + test_cmp result.1-5 file +' + +test_expect_success 'octopus merge with --autostash' ' + git reset --hard c1 && + git merge-file file file.orig file.3 && + git merge --autostash c2 c3 2>err && + test_i18ngrep "Applied autostash." err && + git show HEAD:file >merge-result && + test_cmp result.1-5-9 merge-result && + test_cmp result.1-3-5-9 file +' + +test_expect_success 'conflicted merge with --autostash, --abort restores stash' ' + git reset --hard c3 && + cp file.1 file && + test_must_fail git merge --autostash c7 && + git merge --abort 2>err && + test_i18ngrep "Applied autostash." err && + test_cmp file.1 file +' + +test_expect_success 'completed merge (git commit) with --no-commit and --autostash' ' + git reset --hard c1 && + git merge-file file file.orig file.9 && + git diff >expect && + git merge --no-commit --autostash c2 && + git stash show -p MERGE_AUTOSTASH >actual && + test_cmp expect actual && + git commit 2>err && + test_i18ngrep "Applied autostash." err && + git show HEAD:file >merge-result && + test_cmp result.1-5 merge-result && + test_cmp result.1-5-9 file +' + +test_expect_success 'completed merge (git merge --continue) with --no-commit and --autostash' ' + git reset --hard c1 && + git merge-file file file.orig file.9 && + git diff >expect && + git merge --no-commit --autostash c2 && + git stash show -p MERGE_AUTOSTASH >actual && + test_cmp expect actual && + git merge --continue 2>err && + test_i18ngrep "Applied autostash." err && + git show HEAD:file >merge-result && + test_cmp result.1-5 merge-result && + test_cmp result.1-5-9 file +' + +test_expect_success 'aborted merge (merge --abort) with --no-commit and --autostash' ' + git reset --hard c1 && + git merge-file file file.orig file.9 && + git diff >expect && + git merge --no-commit --autostash c2 && + git stash show -p MERGE_AUTOSTASH >actual && + test_cmp expect actual && + git merge --abort 2>err && + test_i18ngrep "Applied autostash." err && + git diff >actual && + test_cmp expect actual +' + +test_expect_success 'aborted merge (reset --hard) with --no-commit and --autostash' ' + git reset --hard c1 && + git merge-file file file.orig file.9 && + git diff >expect && + git merge --no-commit --autostash c2 && + git stash show -p MERGE_AUTOSTASH >actual && + test_cmp expect actual && + git reset --hard 2>err && + test_i18ngrep "Autostash exists; creating a new stash entry." err && + git diff --exit-code +' + +test_expect_success 'merge with conflicted --autostash changes' ' + git reset --hard c1 && + git merge-file file file.orig file.9y && + git diff >expect && + test_when_finished "test_might_fail git stash drop" && + git merge --autostash c3 2>err && + test_i18ngrep "Applying autostash resulted in conflicts." err && + git show HEAD:file >merge-result && + test_cmp result.1-9 merge-result && + git stash show -p >actual && + test_cmp expect actual +' + cat >expected.branch <<\EOF Merge branch 'c5-branch' (early part) EOF
Re: [PATCH v4 21/23] merge: teach --autostash option
Hi Phillip,
Thanks for the review. The documentation rewordings are particularly helpful.
Quoted 4 lines · Phillip Wood
Quoted 2 lines
--quit::
Forget about the current merge in progress. Leave the indexShould we be saving the stash if the user runs 'git merge --quit'?
Yes, this is its current behaviour, although it's undocumented. In my next reroll, I've added a test case and documentation for this. This is similar to how git rebase --quit behaves.
Now that I think about it, when git rebase --quit happens, the autostash is just dropped out of existence, which is probably undesirable behaviour. I'll probably submit a later series to save that autostash so that it's not lost forever.
Thanks,
Denton
Re: [PATCH v4 21/23] merge: teach --autostash option
Hi Denton
Quoted 4 lines · Denton Liu
Hi Phillip,
Thanks for the review. The documentation rewordings are particularly helpful.
Thanks I'm glad they were useful
Quoted 9 lines · Phillip Wood
Quoted 4 lines
Quoted 2 lines
--quit::
Forget about the current merge in progress. Leave the indexShould we be saving the stash if the user runs 'git merge --quit'?
Yes, this is its current behaviour, although it's undocumented. In my next reroll, I've added a test case and documentation for this. This is similar to how
git rebase --quitbehaves.
That's great - I'll try and read through v5 later in the week, it looks like there aren't too many changes
Quoted 4 lines
Now that I think about it, when
git rebase --quithappens, the autostash is just dropped out of existence, which is probably undesirable behaviour. I'll probably submit a later series to save that autostash so that it's not lost forever.
That's a good idea, Thanks
Phillip
Quoted 4 lines
Thanks,
Denton
PATCH v4 22/23 t5520: make test_pull_autostash() accept expect_parent_num
In reply to Denton LiuBefore, test_pull_autostash() was hardcoded to run test_cmp_rev HEAD^ copy to test that a rebase happened. However, in a future patch, we plan on testing merging as well. Make test_pull_autostash() accept a parent number as an argument so that, in the future, we can test if a merge happened in addition to a rebase.
--- t/t5520-pull.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
t/t5520-pull.sh +8 -6
@@ -10,11 +10,13 @@ modify () { } test_pull_autostash () { + expect_parent_num="$1" && + shift && git reset --hard before-rebase && echo dirty >new_file && git add new_file && git pull "$@" . copy && - test_cmp_rev HEAD^ copy && + test_cmp_rev HEAD^"$expect_parent_num" copy && echo dirty >expect && test_cmp expect new_file && echo "modified again" >expect &&
@@ -369,22 +371,22 @@ test_expect_success '--rebase fails with multiple branches' ' test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' test_config rebase.autostash true && - test_pull_autostash --rebase + test_pull_autostash 1 --rebase ' test_expect_success 'pull --rebase --autostash & rebase.autostash=true' ' test_config rebase.autostash true && - test_pull_autostash --rebase --autostash + test_pull_autostash 1 --rebase --autostash ' test_expect_success 'pull --rebase --autostash & rebase.autostash=false' ' test_config rebase.autostash false && - test_pull_autostash --rebase --autostash + test_pull_autostash 1 --rebase --autostash ' test_expect_success 'pull --rebase --autostash & rebase.autostash unset' ' test_unconfig rebase.autostash && - test_pull_autostash --rebase --autostash + test_pull_autostash 1 --rebase --autostash ' test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
@@ -422,7 +424,7 @@ test_expect_success 'pull.rebase' ' test_expect_success 'pull --autostash & pull.rebase=true' ' test_config pull.rebase true && - test_pull_autostash --autostash + test_pull_autostash 1 --autostash ' test_expect_success 'pull --no-autostash & pull.rebase=true' '
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v4 23/23 pull: pass --autostash to merge
In reply to Denton LiuBefore, --autostash only worked with git pull --rebase. However, in the last patch, merge learned --autostash as well so there's no reason why we should have this restriction anymore. Teach pull to pass --autostash to merge, just like it did for rebase.
--- Documentation/git-pull.txt | 9 ------- Documentation/merge-options.txt | 4 +-- builtin/pull.c | 9 ++++--- t/t5520-pull.sh | 43 +++++++++++++++++++++++++++------ 4 files changed, 42 insertions(+), 23 deletions(-)
Documentation/git-pull.txt +0 -9
@@ -133,15 +133,6 @@ unless you have read linkgit:git-rebase[1] carefully. --no-rebase:: Override earlier --rebase. ---autostash:: ---no-autostash:: - Before starting rebase, stash local modifications away (see - linkgit:git-stash[1]) if needed, and apply the stash entry when - done. `--no-autostash` is useful to override the `rebase.autoStash` - configuration variable (see linkgit:git-config[1]). -+ -This option is only valid when "--rebase" is used. - Options related to fetching ~~~~~~~~~~~~~~~~~~~~~~~~~~~
Documentation/merge-options.txt +2 -2
@@ -155,6 +155,8 @@ ifndef::git-pull[] Note that not all merge strategies may support progress reporting. +endif::git-pull[] + --autostash:: --no-autostash:: Automatically create a temporary stash entry before the operation
@@ -163,8 +165,6 @@ ifndef::git-pull[] with care: the final stash application after a successful merge might result in non-trivial conflicts. -endif::git-pull[] - --allow-unrelated-histories:: By default, `git merge` command refuses to merge histories that do not share a common ancestor. This option can be
builtin/pull.c +5 -4
@@ -163,7 +163,7 @@ static struct option pull_options[] = { N_("verify that the named commit has a valid GPG signature"), PARSE_OPT_NOARG),OPT_BOOL (0 ,"autostash" , &opt_autostash, -N_ ("automatically stash/stash pop before and after rebase" )), +N_ ("automatically stash/stash pop before and after" )),OPT_PASSTHRU_ARGV ('s' ,"strategy" , &opt_strategies,N_ ("strategy" ),N_ ("merge strategy to use" ), 0),
@@ -661,6 +661,10 @@ static int run_merge(void)argv_array_pushv (&args, opt_strategy_opts.argv);if (opt_gpg_sign)argv_array_push (&args, opt_gpg_sign); +if (opt_autostash ==0 ) +argv_array_push (&args,"--no-autostash" ); +else if (opt_autostash ==1 ) +argv_array_push (&args,"--autostash" );if (opt_allow_unrelated_histories >0 )argv_array_push (&args,"--allow-unrelated-histories" );
@@ -908,9 +912,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)if (get_oid ("HEAD" , &orig_head))oidclr (&orig_head); -if (!opt_rebase && opt_autostash !=-1 ) -die (_ ("--[no-]autostash option is only valid with --rebase." )); - autostash = config_autostash;if (opt_rebase) {if (opt_autostash !=-1 )
t/t5520-pull.sh +35 -8
55 lines not on this page · raw message
Signature · 1 line
2.26.0.159.g23e2136ad0
v5 cover merge: learn --autostash
In reply to Denton LiuAlban reported[1] that he expected merge to have an --autostash option, just like rebase. Since there's not really any reason why merge can't have it, let's implement it in this patchset.
The majority of this patchset is spent refactoring. This is so that the actual implementation in merge is done as simply as possible.
Chagnes since v4:
* Drop 'sequencer: reuse strbuf_trim() in read_oneliner()'
* Fix commit message in 'sequencer: make apply_autostash() accept a path'
* Add more detail in commit message 'sequencer: implement save_autostash()'
* In 'merge: teach --autostash option', improve documentation, remove builtin/reset.c changes, add a git merge --quit test case
Changes since v3:
* Replace 'sequencer: use file strbuf for read_oneliner()' with patches 3-5
* Rename flag to READ_ONELINER_WARN_MISSING
* Add 'sequencer: rename stash_sha1 to stash_oid'
* Many changes to 'merge: teach --autostash option'
Changes since v2:
* Change patch 1 to refer to ASCII order[2]
* Make read_oneliner() accept a collective flags arg instead of two separate args[3]
* git reset --hard now leaves the worktree clean[4]
* Rebased against 'master' since the base is quite old and a new release is coming out soon
Changes since v1:
* Completely changed how the refactoring was done
* More tests and documentation
[1]: https://github.com/gitgitgadget/git/issues/394
[2]: https://lore.kernel.org/git/[email protected]/
[3]: https://lore.kernel.org/git/[email protected]/
[4]: https://lore.kernel.org/git/20200123042906.GA29009@generichostname/
Denton Liu (22):
Makefile: ASCII-sort += lists
t7600: use test_write_lines()
sequencer: stop leaking buf
sequencer: make file exists check more efficient
sequencer: make read_oneliner() accept flags
sequencer: configurably warn on non-existent files
sequencer: make read_oneliner() extern
rebase: use read_oneliner()
sequencer: make apply_autostash() accept a path
sequencer: rename stash_sha1 to stash_oid
rebase: use apply_autostash() from sequencer.c
rebase: generify reset_head()
reset: extract reset_head() from rebase
rebase: extract create_autostash()
rebase: generify create_autostash()
sequencer: extract perform_autostash() from rebase
sequencer: unlink autostash in apply_autostash()
sequencer: implement save_autostash()
sequencer: implement apply_autostash_oid()
merge: teach --autostash option
t5520: make test_pull_autostash() accept expect_parent_num
pull: pass --autostash to merge
Documentation/config/merge.txt | 10 ++ Documentation/git-merge.txt | 11 +- Documentation/git-pull.txt | 9 - Documentation/merge-options.txt | 8 + Makefile | 78 ++++---- branch.c | 1 + builtin/commit.c | 2 + builtin/merge.c | 25 +++ builtin/pull.c | 9 +- builtin/rebase.c | 307 +++++--------------------------- parse-options.h | 1 + path.c | 1 + path.h | 4 +- reset.c | 140 +++++++++++++++ reset.h | 20 +++ sequencer.c | 176 +++++++++++++----- sequencer.h | 20 +++ t/t3033-merge-toplevel.sh | 22 +++ t/t5520-pull.sh | 57 ++++-- t/t7600-merge.sh | 154 ++++++++++++++-- 20 files changed, 665 insertions(+), 390 deletions(-) create mode 100644 reset.c create mode 100644 reset.h
Range-diff against v4:
1: c26771e934 = 1: c26771e934 Makefile: ASCII-sort += lists
2: 059e0e8e43 = 2: 059e0e8e43 t7600: use test_write_lines()
3: 76585e5b13 = 3: 76585e5b13 sequencer: stop leaking buf
4: c7a3cfa200 < -: ---------- sequencer: reuse strbuf_trim() in read_oneliner()
5: 58da8e9555 ! 4: dc4674d222 sequencer: make file exists check more efficient
@@ Commit message
check `errno` to see if the file doesn't exist.
## sequencer.c ##
-@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *filename,
- static int read_oneliner(struct strbuf *buf,
- const char *path, int skip_if_empty)
+@@ sequencer.c: static int read_oneliner(struct strbuf *buf,
{
--
+ int orig_len = buf->len;
+
- if (!file_exists(path))
- return 0;
-
- strbuf_reset(buf);
if (strbuf_read_file(buf, path, 0) < 0) {
- warning_errno(_("could not read '%s'"), path);
+ if (errno != ENOENT && errno != ENOTDIR) 6: 6ead240957 ! 5: f30ad07823 sequencer: make read_oneliner() accept flags
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *f
+#define READ_ONELINER_SKIP_IF_EMPTY (1 << 0)
+
/*
- * Resets a strbuf then reads a file that was presumably written by a shell
- * script, i.e. with an end-of-line marker that needs to be stripped.
+ * Reads a file that was presumably written by a shell script, i.e. with an
+ * end-of-line marker that needs to be stripped.
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *filename,
* Returns 1 if the file was read, 0 if it could not be read or does not exist. */
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *f
- const char *path, int skip_if_empty)
+ const char *path, unsigned flags)
{
- strbuf_reset(buf);
- if (strbuf_read_file(buf, path, 0) < 0) {
+ int orig_len = buf->len;
+
@@ sequencer.c: static int read_oneliner(struct strbuf *buf,
+ buf->buf[buf->len] = '\0';
+ }
- strbuf_trim(buf);
-
-- if (skip_if_empty && !buf->len)
-+ if ((flags & READ_ONELINER_SKIP_IF_EMPTY) && !buf->len)
+- if (skip_if_empty && buf->len == orig_len)
++ if ((flags & READ_ONELINER_SKIP_IF_EMPTY) && buf->len == orig_len)
return 0;
return 1;
@@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
strbuf_reset(&buf);
else {
@@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
- }
+ strbuf_reset(&buf);
}
- if (read_oneliner(&buf, rebase_path_allow_rerere_autoupdate(), 1)) {
@@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
opts->allow_rerere_auto = RERERE_AUTOUPDATE;
else if (!strcmp(buf.buf, "--no-rerere-autoupdate"))
@@ sequencer.c: static int read_populate_opts(struct replay_opts *opts)
- read_strategy_opts(opts, &buf);
+ strbuf_reset(&buf);
if (read_oneliner(&opts->current_fixups,
- rebase_path_current_fixups(), 1)) {
7: 2e7922b259 ! 6: 8c504427e3 sequencer: configurably warn on non-existent files
@@ Commit message
read_oneliner() will want the ability to output warnings in the event that the path doesn't exist. Introduce the READ_ONELINER_WARN_MISSING flag which, if active, would issue a
- warning when a file doesn't exist by skipping the !file_exists() check
- and letting strbuf_read_file() handle that case.
+ warning when a file doesn't exist by always executing warning_errno()
+ in the case where strbuf_read_file() fails.
## sequencer.c ##
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *filename,
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *f
+#define READ_ONELINER_WARN_MISSING (1 << 1)
/*
- * Resets a strbuf then reads a file that was presumably written by a shell
+ * Reads a file that was presumably written by a shell script, i.e. with an @@ sequencer.c: static int read_oneliner(struct strbuf *buf,
- {
- strbuf_reset(buf);
+ int orig_len = buf->len;
+
if (strbuf_read_file(buf, path, 0) < 0) {
- if (errno != ENOENT && errno != ENOTDIR)
+ if ((flags & READ_ONELINER_WARN_MISSING) ||
-+ (errno != ENOENT && errno != ENOTDIR))
++ (errno != ENOENT && errno != ENOTDIR))
warning_errno(_("could not read '%s'"), path);
return 0;
}
8: 6c3c37994b ! 7: 50e93770e7 sequencer: make read_oneliner() extern
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *f
-#define READ_ONELINER_WARN_MISSING (1 << 1)
-
-/*
-- * Resets a strbuf then reads a file that was presumably written by a shell -- * script, i.e. with an end-of-line marker that needs to be stripped. +- * Reads a file that was presumably written by a shell script, i.e. with an +- * end-of-line marker that needs to be stripped.
- *
- * Note that only the last end-of-line marker is stripped, consistent with the
- * behavior of "$(cat path)" in a shell script.
@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *f
+int read_oneliner(struct strbuf *buf,
const char *path, unsigned flags)
{
- strbuf_reset(buf);
+ int orig_len = buf->len;
## sequencer.h ##
@@ sequencer.h: void print_commit_summary(struct repository *repo,
@@ sequencer.h: void print_commit_summary(struct repository *repo,
+#define READ_ONELINER_WARN_MISSING (1 << 1)
+
+/*
-+ * Resets a strbuf then reads a file that was presumably written by a shell -+ * script, i.e. with an end-of-line marker that needs to be stripped. ++ * Reads a file that was presumably written by a shell script, i.e. with an ++ * end-of-line marker that needs to be stripped.
+ *
+ * Note that only the last end-of-line marker is stripped, consistent with the
+ * behavior of "$(cat path)" in a shell script.
9: 689f34a2a5 ! 8: 0cc279fc14 rebase: use read_oneliner()
@@ Commit message
Since in sequencer.c, read_one() basically duplicates the functionality
of read_oneliner(), reduce code duplication by replacing read_one() with
- read_oneliner(). Also, delete strbuf_reset() calls prior to
- read_oneliner() as read_oneliner() already resets the strbuf.
+ read_oneliner().
- This was mostly done with the following Coccinelle script
+ This was done with the following Coccinelle script
@@
expression a, b;
@@ Commit message
- read_one(a, b)
+ !read_oneliner(b, a, READ_ONELINER_WARN_NON_EXISTENCE)
- Long lines were broken up and strbuf_reset()s were deleted manually.
+ and long lines were manually broken up.
## builtin/rebase.c ##
@@ builtin/rebase.c: static const char *state_dir_path(const char *filename, struct rebase_options *o
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
opts->head_name = starts_with(head_name.buf, "refs/") ?
xstrdup(head_name.buf) : NULL;
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
- * head. Fall back to reading from head to cover for the case that the
- * user upgraded git with an ongoing interactive rebase.
*/
-- strbuf_reset(&buf);
+ strbuf_reset(&buf);
if (file_exists(state_dir_path("orig-head", opts))) {
- if (read_one(state_dir_path("orig-head", opts), &buf))
+ if (!read_oneliner(&buf, state_dir_path("orig-head", opts),
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
if (get_oid(buf.buf, &opts->orig_head))
return error(_("invalid orig-head: '%s'"), buf.buf);
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
- }
if (file_exists(state_dir_path("allow_rerere_autoupdate", opts))) {
-- strbuf_reset(&buf);
+ strbuf_reset(&buf);
- if (read_one(state_dir_path("allow_rerere_autoupdate", opts),
- &buf))
+ if (!read_oneliner(&buf, state_dir_path("allow_rerere_autoupdate", opts),
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
if (!strcmp(buf.buf, "--rerere-autoupdate"))
opts->allow_rerere_autoupdate = RERERE_AUTOUPDATE;
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
- }
if (file_exists(state_dir_path("gpg_sign_opt", opts))) {
-- strbuf_reset(&buf);
+ strbuf_reset(&buf);
- if (read_one(state_dir_path("gpg_sign_opt", opts),
- &buf))
+ if (!read_oneliner(&buf, state_dir_path("gpg_sign_opt", opts),
@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
return -1;
free(opts->gpg_sign_opt);
opts->gpg_sign_opt = xstrdup(buf.buf);
- }
+@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
if (file_exists(state_dir_path("strategy", opts))) {
-- strbuf_reset(&buf);
+ strbuf_reset(&buf);
- if (read_one(state_dir_path("strategy", opts), &buf))
+ if (!read_oneliner(&buf, state_dir_path("strategy", opts),
+ READ_ONELINER_WARN_MISSING))
return -1;
free(opts->strategy);
opts->strategy = xstrdup(buf.buf);
- }
+@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
if (file_exists(state_dir_path("strategy_opts", opts))) {
-- strbuf_reset(&buf);
+ strbuf_reset(&buf);
- if (read_one(state_dir_path("strategy_opts", opts), &buf))
+ if (!read_oneliner(&buf, state_dir_path("strategy_opts", opts),
+ READ_ONELINER_WARN_MISSING))10: e0b8e409af ! 9: 3dbed2a832 sequencer: make apply_rebase() accept a path
@@ Metadata
Author: Denton Liu <[email protected]>
## Commit message ##
- sequencer: make apply_rebase() accept a path
+ sequencer: make apply_autostash() accept a path
- In order to make apply_rebase() more generic for future extraction, make
+ In order to make apply_autostash() more generic for future extraction, make
it accept a path argument so that the location from where to read the reference to the autostash commit can be customized. Remove the opts argument since it was unused before anyway.
11: a52583beff = 10: 937b4964d1 sequencer: rename stash_sha1 to stash_oid 12: 389b17df33 = 11: 98a7f5280c rebase: use apply_autostash() from sequencer.c 13: fe57e21dbc = 12: 67f3b5c225 rebase: generify reset_head() 14: 045668620d = 13: af5e808667 reset: extract reset_head() from rebase 15: d5f51cd80e = 14: 114d9a7655 rebase: extract create_autostash() 16: 9ab10d23d4 = 15: c2ed4cd39d rebase: generify create_autostash() 17: 26cca49be6 = 16: 3be4a27dfe sequencer: extract perform_autostash() from rebase 18: e703022fda = 17: 129d5d97ae sequencer: unlink autostash in apply_autostash() 19: 75dc3f10a1 ! 18: 7e04ce8d8e sequencer: implement save_autostash()
@@ Commit message
function will be used in a future commit.
The difference between save_autostash() and apply_autostash() is that
- the latter does not try to apply the stash. It skips that step and
+ the former does not try to apply the stash. It skips that step and
just stores the created entry in the stash reflog.
+ This is useful in the case where we abort an operation when an autostash
+ is present but we don't want to dirty the worktree with the application
+ of the stash. For example, in a future commit, we will implement
+ git merge --autostash. Since merges can be aborted using
+ git reset --hard, we'd make use of save_autostash() to save the
+ autostash entry instead of applying it to the worktree thus keeping the
+ worktree undirtied.
+
## sequencer.c ##
@@ sequencer.c: void create_autostash(struct repository *r, const char *path,
strbuf_release(&buf);
20: 598ddea6c1 = 19: 732b3f9945 sequencer: implement apply_autostash_oid() 21: 7adf794192 ! 20: f9f698b79a merge: teach --autostash option
@@ Commit message
git reset --hard, the autostash is saved into the stash reflog instead keeping the worktree clean.
+ Helped-by: Phillip Wood <[email protected]>
## Documentation/config/merge.txt ##
@@ Documentation/git-merge.txt: will be appended to the specified message.
Abort the current conflict resolution process, and
- try to reconstruct the pre-merge state.
+ try to reconstruct the pre-merge state. If an autostash entry is
-+ present, apply it back to the worktree.
++ present, apply it to the worktree.
+
If there were uncommitted worktree changes present when the merge started, 'git merge --abort' will in some cases be unable to
@@ Documentation/git-merge.txt: reconstruct these changes. It is therefore recommen
+
'git merge --abort' is equivalent to 'git reset --merge' when
-MERGE_HEAD is present.
-+MERGE_HEAD is present unless MERGE_AUTOSTASH is also present where -+the stash entry is applied to the worktree.
++MERGE_HEAD is present unless MERGE_AUTOSTASH is also present in ++which case 'git merge --abort' applies the stash entry to the worktree ++whereas 'git reset --merge' will save the stashed changes in the stash ++reflog.
--quit::
Forget about the current merge in progress. Leave the index
+- and the working tree as-is.
++ and the working tree as-is. If MERGE_AUTOSTASH is present, the
++ stash entry will be saved to the stash reflog.
+
+ --continue::
+ After a 'git merge' stops due to conflicts you can conclude the
## Documentation/merge-options.txt ##
@@ Documentation/merge-options.txt: ifndef::git-pull[]
@@ builtin/merge.c: int cmd_merge(int argc, const char **argv, const char *prefix)
die(_("There is no merge to abort (MERGE_HEAD missing)."));
+ if (read_oneliner(&stash_oid, git_path_merge_autostash(the_repository),
-+ READ_ONELINER_SKIP_IF_EMPTY)) {
-+ strbuf_trim(&stash_oid);
++ READ_ONELINER_SKIP_IF_EMPTY))
+ unlink(git_path_merge_autostash(the_repository));
-+ }
+
/* Invoke 'git reset --merge' */
ret = cmd_reset(nargc, nargv, prefix);
@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix
N_("add exec lines after each commit of the "
"editable list")),
- ## builtin/reset.c ##
-@@
- #include "cache-tree.h"
- #include "submodule.h"
- #include "submodule-config.h"
-+#include "sequencer.h"
-
- #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
-
-@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)
- if (reset_type == HARD && !update_ref_status && !quiet)
- print_new_head_line(lookup_commit_reference(the_repository, &oid));
- }
-- if (!pathspec.nr)
-+ if (!pathspec.nr) {
- remove_branch_state(the_repository, 0);
-+ }
-
- return update_ref_status;
- }
-
## parse-options.h ##
@@ parse-options.h: int parse_opt_passthru_argv(const struct option *, const char *, int);
#define OPT_CLEANUP(v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))
@@ t/t7600-merge.sh: test_expect_success 'refresh the index before merging' '
+ git diff --exit-code
+'
+
++test_expect_success 'quit merge with --no-commit and --autostash' '
++ git reset --hard c1 &&
++ git merge-file file file.orig file.9 &&
++ git diff >expect &&
++ git merge --no-commit --autostash c2 &&
++ git stash show -p MERGE_AUTOSTASH >actual &&
++ test_cmp expect actual &&
++ git diff HEAD >expect &&
++ git merge --quit 2>err &&
++ test_i18ngrep "Autostash exists; creating a new stash entry." err &&
++ git diff HEAD >actual &&
++ test_cmp expect actual
++'
++
+test_expect_success 'merge with conflicted --autostash changes' '
+ git reset --hard c1 &&
+ git merge-file file file.orig file.9y &&
22: f1e54622fb = 21: 13f3fadbef t5520: make test_pull_autostash() accept expect_parent_num 23: 177c7e537b = 22: f4fec1e780 pull: pass --autostash to merge
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 1/22 Makefile: ASCII-sort += lists
There are many += lists in the Makefile and, over time, they have gotten slightly out of ASCII order. Sort all += lists to bring them back in order.
ASCII sorting was chosen over strict alphabetical order even though, if we omit file prefixes, the lists aren't sorted in strictly alphabetical order (e.g. archive.o comes after archive-zip.o instead of before archive-tar.o). This is intentional because the purpose of maintaining the sorted list is to ensure line insertions are deterministic. By using ASCII ordering, it is more easily mechanically reproducible in the future, such as by using :sort in Vim.
This patch is best viewed with --color-moved.
--- Makefile | 77 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 38 deletions(-)
Makefile +39 -38
@@ -604,12 +604,12 @@ unexport CDPATH SCRIPT_SH += git-bisect.sh SCRIPT_SH += git-difftool--helper.sh SCRIPT_SH += git-filter-branch.sh +SCRIPT_SH += git-legacy-stash.sh SCRIPT_SH += git-merge-octopus.sh SCRIPT_SH += git-merge-one-file.sh SCRIPT_SH += git-merge-resolve.sh SCRIPT_SH += git-mergetool.sh SCRIPT_SH += git-quiltimport.sh -SCRIPT_SH += git-legacy-stash.sh SCRIPT_SH += git-request-pull.sh SCRIPT_SH += git-submodule.sh SCRIPT_SH += git-web--browse.sh
@@ -617,8 +617,8 @@ SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--preserve-merges -SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n +SCRIPT_LIB += git-sh-setup SCRIPT_PERL += git-add--interactive.perl SCRIPT_PERL += git-archimport.perl
@@ -686,9 +686,9 @@ PROGRAM_OBJS += daemon.o PROGRAM_OBJS += fast-import.o PROGRAM_OBJS += http-backend.o PROGRAM_OBJS += imap-send.o +PROGRAM_OBJS += remote-testsvn.o PROGRAM_OBJS += sh-i18n--envsubst.o PROGRAM_OBJS += shell.o -PROGRAM_OBJS += remote-testsvn.o # Binary suffix, set to .exe for Windows builds X =
@@ -709,9 +709,9 @@ TEST_BUILTINS_OBJS += test-dump-untracked-cache.o TEST_BUILTINS_OBJS += test-example-decorate.o TEST_BUILTINS_OBJS += test-genrandom.o TEST_BUILTINS_OBJS += test-genzeros.o +TEST_BUILTINS_OBJS += test-hash-speed.o TEST_BUILTINS_OBJS += test-hash.o TEST_BUILTINS_OBJS += test-hashmap.o -TEST_BUILTINS_OBJS += test-hash-speed.o TEST_BUILTINS_OBJS += test-index-version.o TEST_BUILTINS_OBJS += test-json-writer.o TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
@@ -737,8 +737,8 @@ TEST_BUILTINS_OBJS += test-revision-walking.o TEST_BUILTINS_OBJS += test-run-command.o TEST_BUILTINS_OBJS += test-scrap-cache-tree.o TEST_BUILTINS_OBJS += test-serve-v2.o -TEST_BUILTINS_OBJS += test-sha1.o TEST_BUILTINS_OBJS += test-sha1-array.o +TEST_BUILTINS_OBJS += test-sha1.o TEST_BUILTINS_OBJS += test-sha256.o TEST_BUILTINS_OBJS += test-sigchain.o TEST_BUILTINS_OBJS += test-strcmp-offset.o
@@ -748,10 +748,10 @@ TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o TEST_BUILTINS_OBJS += test-subprocess.o TEST_BUILTINS_OBJS += test-trace2.o TEST_BUILTINS_OBJS += test-urlmatch-normalization.o -TEST_BUILTINS_OBJS += test-xml-encode.o TEST_BUILTINS_OBJS += test-wildmatch.o TEST_BUILTINS_OBJS += test-windows-named-pipe.o TEST_BUILTINS_OBJS += test-write-cache.o +TEST_BUILTINS_OBJS += test-xml-encode.o # Do not add more tests here unless they have extra dependencies. Add # them in TEST_BUILTINS_OBJS above.
@@ -788,10 +788,10 @@ OTHER_PROGRAMS = git$X # what test wrappers are needed and 'install' will install, in bindir BINDIR_PROGRAMS_NEED_X += git -BINDIR_PROGRAMS_NEED_X += git-upload-pack BINDIR_PROGRAMS_NEED_X += git-receive-pack -BINDIR_PROGRAMS_NEED_X += git-upload-archive BINDIR_PROGRAMS_NEED_X += git-shell +BINDIR_PROGRAMS_NEED_X += git-upload-archive +BINDIR_PROGRAMS_NEED_X += git-upload-pack BINDIR_PROGRAMS_NO_X += git-cvsserver
@@ -831,9 +831,9 @@ LIB_OBJS += advice.o LIB_OBJS += alias.o LIB_OBJS += alloc.o LIB_OBJS += apply.o -LIB_OBJS += archive.o LIB_OBJS += archive-tar.o LIB_OBJS += archive-zip.o +LIB_OBJS += archive.o LIB_OBJS += argv-array.o LIB_OBJS += attr.o LIB_OBJS += base85.o
@@ -849,9 +849,9 @@ LIB_OBJS += checkout.o LIB_OBJS += color.o LIB_OBJS += column.o LIB_OBJS += combine-diff.o -LIB_OBJS += commit.o LIB_OBJS += commit-graph.o LIB_OBJS += commit-reach.o +LIB_OBJS += commit.o LIB_OBJS += compat/obstack.o LIB_OBJS += compat/terminal.o LIB_OBJS += config.o
@@ -865,17 +865,17 @@ LIB_OBJS += ctype.o LIB_OBJS += date.o LIB_OBJS += decorate.o LIB_OBJS += delta-islands.o +LIB_OBJS += diff-delta.o +LIB_OBJS += diff-lib.o +LIB_OBJS += diff-no-index.o +LIB_OBJS += diff.o LIB_OBJS += diffcore-break.o LIB_OBJS += diffcore-delta.o LIB_OBJS += diffcore-order.o LIB_OBJS += diffcore-pickaxe.o LIB_OBJS += diffcore-rename.o -LIB_OBJS += diff-delta.o -LIB_OBJS += diff-lib.o -LIB_OBJS += diff-no-index.o -LIB_OBJS += diff.o -LIB_OBJS += dir.o LIB_OBJS += dir-iterator.o +LIB_OBJS += dir.o LIB_OBJS += editor.o LIB_OBJS += entry.o LIB_OBJS += environment.o
@@ -893,7 +893,6 @@ LIB_OBJS += gpg-interface.o LIB_OBJS += graph.o LIB_OBJS += grep.o LIB_OBJS += hashmap.o -LIB_OBJS += linear-assignment.o LIB_OBJS += help.o LIB_OBJS += hex.o LIB_OBJS += ident.o
@@ -903,9 +902,10 @@ LIB_OBJS += kwset.o LIB_OBJS += levenshtein.o LIB_OBJS += line-log.o LIB_OBJS += line-range.o -LIB_OBJS += list-objects.o -LIB_OBJS += list-objects-filter.o +LIB_OBJS += linear-assignment.o LIB_OBJS += list-objects-filter-options.o +LIB_OBJS += list-objects-filter.o +LIB_OBJS += list-objects.o LIB_OBJS += ll-merge.o LIB_OBJS += lockfile.o LIB_OBJS += log-tree.o
@@ -914,31 +914,31 @@ LIB_OBJS += mailinfo.o LIB_OBJS += mailmap.o LIB_OBJS += match-trees.o LIB_OBJS += mem-pool.o -LIB_OBJS += merge.o LIB_OBJS += merge-blobs.o LIB_OBJS += merge-recursive.o +LIB_OBJS += merge.o LIB_OBJS += mergesort.o LIB_OBJS += midx.o LIB_OBJS += name-hash.o LIB_OBJS += negotiator/default.o LIB_OBJS += negotiator/skipping.o -LIB_OBJS += notes.o LIB_OBJS += notes-cache.o LIB_OBJS += notes-merge.o LIB_OBJS += notes-utils.o +LIB_OBJS += notes.o LIB_OBJS += object.o LIB_OBJS += oidmap.o LIB_OBJS += oidset.o -LIB_OBJS += packfile.o -LIB_OBJS += pack-bitmap.o LIB_OBJS += pack-bitmap-write.o +LIB_OBJS += pack-bitmap.o LIB_OBJS += pack-check.o LIB_OBJS += pack-objects.o LIB_OBJS += pack-revindex.o LIB_OBJS += pack-write.o +LIB_OBJS += packfile.o LIB_OBJS += pager.o -LIB_OBJS += parse-options.o LIB_OBJS += parse-options-cb.o +LIB_OBJS += parse-options.o LIB_OBJS += patch-delta.o LIB_OBJS += patch-ids.o LIB_OBJS += path.o
@@ -955,8 +955,9 @@ LIB_OBJS += quote.o LIB_OBJS += range-diff.o LIB_OBJS += reachable.o LIB_OBJS += read-cache.o -LIB_OBJS += rebase.o LIB_OBJS += rebase-interactive.o +LIB_OBJS += rebase.o +LIB_OBJS += ref-filter.o LIB_OBJS += reflog-walk.o LIB_OBJS += refs.o LIB_OBJS += refs/files-backend.o
@@ -964,7 +965,6 @@ LIB_OBJS += refs/iterator.o LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += refspec.o -LIB_OBJS += ref-filter.o LIB_OBJS += remote.o LIB_OBJS += replace-object.o LIB_OBJS += repo-settings.o
@@ -979,8 +979,8 @@ LIB_OBJS += serve.o LIB_OBJS += server-info.o LIB_OBJS += setup.o LIB_OBJS += sha1-array.o -LIB_OBJS += sha1-lookup.o LIB_OBJS += sha1-file.o +LIB_OBJS += sha1-lookup.o LIB_OBJS += sha1-name.o LIB_OBJS += shallow.o LIB_OBJS += sideband.o
@@ -990,9 +990,9 @@ LIB_OBJS += stable-qsort.o LIB_OBJS += strbuf.o LIB_OBJS += streaming.o LIB_OBJS += string-list.o -LIB_OBJS += submodule.o -LIB_OBJS += submodule-config.o LIB_OBJS += sub-process.o +LIB_OBJS += submodule-config.o +LIB_OBJS += submodule.o LIB_OBJS += symlinks.o LIB_OBJS += tag.o LIB_OBJS += tempfile.o
@@ -1011,11 +1011,11 @@ LIB_OBJS += trace2/tr2_tgt_normal.o LIB_OBJS += trace2/tr2_tgt_perf.o LIB_OBJS += trace2/tr2_tls.o LIB_OBJS += trailer.o -LIB_OBJS += transport.o LIB_OBJS += transport-helper.o +LIB_OBJS += transport.o LIB_OBJS += tree-diff.o -LIB_OBJS += tree.o LIB_OBJS += tree-walk.o +LIB_OBJS += tree.o LIB_OBJS += unpack-trees.o LIB_OBJS += upload-pack.o LIB_OBJS += url.o
@@ -1055,9 +1055,9 @@ BUILTIN_OBJS += builtin/checkout.o BUILTIN_OBJS += builtin/clean.o BUILTIN_OBJS += builtin/clone.o BUILTIN_OBJS += builtin/column.o +BUILTIN_OBJS += builtin/commit-graph.o BUILTIN_OBJS += builtin/commit-tree.o BUILTIN_OBJS += builtin/commit.o -BUILTIN_OBJS += builtin/commit-graph.o BUILTIN_OBJS += builtin/config.o BUILTIN_OBJS += builtin/count-objects.o BUILTIN_OBJS += builtin/credential.o
@@ -1088,13 +1088,13 @@ BUILTIN_OBJS += builtin/ls-remote.o BUILTIN_OBJS += builtin/ls-tree.o BUILTIN_OBJS += builtin/mailinfo.o BUILTIN_OBJS += builtin/mailsplit.o -BUILTIN_OBJS += builtin/merge.o BUILTIN_OBJS += builtin/merge-base.o BUILTIN_OBJS += builtin/merge-file.o BUILTIN_OBJS += builtin/merge-index.o BUILTIN_OBJS += builtin/merge-ours.o BUILTIN_OBJS += builtin/merge-recursive.o BUILTIN_OBJS += builtin/merge-tree.o +BUILTIN_OBJS += builtin/merge.o BUILTIN_OBJS += builtin/mktag.o BUILTIN_OBJS += builtin/mktree.o BUILTIN_OBJS += builtin/multi-pack-index.o
@@ -1114,9 +1114,9 @@ BUILTIN_OBJS += builtin/read-tree.o BUILTIN_OBJS += builtin/rebase.o BUILTIN_OBJS += builtin/receive-pack.o BUILTIN_OBJS += builtin/reflog.o -BUILTIN_OBJS += builtin/remote.o BUILTIN_OBJS += builtin/remote-ext.o BUILTIN_OBJS += builtin/remote-fd.o +BUILTIN_OBJS += builtin/remote.o BUILTIN_OBJS += builtin/repack.o BUILTIN_OBJS += builtin/replace.o BUILTIN_OBJS += builtin/rerere.o
@@ -2335,16 +2335,16 @@ reconfigure config.mak.autogen: config.status endif XDIFF_OBJS += xdiff/xdiffi.o -XDIFF_OBJS += xdiff/xprepare.o -XDIFF_OBJS += xdiff/xutils.o XDIFF_OBJS += xdiff/xemit.o +XDIFF_OBJS += xdiff/xhistogram.o XDIFF_OBJS += xdiff/xmerge.o XDIFF_OBJS += xdiff/xpatience.o -XDIFF_OBJS += xdiff/xhistogram.o +XDIFF_OBJS += xdiff/xprepare.o +XDIFF_OBJS += xdiff/xutils.o +VCSSVN_OBJS += vcs-svn/fast_export.o VCSSVN_OBJS += vcs-svn/line_buffer.o VCSSVN_OBJS += vcs-svn/sliding_window.o -VCSSVN_OBJS += vcs-svn/fast_export.o VCSSVN_OBJS += vcs-svn/svndiff.o VCSSVN_OBJS += vcs-svn/svndump.o
@@ -3152,9 +3152,10 @@ endif # ALL_COMMANDS = $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) ALL_COMMANDS += git +ALL_COMMANDS += git-citool +ALL_COMMANDS += git-gui ALL_COMMANDS += gitk ALL_COMMANDS += gitweb -ALL_COMMANDS += git-gui git-citool .PHONY: check-docs check-docs::
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 2/22 t7600: use test_write_lines()
In reply to Denton LiuIn t7600, we were rewriting printf '%s\n' ... to create files from parameters, one per line. However, we already have a function that wraps this for us: test_write_lines(). Rewrite these instances to use that function instead of open coding it.
--- t/t7600-merge.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
t/t7600-merge.sh +11 -11
@@ -29,15 +29,15 @@ Testing basic merge operations/option parsing. . ./test-lib.sh . "$TEST_DIRECTORY"/lib-gpg.sh -printf '%s\n' 1 2 3 4 5 6 7 8 9 >file -printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >file.1 -printf '%s\n' 1 2 3 4 '5 X' 6 7 8 9 >file.5 -printf '%s\n' 1 2 3 4 5 6 7 8 '9 X' >file.9 -printf '%s\n' 1 2 3 4 5 6 7 8 '9 Y' >file.9y -printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >result.1 -printf '%s\n' '1 X' 2 3 4 '5 X' 6 7 8 9 >result.1-5 -printf '%s\n' '1 X' 2 3 4 '5 X' 6 7 8 '9 X' >result.1-5-9 -printf '%s\n' 1 2 3 4 5 6 7 8 '9 Z' >result.9z +test_write_lines 1 2 3 4 5 6 7 8 9 >file +test_write_lines '1 X' 2 3 4 5 6 7 8 9 >file.1 +test_write_lines 1 2 3 4 '5 X' 6 7 8 9 >file.5 +test_write_lines 1 2 3 4 5 6 7 8 '9 X' >file.9 +test_write_lines 1 2 3 4 5 6 7 8 '9 Y' >file.9y +test_write_lines '1 X' 2 3 4 5 6 7 8 9 >result.1 +test_write_lines '1 X' 2 3 4 '5 X' 6 7 8 9 >result.1-5 +test_write_lines '1 X' 2 3 4 '5 X' 6 7 8 '9 X' >result.1-5-9 +test_write_lines 1 2 3 4 5 6 7 8 '9 Z' >result.9z create_merge_msgs () { echo "Merge tag 'c2'" >msg.1-5 &&
@@ -81,7 +81,7 @@ verify_head () { } verify_parents () { - printf '%s\n' "$@" >parents.expected && + test_write_lines "$@" >parents.expected && >parents.actual && i=1 && while test $i -le $#
@@ -95,7 +95,7 @@ verify_parents () { } verify_mergeheads () { - printf '%s\n' "$@" >mergehead.expected && + test_write_lines "$@" >mergehead.expected && while read sha1 rest do git rev-parse $sha1
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 3/22 sequencer: stop leaking buf
In reply to Denton LiuIn read_populate_opts(), we call read_oneliner() _after_ calling strbuf_release(). This means that buf is leaked at the end of the function.
Always clean up the strbuf by going to done_rebase_i whether or not we return an error.
--- sequencer.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
sequencer.c +9 -4
@@ -2485,6 +2485,7 @@ static int read_populate_opts(struct replay_opts *opts) {if (is_rebase_i (opts)) {struct strbuf buf =STRBUF_INIT ; +int ret =0 ;if (read_oneliner (&buf,rebase_path_gpg_sign_opt (),1 )) {if (!starts_with (buf.buf,"-S" ))
@@ -2525,7 +2526,7 @@ static int read_populate_opts(struct replay_opts *opts) opts->keep_redundant_commits =1 ;read_strategy_opts (opts, &buf); -strbuf_release (&buf); +strbuf_reset (&buf);if (read_oneliner (&opts->current_fixups,rebase_path_current_fixups (),1 )) {
@@ -2538,12 +2539,16 @@ static int read_populate_opts(struct replay_opts *opts) }if (read_oneliner (&buf,rebase_path_squash_onto (),0 )) { -if (get_oid_hex (buf.buf, &opts->squash_onto) <0 ) -return error (_ ("unusable squash-onto" )); +if (get_oid_hex (buf.buf, &opts->squash_onto) <0 ) { + ret =error (_ ("unusable squash-onto" )); + goto done_rebase_i; + } opts->have_squash_onto =1 ; } -return 0 ; +done_rebase_i: +strbuf_release (&buf); +return ret; }if (!file_exists (git_path_opts_file ()))
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 4/22 sequencer: make file exists check more efficient
In reply to Denton LiuWe currently check whether a file exists and return early before reading the file. Instead of accessing the file twice, always read the file and check errno to see if the file doesn't exist.
--- sequencer.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
sequencer.c +2 -4
@@ -433,11 +433,9 @@ static int read_oneliner(struct strbuf *buf, {int orig_len = buf->len; -if (!file_exists (path)) -return 0 ; -if (strbuf_read_file (buf, path,0 ) <0 ) { -warning_errno (_ ("could not read '%s'" ), path); +if (errno !=ENOENT && errno !=ENOTDIR ) +warning_errno (_ ("could not read '%s'" ), path);return 0 ; }
Signature · 1 line
2.26.0.159.g23e2136ad0
Re: [PATCH v5 04/22] sequencer: make file exists check more efficient
Quoted 8 lines · Denton Liu
We currently check whether a file exists and return early before reading the file. Instead of accessing the file twice, always read the file and check
errnoto see if the file doesn't exist.Signed-off-by Denton Liu <[email protected]>--- sequencer.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
It's not likely for us to break strbuf_read_file() in such a way that it would clobber errno when it sees a failure from open, so this should be OK. If we were racing with somebody else who is trying to remove 'path', we may have said "ah, the file is there, let's try to read --- oops, we cannot read it after all" with the current code, but the updated code would take advantage of the atomicity of open() to avoid such race. If we can open it, we would read it unless there is an I/O error. If we failed to open it, we know the file wasn't there when we attempted to read. Good.
Quoted 17 lines
sequencer.c +2
-4@@ -433,11 +433,9 @@ static int read_oneliner(struct strbuf *buf, {int orig_len = buf->len; -if (!file_exists (path)) -return 0 ; -if (strbuf_read_file (buf, path,0 ) <0 ) { -warning_errno (_ ("could not read '%s'" ), path); +if (errno !=ENOENT && errno !=ENOTDIR ) +warning_errno (_ ("could not read '%s'" ), path);return 0 ; }
PATCH v5 5/22 sequencer: make read_oneliner() accept flags
In reply to Denton LiuIn a future commit, we will need read_oneliner() to accept flags other than just skip_if_empty. Instead of having an argument for each flag, teach read_oneliner() to accept the bitfield flags instead. For now, only recognize the READ_ONELINER_SKIP_IF_EMPTY flag. More flags will be added in a future commit.
The result of this is that parallel topics which introduce invocations of read_oneliner() will still be compatible with this new function signature since, instead of passing 1 or 0 for skip_if_empty, they'll be passing 1 or 0 to flags, which gives equivalent behavior.
Mechanically fix up invocations of read_oneliner() with the following spatch
@@ expression a, b; @@ read_oneliner(a, b, - 1 + READ_ONELINER_SKIP_IF_EMPTY )
and manually break up long lines in the result.
--- sequencer.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
sequencer.c +14 -7
@@ -419,6 +419,8 @@ static int write_message(const void *buf, size_t len, const char *filename,return 0 ; } +#define READ_ONELINER_SKIP_IF_EMPTY (1 << 0) +/* * Reads a file that was presumably written by a shell script, i.e. with an * end-of-line marker that needs to be stripped.
@@ -429,7 +431,7 @@ static int write_message(const void *buf, size_t len, const char *filename,* Returns 1 if the file was read, 0 if it could not be read or does not exist. */ static int read_oneliner (struct strbuf *buf, -const char *path,int skip_if_empty) +const char *path,unsigned flags) {int orig_len = buf->len;
@@ -445,7 +447,7 @@ static int read_oneliner(struct strbuf *buf, buf->buf[buf->len] ='\0' ; } -if (skip_if_empty && buf->len == orig_len) +if ((flags &READ_ONELINER_SKIP_IF_EMPTY ) && buf->len == orig_len)return 0 ;return 1 ;
@@ -2485,7 +2487,8 @@ static int read_populate_opts(struct replay_opts *opts)struct strbuf buf =STRBUF_INIT ;int ret =0 ; -if (read_oneliner (&buf,rebase_path_gpg_sign_opt (),1 )) { +if (read_oneliner (&buf,rebase_path_gpg_sign_opt (), +READ_ONELINER_SKIP_IF_EMPTY )) {if (!starts_with (buf.buf,"-S" ))strbuf_reset (&buf);else {
@@ -2495,7 +2498,8 @@ static int read_populate_opts(struct replay_opts *opts)strbuf_reset (&buf); } -if (read_oneliner (&buf,rebase_path_allow_rerere_autoupdate (),1 )) { +if (read_oneliner (&buf,rebase_path_allow_rerere_autoupdate (), +READ_ONELINER_SKIP_IF_EMPTY )) {if (!strcmp (buf.buf,"--rerere-autoupdate" )) opts->allow_rerere_auto =RERERE_AUTOUPDATE ;else if (!strcmp (buf.buf,"--no-rerere-autoupdate" ))
@@ -2527,7 +2531,8 @@ static int read_populate_opts(struct replay_opts *opts)strbuf_reset (&buf);if (read_oneliner (&opts->current_fixups, -rebase_path_current_fixups (),1 )) { +rebase_path_current_fixups (), +READ_ONELINER_SKIP_IF_EMPTY )) {const char *p = opts->current_fixups.buf; opts->current_fixup_count =1 ;while ((p =strchr (p,'\n' ))) {
@@ -3668,7 +3673,8 @@ static int apply_autostash(struct replay_opts *opts)struct child_process child =CHILD_PROCESS_INIT ;int ret =0 ; -if (!read_oneliner (&stash_sha1,rebase_path_autostash (),1 )) { +if (!read_oneliner (&stash_sha1,rebase_path_autostash (), +READ_ONELINER_SKIP_IF_EMPTY )) {strbuf_release (&stash_sha1);return 0 ; }
@@ -4292,7 +4298,8 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)struct strbuf buf =STRBUF_INIT ;struct object_id oid; -if (read_oneliner (&buf,rebase_path_stopped_sha (),1 ) && +if (read_oneliner (&buf,rebase_path_stopped_sha (), +READ_ONELINER_SKIP_IF_EMPTY ) && !get_oid_committish (buf.buf, &oid))record_in_rewritten (&oid,peek_command (&todo_list,0 ));strbuf_release (&buf);
Signature · 1 line
2.26.0.159.g23e2136ad0
Re: [PATCH v5 05/22] sequencer: make read_oneliner() accept flags
Quoted 5 lines · Denton Liu
In a future commit, we will need
read_oneliner()to accept flags other than justskip_if_empty. Instead of having an argument for each flag, teachread_oneliner()to accept the bitfieldflagsinstead. For now, only recognize theREAD_ONELINER_SKIP_IF_EMPTYflag. More flags will be added in a future commit.
Good move. Looked sensible.
Thanks.
PATCH v5 6/22 sequencer: configurably warn on non-existent files
In reply to Denton LiuIn the future, we plan on externing read_oneliner(). Future users of read_oneliner() will want the ability to output warnings in the event that the path doesn't exist. Introduce the READ_ONELINER_WARN_MISSING flag which, if active, would issue a warning when a file doesn't exist by always executing warning_errno() in the case where strbuf_read_file() fails.
--- sequencer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
sequencer.c +3 -1
@@ -420,6 +420,7 @@ static int write_message(const void *buf, size_t len, const char *filename, }#define READ_ONELINER_SKIP_IF_EMPTY (1 << 0) +#define READ_ONELINER_WARN_MISSING (1 << 1)/* * Reads a file that was presumably written by a shell script, i.e. with an
@@ -436,7 +437,8 @@ static int read_oneliner(struct strbuf *buf,int orig_len = buf->len;if (strbuf_read_file (buf, path,0 ) <0 ) { -if (errno !=ENOENT && errno !=ENOTDIR ) +if ((flags &READ_ONELINER_WARN_MISSING ) || + (errno !=ENOENT && errno !=ENOTDIR ))warning_errno (_ ("could not read '%s'" ), path);return 0 ; }
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 7/22 sequencer: make read_oneliner() extern
In reply to Denton LiuThe function read_oneliner() is a generally useful util function. Instead of hiding it as a static function within sequencer.c, extern it so that it can be reused by others.
This patch is best viewed with --color-moved.
--- sequencer.c | 14 +------------- sequencer.h | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 13 deletions(-)
sequencer.c +1 -13
@@ -419,19 +419,7 @@ static int write_message(const void *buf, size_t len, const char *filename,return 0 ; } -#define READ_ONELINER_SKIP_IF_EMPTY (1 << 0) -#define READ_ONELINER_WARN_MISSING (1 << 1) - -/* -* Reads a file that was presumably written by a shell script, i.e. with an -* end-of-line marker that needs to be stripped. -* -* Note that only the last end-of-line marker is stripped, consistent with the -* behavior of "$(cat path)" in a shell script. -* -* Returns 1 if the file was read, 0 if it could not be read or does not exist. -*/ -static int read_oneliner (struct strbuf *buf, +int read_oneliner (struct strbuf *buf,const char *path,unsigned flags) {int orig_len = buf->len;
sequencer.h +14 -0
@@ -198,6 +198,20 @@ void print_commit_summary(struct repository *repo,const struct object_id *oid, unsigned int flags); +#define READ_ONELINER_SKIP_IF_EMPTY (1 << 0) +#define READ_ONELINER_WARN_MISSING (1 << 1) + +/* +* Reads a file that was presumably written by a shell script, i.e. with an +* end-of-line marker that needs to be stripped. +* +* Note that only the last end-of-line marker is stripped, consistent with the +* behavior of "$(cat path)" in a shell script. +* +* Returns 1 if the file was read, 0 if it could not be read or does not exist. +*/ +int read_oneliner (struct strbuf *buf, +const char *path,unsigned flags);int read_author_script (const char *path,char **name,char **email,char **date,int allow_missing);void parse_strategy_opts (struct replay_opts *opts,char *raw_opts);
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 8/22 rebase: use read_oneliner()
In reply to Denton LiuSince in sequencer.c, read_one() basically duplicates the functionality of read_oneliner(), reduce code duplication by replacing read_one() with read_oneliner().
This was done with the following Coccinelle script
@@ expression a, b; @@ - read_one(a, b) + !read_oneliner(b, a, READ_ONELINER_WARN_NON_EXISTENCE)
and long lines were manually broken up.
--- builtin/rebase.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)
builtin/rebase.c +17 -20
@@ -586,15 +586,6 @@ static const char *state_dir_path(const char *filename, struct rebase_options *oreturn path.buf; } -/* Read one file, then strip line endings */ -static int read_one (const char *path,struct strbuf *buf) -{ -if (strbuf_read_file (buf, path,0 ) <0 ) -return error_errno (_ ("could not read '%s'" ), path); -strbuf_trim_trailing_newline (buf); -return 0 ; -} -/* Initialize the rebase options from the state directory. */ static int read_basic_state (struct rebase_options *opts) {
@@ -602,8 +593,10 @@ static int read_basic_state(struct rebase_options *opts)struct strbuf buf =STRBUF_INIT ;struct object_id oid; -if (read_one (state_dir_path ("head-name" , opts), &head_name) || -read_one (state_dir_path ("onto" , opts), &buf)) +if (!read_oneliner (&head_name,state_dir_path ("head-name" , opts), +READ_ONELINER_WARN_MISSING ) || + !read_oneliner (&buf,state_dir_path ("onto" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ; opts->head_name =starts_with (head_name.buf,"refs/" ) ?xstrdup (head_name.buf) :NULL ;
@@ -619,9 +612,11 @@ static int read_basic_state(struct rebase_options *opts)*/ strbuf_reset (&buf);if (file_exists (state_dir_path ("orig-head" , opts))) { -if (read_one (state_dir_path ("orig-head" , opts), &buf)) +if (!read_oneliner (&buf,state_dir_path ("orig-head" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ; - }else if (read_one (state_dir_path ("head" , opts), &buf)) + }else if (!read_oneliner (&buf,state_dir_path ("head" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ;if (get_oid (buf.buf, &opts->orig_head))return error (_ ("invalid orig-head: '%s'" ), buf.buf);
@@ -641,8 +636,8 @@ static int read_basic_state(struct rebase_options *opts)if (file_exists (state_dir_path ("allow_rerere_autoupdate" , opts))) {strbuf_reset (&buf); -if (read_one (state_dir_path ("allow_rerere_autoupdate" , opts), - &buf)) +if (!read_oneliner (&buf,state_dir_path ("allow_rerere_autoupdate" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ;if (!strcmp (buf.buf,"--rerere-autoupdate" )) opts->allow_rerere_autoupdate =RERERE_AUTOUPDATE ;
@@ -655,8 +650,8 @@ static int read_basic_state(struct rebase_options *opts)if (file_exists (state_dir_path ("gpg_sign_opt" , opts))) {strbuf_reset (&buf); -if (read_one (state_dir_path ("gpg_sign_opt" , opts), - &buf)) +if (!read_oneliner (&buf,state_dir_path ("gpg_sign_opt" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ;free (opts->gpg_sign_opt); opts->gpg_sign_opt =xstrdup (buf.buf);
@@ -664,7 +659,8 @@ static int read_basic_state(struct rebase_options *opts)if (file_exists (state_dir_path ("strategy" , opts))) {strbuf_reset (&buf); -if (read_one (state_dir_path ("strategy" , opts), &buf)) +if (!read_oneliner (&buf,state_dir_path ("strategy" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ;free (opts->strategy); opts->strategy =xstrdup (buf.buf);
@@ -672,7 +668,8 @@ static int read_basic_state(struct rebase_options *opts)if (file_exists (state_dir_path ("strategy_opts" , opts))) {strbuf_reset (&buf); -if (read_one (state_dir_path ("strategy_opts" , opts), &buf)) +if (!read_oneliner (&buf,state_dir_path ("strategy_opts" , opts), +READ_ONELINER_WARN_MISSING ))return -1 ;free (opts->strategy_opts); opts->strategy_opts =xstrdup (buf.buf);
@@ -724,7 +721,7 @@ static int apply_autostash(struct rebase_options *opts)if (!file_exists (path))return 0 ; -if (read_one (path, &autostash)) +if (!read_oneliner (&autostash, path,READ_ONELINER_WARN_MISSING ))return error (_ ("Could not read '%s'" ), path);/* Ensure that the hash is not mistaken for a number */ strbuf_addstr (&autostash,"^0" );
Signature · 1 line
2.26.0.159.g23e2136ad0
Re: [PATCH v5 08/22] rebase: use read_oneliner()
Quoted 11 lines · Denton Liu
Since in
sequencer.c,read_one()basically duplicates the functionality ofread_oneliner(), reduce code duplication by replacingread_one()withread_oneliner().This was done with the following Coccinelle script
@@ expression a, b; @@ - read_one(a, b) + !read_oneliner(b, a, READ_ONELINER_WARN_NON_EXISTENCE)
WARN_MISSING, you mean?
builtin/rebase.c static const char *state_dir_path(const char *filename, struct rebase_options *o · line 597
Quoted 9 lines
builtin/rebase.c +0
-9-/* Read one file, then strip line endings */ -static int read_one (const char *path,struct strbuf *buf) -{ -if (strbuf_read_file (buf, path,0 ) <0 ) -return error_errno (_ ("could not read '%s'" ), path); -strbuf_trim_trailing_newline (buf); -return 0 ; -} -
OK. Both functions trim the end-of-line marker from the end of the string, so this is truly a no-op that reduces the code. Good.
PATCH v5 9/22 sequencer: make apply_autostash() accept a path
In reply to Denton LiuIn order to make apply_autostash() more generic for future extraction, make it accept a path argument so that the location from where to read the reference to the autostash commit can be customized. Remove the opts argument since it was unused before anyway.
--- sequencer.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
sequencer.c +7 -7
@@ -3657,13 +3657,13 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)return -1 ; } -static int apply_autostash (struct replay_opts *opts) +static int apply_autostash (const char *path) {struct strbuf stash_sha1 =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;int ret =0 ; -if (!read_oneliner (&stash_sha1,rebase_path_autostash (), +if (!read_oneliner (&stash_sha1, path,READ_ONELINER_SKIP_IF_EMPTY )) {strbuf_release (&stash_sha1);return 0 ;
@@ -3756,7 +3756,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,return error (_ ("%s: not a valid OID" ), orig_head);if (run_git_checkout (r, opts,oid_to_hex (onto), action)) { -apply_autostash (opts); +apply_autostash (rebase_path_autostash ());sequencer_remove_state (opts);return error (_ ("could not detach HEAD" )); }
@@ -4070,7 +4070,7 @@ static int pick_commits(struct repository *r,run_command (&hook); } } -apply_autostash (opts); +apply_autostash (rebase_path_autostash ());if (!opts->quiet) {if (!opts->verbose)
@@ -5079,7 +5079,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned flatodo_list_add_exec_commands (todo_list, commands);if (count_commands (todo_list) ==0 ) { -apply_autostash (opts); +apply_autostash (rebase_path_autostash ());sequencer_remove_state (opts);return error (_ ("nothing to do" ));
@@ -5090,12 +5090,12 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned flaif (res ==-1 )return -1 ;else if (res ==-2 ) { -apply_autostash (opts); +apply_autostash (rebase_path_autostash ());sequencer_remove_state (opts);return -1 ; }else if (res ==-3 ) { -apply_autostash (opts); +apply_autostash (rebase_path_autostash ());sequencer_remove_state (opts);todo_list_release (&new_todo);
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 10/22 sequencer: rename stash_sha1 to stash_oid
In reply to Denton LiuThe preferred terminology is to refer to object identifiers as "OIDs". Rename the stash_sha1 variable to stash_oid in order to conform to this.
--- sequencer.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
sequencer.c +8 -8
@@ -3659,23 +3659,23 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)static int apply_autostash (const char *path) { -struct strbuf stash_sha1 =STRBUF_INIT ; +struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;int ret =0 ; -if (!read_oneliner (&stash_sha1, path, +if (!read_oneliner (&stash_oid, path,READ_ONELINER_SKIP_IF_EMPTY )) { -strbuf_release (&stash_sha1); +strbuf_release (&stash_oid);return 0 ; } -strbuf_trim (&stash_sha1); +strbuf_trim (&stash_oid); child.git_cmd =1 ; child.no_stdout =1 ; child.no_stderr =1 ;argv_array_push (&child.args,"stash" );argv_array_push (&child.args,"apply" ); -argv_array_push (&child.args, stash_sha1.buf); +argv_array_push (&child.args, stash_oid.buf);if (!run_command (&child))fprintf (stderr,_ ("Applied autostash.\n" ));else {
@@ -3687,9 +3687,9 @@ static int apply_autostash(const char *path)argv_array_push (&store.args,"-m" );argv_array_push (&store.args,"autostash" );argv_array_push (&store.args,"-q" ); -argv_array_push (&store.args, stash_sha1.buf); +argv_array_push (&store.args, stash_oid.buf);if (run_command (&store)) - ret =error (_ ("cannot store %s" ), stash_sha1.buf); + ret =error (_ ("cannot store %s" ), stash_oid.buf);else fprintf(stderr, _("Applying autostash resulted in conflicts.\n"
@@ -3698,7 +3698,7 @@ static int apply_autostash(const char *path) " \"git stash drop\" at any time.\n")); } -strbuf_release (&stash_sha1); +strbuf_release (&stash_oid);return ret; }
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 11/22 rebase: use apply_autostash() from sequencer.c
In reply to Denton LiuThe apply_autostash() function in builtin/rebase.c is similar enough to the apply_autostash() function in sequencer.c that they are almost interchangeable, except for the type of arg they accept. Make the sequencer.c version extern and use it in rebase.
The rebase version was introduced in 6defce2b02 (builtin rebase: support --autostash option, 2018-09-04) as part of the shell to C conversion. It opted to duplicate the function because, at the time, there was another in-progress project converting interactive rebase from shell to C as well and they did not want to clash with them by refactoring sequencer.c version of apply_autostash(). Since both efforts are long done, we can freely combine them together now.
--- builtin/rebase.c | 49 ++---------------------------------------------- sequencer.c | 2 +- sequencer.h | 2 ++ 3 files changed, 5 insertions(+), 48 deletions(-)
builtin/rebase.c +2 -47
@@ -712,51 +712,6 @@ static int rebase_write_basic_state(struct rebase_options *opts)return 0 ; } -static int apply_autostash (struct rebase_options *opts) -{ -const char *path =state_dir_path ("autostash" , opts); -struct strbuf autostash =STRBUF_INIT ; -struct child_process stash_apply =CHILD_PROCESS_INIT ; - -if (!file_exists (path)) -return 0 ; - -if (!read_oneliner (&autostash, path,READ_ONELINER_WARN_MISSING )) -return error (_ ("Could not read '%s'" ), path); -/* Ensure that the hash is not mistaken for a number */ -strbuf_addstr (&autostash,"^0" ); -argv_array_pushl (&stash_apply.args, -"stash" ,"apply" , autostash.buf,NULL ); - stash_apply.git_cmd =1 ; - stash_apply.no_stderr = stash_apply.no_stdout = - stash_apply.no_stdin =1 ; -if (!run_command (&stash_apply)) -printf (_ ("Applied autostash.\n" )); -else { -struct argv_array args =ARGV_ARRAY_INIT ; -int res =0 ; - -argv_array_pushl (&args, -"stash" ,"store" ,"-m" ,"autostash" ,"-q" , - autostash.buf,NULL ); -if (run_command_v_opt (args.argv,RUN_GIT_CMD )) - res =error (_ ("Cannot store %s" ), autostash.buf); -argv_array_clear (&args); -strbuf_release (&autostash); -if (res) -return res; - -fprintf (stderr, -_ ("Applying autostash resulted in conflicts.\n" -"Your changes are safe in the stash.\n" -"You can run \"git stash pop\" or \"git stash drop\" " -"at any time.\n" )); - } - -strbuf_release (&autostash); -return 0 ; -} -static int finish_rebase (struct rebase_options *opts) {struct strbuf dir =STRBUF_INIT ;
@@ -764,7 +719,7 @@ static int finish_rebase(struct rebase_options *opts)int ret =0 ;delete_ref (NULL ,"REBASE_HEAD" ,NULL ,REF_NO_DEREF ); -apply_autostash (opts); +apply_autostash (state_dir_path ("autostash" , opts));close_object_store (the_repository->objects);/* * We ignore errors in 'gc --auto', since the
@@ -1209,7 +1164,7 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action) }else if (status ==2 ) {struct strbuf dir =STRBUF_INIT ; -apply_autostash (opts); +apply_autostash (state_dir_path ("autostash" , opts));strbuf_addstr (&dir, opts->state_dir);remove_dir_recursively (&dir,0 );strbuf_release (&dir);
sequencer.c +1 -1
@@ -3657,7 +3657,7 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)return -1 ; } -static int apply_autostash (const char *path) +int apply_autostash (const char *path) {struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;
sequencer.h +2 -0
@@ -191,6 +191,8 @@ void commit_post_rewrite(struct repository *r,const struct commit *current_head, const struct object_id *new_head); +int apply_autostash (const char *path); +#define SUMMARY_INITIAL_COMMIT (1 << 0)#define SUMMARY_SHOW_AUTHOR_DATE (1 << 1) void print_commit_summary(struct repository *repo,
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 12/22 rebase: generify reset_head()
In reply to Denton LiuIn the future, we plan on lib-ifying reset_head() so we need it to be more generic. Make it more generic by making it accept a struct repository argument instead of implicitly using the non-repo functions. Also, make it accept a const char *default_reflog_action argument so that the default action of "rebase" isn't hardcoded in.
--- builtin/rebase.c | 64 ++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 27 deletions(-)
builtin/rebase.c +37 -27
@@ -28,6 +28,8 @@#include "sequencer.h" #include "rebase-interactive.h" +#define DEFAULT_REFLOG_ACTION "rebase" +static char const *const builtin_rebase_usage[] = {N_ ("git rebase [-i] [options] [--exec <cmd>] " "[--onto <newbase> | --keep-base] [<upstream> [<branch>]]" ),
@@ -772,9 +774,10 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)#define RESET_HEAD_REFS_ONLY (1<<3)#define RESET_ORIG_HEAD (1<<4) -static int reset_head (struct object_id *oid,const char *action, +static int reset_head (struct repository *r,struct object_id *oid,const char *action,const char *switch_to_branch,unsigned flags, -const char *reflog_orig_head,const char *reflog_head) +const char *reflog_orig_head,const char *reflog_head, +const char *default_reflog_action) {unsigned detach_head = flags &RESET_HEAD_DETACH ;unsigned reset_hard = flags &RESET_HEAD_HARD ;
@@ -796,7 +799,7 @@ static int reset_head(struct object_id *oid, const char *action,if (switch_to_branch && !starts_with (switch_to_branch,"refs/" ))BUG ("Not a fully qualified branch: '%s'" , switch_to_branch); -if (!refs_only &&hold_locked_index (&lock,LOCK_REPORT_ON_ERROR ) <0 ) { +if (!refs_only &&repo_hold_locked_index (r, &lock,LOCK_REPORT_ON_ERROR ) <0 ) { ret =-1 ; goto leave_reset_head; }
@@ -815,26 +818,26 @@ static int reset_head(struct object_id *oid, const char *action,memset (&unpack_tree_opts,0 ,sizeof (unpack_tree_opts));setup_unpack_trees_porcelain (&unpack_tree_opts, action); unpack_tree_opts.head_idx =1 ; - unpack_tree_opts.src_index = the_repository->index; - unpack_tree_opts.dst_index = the_repository->index; + unpack_tree_opts.src_index = r->index; + unpack_tree_opts.dst_index = r->index; unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge; unpack_tree_opts.update =1 ; unpack_tree_opts.merge =1 ;if (!detach_head) unpack_tree_opts.reset =1 ; -if (repo_read_index_unmerged (the_repository) <0 ) { +if (repo_read_index_unmerged (r) <0 ) { ret =error (_ ("could not read index" )); goto leave_reset_head; } -if (!reset_hard && !fill_tree_descriptor (the_repository, &desc[nr++], &head_oid)) { +if (!reset_hard && !fill_tree_descriptor (r, &desc[nr++], &head_oid)) { ret =error (_ ("failed to find tree of %s" ),oid_to_hex (&head_oid)); goto leave_reset_head; } -if (!fill_tree_descriptor (the_repository, &desc[nr++], oid)) { +if (!fill_tree_descriptor (r, &desc[nr++], oid)) { ret =error (_ ("failed to find tree of %s" ),oid_to_hex (oid)); goto leave_reset_head; }
@@ -845,16 +848,16 @@ static int reset_head(struct object_id *oid, const char *action, } tree =parse_tree_indirect (oid); -prime_cache_tree (the_repository, the_repository->index, tree); +prime_cache_tree (r, r->index, tree); -if (write_locked_index (the_repository->index, &lock,COMMIT_LOCK ) <0 ) { +if (write_locked_index (r->index, &lock,COMMIT_LOCK ) <0 ) { ret =error (_ ("could not write index" )); goto leave_reset_head; } reset_head_refs: reflog_action =getenv (GIT_REFLOG_ACTION_ENVIRONMENT ); -strbuf_addf (&msg,"%s: " , reflog_action ? reflog_action :"rebase" ); +strbuf_addf (&msg,"%s: " , reflog_action ? reflog_action : default_reflog_action); prefix_len = msg.len;if (update_orig_head) {
@@ -916,8 +919,10 @@ static int move_to_original_branch(struct rebase_options *opts) opts->head_name, oid_to_hex(&opts->onto->object.oid));strbuf_addf (&head_reflog,"rebase finished: returning to %s" , opts->head_name); - ret =reset_head (NULL ,"" , opts->head_name,RESET_HEAD_REFS_ONLY , - orig_head_reflog.buf, head_reflog.buf); + ret =reset_head (the_repository,NULL ,"" , opts->head_name, +RESET_HEAD_REFS_ONLY , + orig_head_reflog.buf, head_reflog.buf, +DEFAULT_REFLOG_ACTION );strbuf_release (&orig_head_reflog);strbuf_release (&head_reflog);
@@ -1005,8 +1010,9 @@ static int run_am(struct rebase_options *opts)free (rebased_patches);argv_array_clear (&am.args); -reset_head (&opts->orig_head,"checkout" , opts->head_name,0 , -"HEAD" ,NULL ); +reset_head (the_repository, &opts->orig_head,"checkout" , + opts->head_name,0 , +"HEAD" ,NULL ,DEFAULT_REFLOG_ACTION ); error(_("\ngit encountered an error while preparing the " "patches to replay\n" "these revisions:\n"
@@ -1661,8 +1667,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)rerere_clear (the_repository, &merge_rr);string_list_clear (&merge_rr,1 ); -if (reset_head (NULL ,"reset" ,NULL ,RESET_HEAD_HARD , -NULL ,NULL ) <0 ) +if (reset_head (the_repository,NULL ,"reset" ,NULL ,RESET_HEAD_HARD , +NULL ,NULL ,DEFAULT_REFLOG_ACTION ) <0 )die (_ ("could not discard worktree changes" ));remove_branch_state (the_repository,0 );if (read_basic_state (&options))
@@ -1679,9 +1685,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (read_basic_state (&options))exit (1 ); -if (reset_head (&options.orig_head,"reset" , +if (reset_head (the_repository, &options.orig_head,"reset" , options.head_name,RESET_HEAD_HARD , -NULL ,NULL ) <0 ) +NULL ,NULL ,DEFAULT_REFLOG_ACTION ) <0 )die (_ ("could not move back to %s" ),oid_to_hex (&options.orig_head));remove_branch_state (the_repository,0 );
@@ -2073,8 +2079,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.state_dir);write_file (autostash,"%s" ,oid_to_hex (&oid));printf (_ ("Created autostash: %s\n" ), buf.buf); -if (reset_head (NULL ,"reset --hard" , -NULL ,RESET_HEAD_HARD ,NULL ,NULL ) <0 ) +if (reset_head (the_repository,NULL ,"reset --hard" , +NULL ,RESET_HEAD_HARD ,NULL ,NULL , +DEFAULT_REFLOG_ACTION ) <0 )die (_ ("could not reset --hard" )); if (discard_index(the_repository->index) < 0 ||
@@ -2114,10 +2121,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)strbuf_addf (&buf,"%s: checkout %s" ,getenv (GIT_REFLOG_ACTION_ENVIRONMENT ), options.switch_to); -if (reset_head (&options.orig_head,"checkout" , +if (reset_head (the_repository, + &options.orig_head,"checkout" , options.head_name,RESET_HEAD_RUN_POST_CHECKOUT_HOOK , -NULL , buf.buf) <0 ) { +NULL , buf.buf, +DEFAULT_REFLOG_ACTION ) <0 ) { ret = !!error (_ ("could not switch to " "%s" ), options.switch_to);
@@ -2189,10 +2198,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)strbuf_addf (&msg,"%s: checkout %s" ,getenv (GIT_REFLOG_ACTION_ENVIRONMENT ), options.onto_name); -if (reset_head (&options.onto->object.oid,"checkout" ,NULL , +if (reset_head (the_repository, &options.onto->object.oid,"checkout" ,NULL ,RESET_HEAD_DETACH |RESET_ORIG_HEAD |RESET_HEAD_RUN_POST_CHECKOUT_HOOK , -NULL , msg.buf)) +NULL , msg.buf,DEFAULT_REFLOG_ACTION ))die (_ ("Could not detach HEAD" ));strbuf_release (&msg);
@@ -2207,8 +2216,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)strbuf_addf (&msg,"rebase finished: %s onto %s" , options.head_name ? options.head_name :"detached HEAD" ,oid_to_hex (&options.onto->object.oid)); -reset_head (NULL ,"Fast-forwarded" , options.head_name, -RESET_HEAD_REFS_ONLY ,"HEAD" , msg.buf); +reset_head (the_repository,NULL ,"Fast-forwarded" , options.head_name, +RESET_HEAD_REFS_ONLY ,"HEAD" , msg.buf, +DEFAULT_REFLOG_ACTION );strbuf_release (&msg); ret = !!finish_rebase (&options); goto cleanup;
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 13/22 reset: extract reset_head() from rebase
In reply to Denton LiuContinue the process of lib-ifying the autostash code. In a future commit, this will be used to implement --autostash in other builtins.
This patch is best viewed with --color-moved.
--- Makefile | 1 + builtin/rebase.c | 139 +--------------------------------------------- reset.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ reset.h | 20 +++++++ 4 files changed, 162 insertions(+), 138 deletions(-) create mode 100644 reset.c create mode 100644 reset.h
Makefile +1 -0
@@ -970,6 +970,7 @@ LIB_OBJS += replace-object.o LIB_OBJS += repo-settings.o LIB_OBJS += repository.o LIB_OBJS += rerere.o +LIB_OBJS += reset.o LIB_OBJS += resolve-undo.o LIB_OBJS += revision.o LIB_OBJS += run-command.o
builtin/rebase.c +1 -138
151 lines not on this page · raw message
reset.c new file +140 -0
140 lines not on this page · raw message
reset.h new file +20 -0
@@ -0,0 +1,20 @@ +#ifndef RESET_H +#define RESET_H + +#include "hash.h" +#include "repository.h" + +#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION" + +#define RESET_HEAD_DETACH (1<<0) +#define RESET_HEAD_HARD (1<<1) +#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2) +#define RESET_HEAD_REFS_ONLY (1<<3) +#define RESET_ORIG_HEAD (1<<4) + +int reset_head (struct repository *r,struct object_id *oid,const char *action, +const char *switch_to_branch,unsigned flags, +const char *reflog_orig_head,const char *reflog_head, +const char *default_reflog_action); + +#endif
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 14/22 rebase: extract create_autostash()
In reply to Denton LiuIn a future commit, we will lib-ify this code. In preparation for this, extract the code into the create_autostash() function so that it can be cleaned up before it is finally lib-ified.
This patch is best viewed with --color-moved and --color-moved-ws=allow-indentation-change.
--- builtin/rebase.c | 94 +++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 44 deletions(-)
builtin/rebase.c +50 -44
@@ -1274,6 +1274,55 @@ static int check_exec_cmd(const char *cmd)return 0 ; } +static void create_autostash (struct rebase_options *options) +{ +struct strbuf buf =STRBUF_INIT ; +struct lock_file lock_file =LOCK_INIT ; +int fd; + + fd =hold_locked_index (&lock_file,0 ); +refresh_cache (REFRESH_QUIET ); +if (0 <= fd) +repo_update_index_if_able (the_repository, &lock_file); +rollback_lock_file (&lock_file); + +if (has_unstaged_changes (the_repository,1 ) || +has_uncommitted_changes (the_repository,1 )) { +const char *autostash = +state_dir_path ("autostash" , options); +struct child_process stash =CHILD_PROCESS_INIT ; +struct object_id oid; + +argv_array_pushl (&stash.args, +"stash" ,"create" ,"autostash" ,NULL ); + stash.git_cmd =1 ; + stash.no_stdin =1 ; +strbuf_reset (&buf); +if (capture_command (&stash, &buf,GIT_MAX_HEXSZ )) +die (_ ("Cannot autostash" )); +strbuf_trim_trailing_newline (&buf); +if (get_oid (buf.buf, &oid)) +die (_ ("Unexpected stash response: '%s'" ), + buf.buf); +strbuf_reset (&buf); +strbuf_add_unique_abbrev (&buf, &oid,DEFAULT_ABBREV ); + +if (safe_create_leading_directories_const (autostash)) +die (_ ("Could not create directory for '%s'" ), + options->state_dir); +write_file (autostash,"%s" ,oid_to_hex (&oid)); +printf (_ ("Created autostash: %s\n" ), buf.buf); +if (reset_head (the_repository,NULL ,"reset --hard" , +NULL ,RESET_HEAD_HARD ,NULL ,NULL , +DEFAULT_REFLOG_ACTION ) <0 ) +die (_ ("could not reset --hard" )); + +if (discard_index (the_repository->index) <0 || +repo_read_index (the_repository) <0 ) +die (_ ("could not read index" )); + } +strbuf_release (&buf); +}int cmd_rebase (int argc,const char **argv,const char *prefix) {
@@ -1907,50 +1956,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)die (_ ("could not read index" ));if (options.autostash) { -struct lock_file lock_file =LOCK_INIT ; -int fd; - - fd =hold_locked_index (&lock_file,0 ); -refresh_cache (REFRESH_QUIET ); -if (0 <= fd) -repo_update_index_if_able (the_repository, &lock_file); -rollback_lock_file (&lock_file); - -if (has_unstaged_changes (the_repository,1 ) || -has_uncommitted_changes (the_repository,1 )) { -const char *autostash = -state_dir_path ("autostash" , &options); -struct child_process stash =CHILD_PROCESS_INIT ; -struct object_id oid; - -argv_array_pushl (&stash.args, -"stash" ,"create" ,"autostash" ,NULL ); - stash.git_cmd =1 ; - stash.no_stdin =1 ; -strbuf_reset (&buf); -if (capture_command (&stash, &buf,GIT_MAX_HEXSZ )) -die (_ ("Cannot autostash" )); -strbuf_trim_trailing_newline (&buf); -if (get_oid (buf.buf, &oid)) -die (_ ("Unexpected stash response: '%s'" ), - buf.buf); -strbuf_reset (&buf); -strbuf_add_unique_abbrev (&buf, &oid,DEFAULT_ABBREV ); - -if (safe_create_leading_directories_const (autostash)) -die (_ ("Could not create directory for '%s'" ), - options.state_dir); -write_file (autostash,"%s" ,oid_to_hex (&oid)); -printf (_ ("Created autostash: %s\n" ), buf.buf); -if (reset_head (the_repository,NULL ,"reset --hard" , -NULL ,RESET_HEAD_HARD ,NULL ,NULL , -DEFAULT_REFLOG_ACTION ) <0 ) -die (_ ("could not reset --hard" )); - -if (discard_index (the_repository->index) <0 || -repo_read_index (the_repository) <0 ) -die (_ ("could not read index" )); - } +create_autostash (&options); } if (require_clean_work_tree(the_repository, "rebase",
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 15/22 rebase: generify create_autostash()
In reply to Denton LiuIn the future, we plan on lib-ifying create_autostash() so we need it to be more generic. Make it more generic by making it accept a struct repository argument instead of implicitly using the non-repo functions and the_repository. Also, make it accept a path argument so that we no longer rely have to rely on struct rebase_options. Finally, make it accept a default_reflog_action argument so we no longer have to rely on DEFAULT_REFLOG_ACTION.
--- builtin/rebase.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
builtin/rebase.c +16 -16
@@ -1274,22 +1274,21 @@ static int check_exec_cmd(const char *cmd)return 0 ; } -static void create_autostash (struct rebase_options *options) +static void create_autostash (struct repository *r,const char *path, +const char *default_reflog_action) {struct strbuf buf =STRBUF_INIT ;struct lock_file lock_file =LOCK_INIT ;int fd; - fd =hold_locked_index (&lock_file,0 ); -refresh_cache (REFRESH_QUIET ); + fd =repo_hold_locked_index (r, &lock_file,0 ); +refresh_index (r->index,REFRESH_QUIET ,NULL ,NULL ,NULL );if (0 <= fd) -repo_update_index_if_able (the_repository, &lock_file); +repo_update_index_if_able (r, &lock_file);rollback_lock_file (&lock_file); -if (has_unstaged_changes (the_repository,1 ) || -has_uncommitted_changes (the_repository,1 )) { -const char *autostash = -state_dir_path ("autostash" , options); +if (has_unstaged_changes (r,1 ) || +has_uncommitted_changes (r,1 )) {struct child_process stash =CHILD_PROCESS_INIT ;struct object_id oid;
@@ -1307,18 +1306,18 @@ static void create_autostash(struct rebase_options *options)strbuf_reset (&buf);strbuf_add_unique_abbrev (&buf, &oid,DEFAULT_ABBREV ); -if (safe_create_leading_directories_const (autostash)) +if (safe_create_leading_directories_const (path))die (_ ("Could not create directory for '%s'" ), - options->state_dir); -write_file (autostash,"%s" ,oid_to_hex (&oid)); + path); +write_file (path,"%s" ,oid_to_hex (&oid));printf (_ ("Created autostash: %s\n" ), buf.buf); -if (reset_head (the_repository,NULL ,"reset --hard" , +if (reset_head (r,NULL ,"reset --hard" ,NULL ,RESET_HEAD_HARD ,NULL ,NULL , -DEFAULT_REFLOG_ACTION ) <0 ) + default_reflog_action) <0 )die (_ ("could not reset --hard" )); -if (discard_index (the_repository->index) <0 || -repo_read_index (the_repository) <0 ) +if (discard_index (r->index) <0 || +repo_read_index (r) <0 )die (_ ("could not read index" )); }strbuf_release (&buf);
@@ -1956,7 +1955,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)die (_ ("could not read index" ));if (options.autostash) { -create_autostash (&options); +create_autostash (the_repository,state_dir_path ("autostash" , &options), +DEFAULT_REFLOG_ACTION ); } if (require_clean_work_tree(the_repository, "rebase",
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 16/22 sequencer: extract perform_autostash() from rebase
In reply to Denton LiuLib-ify the autostash code by extracting perform_autostash() from rebase into sequencer. In a future commit, this will be used to implement --autostash in other builtins.
This patch is best viewed with --color-moved.
--- builtin/rebase.c | 49 ----------------------------------------------- sequencer.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ sequencer.h | 2 ++ 3 files changed, 52 insertions(+), 49 deletions(-)
builtin/rebase.c +0 -49
@@ -1274,55 +1274,6 @@ static int check_exec_cmd(const char *cmd)return 0 ; } -static void create_autostash (struct repository *r,const char *path, -const char *default_reflog_action) -{ -struct strbuf buf =STRBUF_INIT ; -struct lock_file lock_file =LOCK_INIT ; -int fd; - - fd =repo_hold_locked_index (r, &lock_file,0 ); -refresh_index (r->index,REFRESH_QUIET ,NULL ,NULL ,NULL ); -if (0 <= fd) -repo_update_index_if_able (r, &lock_file); -rollback_lock_file (&lock_file); - -if (has_unstaged_changes (r,1 ) || -has_uncommitted_changes (r,1 )) { -struct child_process stash =CHILD_PROCESS_INIT ; -struct object_id oid; - -argv_array_pushl (&stash.args, -"stash" ,"create" ,"autostash" ,NULL ); - stash.git_cmd =1 ; - stash.no_stdin =1 ; -strbuf_reset (&buf); -if (capture_command (&stash, &buf,GIT_MAX_HEXSZ )) -die (_ ("Cannot autostash" )); -strbuf_trim_trailing_newline (&buf); -if (get_oid (buf.buf, &oid)) -die (_ ("Unexpected stash response: '%s'" ), - buf.buf); -strbuf_reset (&buf); -strbuf_add_unique_abbrev (&buf, &oid,DEFAULT_ABBREV ); - -if (safe_create_leading_directories_const (path)) -die (_ ("Could not create directory for '%s'" ), - path); -write_file (path,"%s" ,oid_to_hex (&oid)); -printf (_ ("Created autostash: %s\n" ), buf.buf); -if (reset_head (r,NULL ,"reset --hard" , -NULL ,RESET_HEAD_HARD ,NULL ,NULL , - default_reflog_action) <0 ) -die (_ ("could not reset --hard" )); - -if (discard_index (r->index) <0 || -repo_read_index (r) <0 ) -die (_ ("could not read index" )); - } -strbuf_release (&buf); -} -int cmd_rebase (int argc,const char **argv,const char *prefix) {struct rebase_options options =REBASE_OPTIONS_INIT ;
sequencer.c +50 -0
62 lines not on this page · raw message
sequencer.h +2 -0
@@ -191,6 +191,8 @@ void commit_post_rewrite(struct repository *r,const struct commit *current_head, const struct object_id *new_head); +void create_autostash (struct repository *r,const char *path, +const char *default_reflog_action);int apply_autostash (const char *path);#define SUMMARY_INITIAL_COMMIT (1 << 0)
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 17/22 sequencer: unlink autostash in apply_autostash()
In reply to Denton LiuExplicitly remove autostash file in apply_autostash() once it has been applied successfully.
This is currently a no-op because the only users of this function will unlink the state (including the autostash file) after this function runs. However, in the future, we will introduce a user of the function that does not explicitly remove the state so we do it here.
--- sequencer.c | 1 + 1 file changed, 1 insertion(+)
sequencer.c +1 -0
@@ -3748,6 +3748,7 @@ int apply_autostash(const char *path) " \"git stash drop\" at any time.\n")); } +unlink (path);strbuf_release (&stash_oid);return ret; }
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 18/22 sequencer: implement save_autostash()
In reply to Denton LiuExtract common functionality of apply_autostash() into apply_save_autostash() and use it to implement save_autostash(). This function will be used in a future commit.
The difference between save_autostash() and apply_autostash() is that the former does not try to apply the stash. It skips that step and just stores the created entry in the stash reflog.
This is useful in the case where we abort an operation when an autostash is present but we don't want to dirty the worktree with the application of the stash. For example, in a future commit, we will implement git merge --autostash. Since merges can be aborted using git reset --hard, we'd make use of save_autostash() to save the autostash entry instead of applying it to the worktree thus keeping the worktree undirtied.
--- sequencer.c | 37 +++++++++++++++++++++++++++---------- sequencer.h | 1 + 2 files changed, 28 insertions(+), 10 deletions(-)
sequencer.c +27 -10
@@ -3707,7 +3707,7 @@ void create_autostash(struct repository *r, const char *path,strbuf_release (&buf); } -int apply_autostash (const char *path) +static int apply_save_autostash (const char *path,int attempt_apply) {struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;
@@ -3720,13 +3720,17 @@ int apply_autostash(const char *path) }strbuf_trim (&stash_oid); - child.git_cmd =1 ; - child.no_stdout =1 ; - child.no_stderr =1 ; -argv_array_push (&child.args,"stash" ); -argv_array_push (&child.args,"apply" ); -argv_array_push (&child.args, stash_oid.buf); -if (!run_command (&child)) +if (attempt_apply) { + child.git_cmd =1 ; + child.no_stdout =1 ; + child.no_stderr =1 ; +argv_array_push (&child.args,"stash" ); +argv_array_push (&child.args,"apply" ); +argv_array_push (&child.args, stash_oid.buf); + ret =run_command (&child); + } + +if (attempt_apply && !ret)fprintf (stderr,_ ("Applied autostash.\n" ));else {struct child_process store =CHILD_PROCESS_INIT ;
@@ -3742,10 +3746,13 @@ int apply_autostash(const char *path) ret =error (_ ("cannot store %s" ), stash_oid.buf); else fprintf(stderr, - _("Applying autostash resulted in conflicts.\n" + _("%s\n""Your changes are safe in the stash.\n" "You can run \"git stash pop\" or" - " \"git stash drop\" at any time.\n")); + " \"git stash drop\" at any time.\n"), + attempt_apply ? +_ ("Applying autostash resulted in conflicts." ) : + _("Autostash exists; creating a new stash entry.")); }unlink (path);
@@ -3753,6 +3760,16 @@ int apply_autostash(const char *path)return ret; } +int save_autostash (const char *path) +{ +return apply_save_autostash (path,0 ); +} + +int apply_autostash (const char *path) +{ +return apply_save_autostash (path,1 ); +} +static const char *reflog_message (struct replay_opts *opts,const char *sub_action,const char *fmt, ...) {
sequencer.h +1 -0
@@ -193,6 +193,7 @@ void commit_post_rewrite(struct repository *r,void create_autostash (struct repository *r,const char *path,const char *default_reflog_action); +int save_autostash (const char *path);int apply_autostash (const char *path);#define SUMMARY_INITIAL_COMMIT (1 << 0)
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 19/22 sequencer: implement apply_autostash_oid()
In reply to Denton LiuSplit apply_save_autostash() into apply_autostash_oid() and apply_save_autostash() where the former operates on an OID string and the latter reads the OID from a file before passing it into apply_save_autostash_oid().
This function is required for a future commmit which will rely on being able to apply an autostash whose OID is stored as a string.
--- sequencer.c | 38 ++++++++++++++++++++++++++------------ sequencer.h | 1 + 2 files changed, 27 insertions(+), 12 deletions(-)
sequencer.c +26 -12
@@ -3707,26 +3707,18 @@ void create_autostash(struct repository *r, const char *path,strbuf_release (&buf); } -static int apply_save_autostash (const char *path,int attempt_apply) +static int apply_save_autostash_oid (const char *stash_oid,int attempt_apply) { -struct strbuf stash_oid =STRBUF_INIT ;struct child_process child =CHILD_PROCESS_INIT ;int ret =0 ; -if (!read_oneliner (&stash_oid, path, -READ_ONELINER_SKIP_IF_EMPTY )) { -strbuf_release (&stash_oid); -return 0 ; - } -strbuf_trim (&stash_oid); -if (attempt_apply) { child.git_cmd =1 ; child.no_stdout =1 ; child.no_stderr =1 ;argv_array_push (&child.args,"stash" );argv_array_push (&child.args,"apply" ); -argv_array_push (&child.args, stash_oid.buf); +argv_array_push (&child.args, stash_oid); ret =run_command (&child); }
@@ -3741,9 +3733,9 @@ static int apply_save_autostash(const char *path, int attempt_apply)argv_array_push (&store.args,"-m" );argv_array_push (&store.args,"autostash" );argv_array_push (&store.args,"-q" ); -argv_array_push (&store.args, stash_oid.buf); +argv_array_push (&store.args, stash_oid);if (run_command (&store)) - ret =error (_ ("cannot store %s" ), stash_oid.buf); + ret =error (_ ("cannot store %s" ), stash_oid);else fprintf(stderr, _("%s\n"
@@ -3755,6 +3747,23 @@ static int apply_save_autostash(const char *path, int attempt_apply) _("Autostash exists; creating a new stash entry.")); } +return ret; +} + +static int apply_save_autostash (const char *path,int attempt_apply) +{ +struct strbuf stash_oid =STRBUF_INIT ; +int ret =0 ; + +if (!read_oneliner (&stash_oid, path, +READ_ONELINER_SKIP_IF_EMPTY )) { +strbuf_release (&stash_oid); +return 0 ; + } +strbuf_trim (&stash_oid); + + ret =apply_save_autostash_oid (stash_oid.buf, attempt_apply); +unlink (path);strbuf_release (&stash_oid);return ret;
@@ -3770,6 +3779,11 @@ int apply_autostash(const char *path)return apply_save_autostash (path,1 ); } +int apply_autostash_oid (const char *stash_oid) +{ +return apply_save_autostash_oid (stash_oid,1 ); +} +static const char *reflog_message (struct replay_opts *opts,const char *sub_action,const char *fmt, ...) {
sequencer.h +1 -0
@@ -195,6 +195,7 @@ void create_autostash(struct repository *r, const char *path, const char *default_reflog_action);int save_autostash (const char *path);int apply_autostash (const char *path); +int apply_autostash_oid (const char *stash_oid);#define SUMMARY_INITIAL_COMMIT (1 << 0)#define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 20/22 merge: teach --autostash option
In reply to Denton LiuIn rebase, one can pass the --autostash option to cause the worktree to be automatically stashed before continuing with the rebase. This option is missing in merge, however.
Implement the --autostash option and corresponding merge.autoStash option in merge which stashes before merging and then pops after.
This option is useful when a developer has some local changes on a topic branch but they realize that their work depends on another branch. Previously, they had to run something like
git fetch ...
git stash push
git merge FETCH_HEAD
git stash pop
but now, that is reduced to
git fetch ... git merge --autostash FETCH_HEAD
When an autostash is generated, it is automatically reapplied to the worktree only in three explicit situations:
1. An incomplete merge is commit using git commit.
2. A merge completes successfully.
3. A merge is aborted using git merge --abort.
In all other situations where the merge state is removed using remove_merge_branch_state() such as aborting a merge via git reset --hard, the autostash is saved into the stash reflog instead keeping the worktree clean.
--- Documentation/config/merge.txt | 10 +++ Documentation/git-merge.txt | 11 ++- Documentation/merge-options.txt | 8 ++ branch.c | 1 + builtin/commit.c | 2 + builtin/merge.c | 25 ++++++ builtin/rebase.c | 3 +- parse-options.h | 1 + path.c | 1 + path.h | 4 +- t/t3033-merge-toplevel.sh | 22 ++++++ t/t7600-merge.sh | 132 ++++++++++++++++++++++++++++++++ 12 files changed, 214 insertions(+), 6 deletions(-)
Documentation/config/merge.txt +10 -0
@@ -70,6 +70,16 @@ merge.stat:: Whether to print the diffstat between ORIG_HEAD and the merge result at the end of the merge. True by default. +merge.autoStash:: + When set to true, automatically create a temporary stash entry + before the operation begins, and apply it after the operation + ends. This means that you can run merge on a dirty worktree. + However, use with care: the final stash application after a + successful merge might result in non-trivial conflicts. + This option can be overridden by the `--no-autostash` and + `--autostash` options of linkgit:git-merge[1]. + Defaults to false. + merge.tool:: Controls which merge tool is used by linkgit:git-mergetool[1]. The list below shows the valid built-in values.
Documentation/git-merge.txt +8 -3
@@ -94,7 +94,8 @@ will be appended to the specified message. --abort:: Abort the current conflict resolution process, and - try to reconstruct the pre-merge state. + try to reconstruct the pre-merge state. If an autostash entry is + present, apply it to the worktree. + If there were uncommitted worktree changes present when the merge started, 'git merge --abort' will in some cases be unable to
@@ -102,11 +103,15 @@ reconstruct these changes. It is therefore recommended to always commit or stash your changes before running 'git merge'. + 'git merge --abort' is equivalent to 'git reset --merge' when -`MERGE_HEAD` is present. +`MERGE_HEAD` is present unless `MERGE_AUTOSTASH` is also present in +which case 'git merge --abort' applies the stash entry to the worktree +whereas 'git reset --merge' will save the stashed changes in the stash +reflog. --quit:: Forget about the current merge in progress. Leave the index - and the working tree as-is. + and the working tree as-is. If `MERGE_AUTOSTASH` is present, the + stash entry will be saved to the stash reflog. --continue:: After a 'git merge' stops due to conflicts you can conclude the
Documentation/merge-options.txt +8 -0
@@ -155,6 +155,14 @@ ifndef::git-pull[] Note that not all merge strategies may support progress reporting. +--autostash:: +--no-autostash:: + Automatically create a temporary stash entry before the operation + begins, and apply it after the operation ends. This means + that you can run the operation on a dirty worktree. However, use + with care: the final stash application after a successful + merge might result in non-trivial conflicts. + endif::git-pull[] --allow-unrelated-histories::
branch.c +1 -0
@@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r)unlink (git_path_merge_rr (r));unlink (git_path_merge_msg (r));unlink (git_path_merge_mode (r)); +save_autostash (git_path_merge_autostash (r)); }void remove_branch_state (struct repository *r,int verbose)
builtin/commit.c +2 -0
@@ -1713,6 +1713,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) &oid, flags); } +apply_autostash (git_path_merge_autostash (the_repository)); +UNLEAK (err);UNLEAK (sb);return 0 ;
builtin/merge.c +25 -0
@@ -82,6 +82,7 @@ static int show_progress = -1;static int default_to_upstream =1 ;static int signoff;static const char *sign_commit; +static int autostash;static int no_verify;static struct strategy all_strategy[] = {
@@ -286,6 +287,7 @@ static struct option builtin_merge_options[] = {OPT_SET_INT (0 ,"progress" , &show_progress,N_ ("force progress reporting" ),1 ), { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"), N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, +OPT_AUTOSTASH (&autostash),OPT_BOOL (0 ,"overwrite-ignore" , &overwrite_ignore,N_ ("update ignored files (default)" )),OPT_BOOL (0 ,"signoff" , &signoff,N_ ("add Signed-off-by:" )), OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
@@ -475,6 +477,7 @@ static void finish(struct commit *head_commit,/* Run a post-merge hook */ run_hook_le (NULL ,"post-merge" , squash ?"1" :"0" ,NULL ); +apply_autostash (git_path_merge_autostash (the_repository));strbuf_release (&reflog_message); }
@@ -634,6 +637,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)return 0 ; }else if (!strcmp (k,"gpg.mintrustlevel" )) { check_trust_level =0 ; + }else if (!strcmp (k,"merge.autostash" )) { + autostash =git_config_bool (k, v); +return 0 ; } status =fmt_merge_msg_config (k, v, cb);
@@ -1281,6 +1287,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (abort_current_merge) {int nargc =2 ;const char *nargv[] = {"reset" ,"--merge" ,NULL }; +struct strbuf stash_oid =STRBUF_INIT ;if (orig_argc !=2 ) usage_msg_opt(_("--abort expects no arguments"),
@@ -1289,8 +1296,17 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (!file_exists (git_path_merge_head (the_repository)))die (_ ("There is no merge to abort (MERGE_HEAD missing)." )); +if (read_oneliner (&stash_oid,git_path_merge_autostash (the_repository), +READ_ONELINER_SKIP_IF_EMPTY )) +unlink (git_path_merge_autostash (the_repository)); +/* Invoke 'git reset --merge' */ ret =cmd_reset (nargc, nargv, prefix); + +if (stash_oid.len) +apply_autostash_oid (stash_oid.buf); + +strbuf_release (&stash_oid); goto done; }
@@ -1513,6 +1529,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix) goto done; } +if (autostash) +create_autostash (the_repository, +git_path_merge_autostash (the_repository), +"merge" ); if (checkout_fast_forward(the_repository, &head_commit->object.oid, &commit->object.oid,
@@ -1579,6 +1599,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)if (fast_forward ==FF_ONLY )die (_ ("Not possible to fast-forward, aborting." )); +if (autostash) +create_autostash (the_repository, +git_path_merge_autostash (the_repository), +"merge" ); +/* We are going to make a new commit. */ git_committer_info (IDENT_STRICT );
builtin/rebase.c +1 -2
@@ -1376,8 +1376,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) {OPTION_STRING ,'S' ,"gpg-sign" , &gpg_sign,N_ ("key-id" ),N_ ("GPG-sign commits" ), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, -OPT_BOOL (0 ,"autostash" , &options.autostash, -N_ ("automatically stash/stash pop before and after" )), +OPT_AUTOSTASH (&options.autostash),OPT_STRING_LIST ('x' ,"exec" , &exec,N_ ("exec" ),N_ ("add exec lines after each commit of the " "editable list")),
parse-options.h +1 -0
@@ -336,5 +336,6 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);#define OPT_CLEANUP (v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))#define OPT_PATHSPEC_FROM_FILE (v) OPT_FILENAME(0, "pathspec-from-file", v, N_("read pathspec from file"))#define OPT_PATHSPEC_FILE_NUL (v) OPT_BOOL(0, "pathspec-file-nul", v, N_("with --pathspec-from-file, pathspec elements are separated with NUL character")) +#define OPT_AUTOSTASH (v) OPT_BOOL(0, "autostash", v, N_("automatically stash/stash pop before and after"))#endif
path.c +1 -0
@@ -1535,5 +1535,6 @@ REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG") REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR") REPO_GIT_PATH_FUNC(merge_mode, "MERGE_MODE") REPO_GIT_PATH_FUNC(merge_head, "MERGE_HEAD") +REPO_GIT_PATH_FUNC(merge_autostash, "MERGE_AUTOSTASH") REPO_GIT_PATH_FUNC(fetch_head, "FETCH_HEAD")REPO_GIT_PATH_FUNC (shallow,"shallow" )
path.h +3 -1
@@ -177,11 +177,12 @@ struct path_cache {const char *merge_rr;const char *merge_mode;const char *merge_head; +const char *merge_autostash;const char *fetch_head;const char *shallow; }; -#define PATH_CACHE_INIT { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL } +#define PATH_CACHE_INIT { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL }const char *git_path_cherry_pick_head (struct repository *r);const char *git_path_revert_head (struct repository *r);
@@ -190,6 +191,7 @@ const char *git_path_merge_msg(struct repository *r);const char *git_path_merge_rr (struct repository *r);const char *git_path_merge_mode (struct repository *r);const char *git_path_merge_head (struct repository *r); +const char *git_path_merge_autostash (struct repository *r);const char *git_path_fetch_head (struct repository *r);const char *git_path_shallow (struct repository *r);
t/t3033-merge-toplevel.sh +22 -0
@@ -142,6 +142,17 @@ test_expect_success 'refuse two-project merge by default' ' test_must_fail git merge five ' +test_expect_success 'refuse two-project merge by default, quit before --autostash happens' ' + t3033_reset && + git reset --hard four && + echo change >>one.t && + git diff >expect && + test_must_fail git merge --autostash five 2>err && + test_i18ngrep ! "stash" err && + git diff >actual && + test_cmp expect actual +' + test_expect_success 'two-project merge with --allow-unrelated-histories' ' t3033_reset && git reset --hard four &&
@@ -149,4 +160,15 @@ test_expect_success 'two-project merge with --allow-unrelated-histories' ' git diff --exit-code five ' +test_expect_success 'two-project merge with --allow-unrelated-histories with --autostash' ' + t3033_reset && + git reset --hard four && + echo change >>one.t && + git diff one.t >expect && + git merge --allow-unrelated-histories --autostash five 2>err && + test_i18ngrep "Applied autostash." err && + git diff one.t >actual && + test_cmp expect actual +' + test_done
t/t7600-merge.sh +132 -0
151 lines not on this page · raw message
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 21/22 t5520: make test_pull_autostash() accept expect_parent_num
In reply to Denton LiuBefore, test_pull_autostash() was hardcoded to run test_cmp_rev HEAD^ copy to test that a rebase happened. However, in a future patch, we plan on testing merging as well. Make test_pull_autostash() accept a parent number as an argument so that, in the future, we can test if a merge happened in addition to a rebase.
--- t/t5520-pull.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
t/t5520-pull.sh +8 -6
@@ -10,11 +10,13 @@ modify () { } test_pull_autostash () { + expect_parent_num="$1" && + shift && git reset --hard before-rebase && echo dirty >new_file && git add new_file && git pull "$@" . copy && - test_cmp_rev HEAD^ copy && + test_cmp_rev HEAD^"$expect_parent_num" copy && echo dirty >expect && test_cmp expect new_file && echo "modified again" >expect &&
@@ -369,22 +371,22 @@ test_expect_success '--rebase fails with multiple branches' ' test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' test_config rebase.autostash true && - test_pull_autostash --rebase + test_pull_autostash 1 --rebase ' test_expect_success 'pull --rebase --autostash & rebase.autostash=true' ' test_config rebase.autostash true && - test_pull_autostash --rebase --autostash + test_pull_autostash 1 --rebase --autostash ' test_expect_success 'pull --rebase --autostash & rebase.autostash=false' ' test_config rebase.autostash false && - test_pull_autostash --rebase --autostash + test_pull_autostash 1 --rebase --autostash ' test_expect_success 'pull --rebase --autostash & rebase.autostash unset' ' test_unconfig rebase.autostash && - test_pull_autostash --rebase --autostash + test_pull_autostash 1 --rebase --autostash ' test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '
@@ -422,7 +424,7 @@ test_expect_success 'pull.rebase' ' test_expect_success 'pull --autostash & pull.rebase=true' ' test_config pull.rebase true && - test_pull_autostash --autostash + test_pull_autostash 1 --autostash ' test_expect_success 'pull --no-autostash & pull.rebase=true' '
Signature · 1 line
2.26.0.159.g23e2136ad0
PATCH v5 22/22 pull: pass --autostash to merge
In reply to Denton LiuBefore, --autostash only worked with git pull --rebase. However, in the last patch, merge learned --autostash as well so there's no reason why we should have this restriction anymore. Teach pull to pass --autostash to merge, just like it did for rebase.
--- Documentation/git-pull.txt | 9 ------- Documentation/merge-options.txt | 4 +-- builtin/pull.c | 9 ++++--- t/t5520-pull.sh | 43 +++++++++++++++++++++++++++------ 4 files changed, 42 insertions(+), 23 deletions(-)
Documentation/git-pull.txt +0 -9
@@ -133,15 +133,6 @@ unless you have read linkgit:git-rebase[1] carefully. --no-rebase:: Override earlier --rebase. ---autostash:: ---no-autostash:: - Before starting rebase, stash local modifications away (see - linkgit:git-stash[1]) if needed, and apply the stash entry when - done. `--no-autostash` is useful to override the `rebase.autoStash` - configuration variable (see linkgit:git-config[1]). -+ -This option is only valid when "--rebase" is used. - Options related to fetching ~~~~~~~~~~~~~~~~~~~~~~~~~~~
Documentation/merge-options.txt +2 -2
@@ -155,6 +155,8 @@ ifndef::git-pull[] Note that not all merge strategies may support progress reporting. +endif::git-pull[] + --autostash:: --no-autostash:: Automatically create a temporary stash entry before the operation
@@ -163,8 +165,6 @@ ifndef::git-pull[] with care: the final stash application after a successful merge might result in non-trivial conflicts. -endif::git-pull[] - --allow-unrelated-histories:: By default, `git merge` command refuses to merge histories that do not share a common ancestor. This option can be
builtin/pull.c +5 -4
@@ -163,7 +163,7 @@ static struct option pull_options[] = { N_("verify that the named commit has a valid GPG signature"), PARSE_OPT_NOARG),OPT_BOOL (0 ,"autostash" , &opt_autostash, -N_ ("automatically stash/stash pop before and after rebase" )), +N_ ("automatically stash/stash pop before and after" )),OPT_PASSTHRU_ARGV ('s' ,"strategy" , &opt_strategies,N_ ("strategy" ),N_ ("merge strategy to use" ), 0),
@@ -661,6 +661,10 @@ static int run_merge(void)argv_array_pushv (&args, opt_strategy_opts.argv);if (opt_gpg_sign)argv_array_push (&args, opt_gpg_sign); +if (opt_autostash ==0 ) +argv_array_push (&args,"--no-autostash" ); +else if (opt_autostash ==1 ) +argv_array_push (&args,"--autostash" );if (opt_allow_unrelated_histories >0 )argv_array_push (&args,"--allow-unrelated-histories" );
@@ -908,9 +912,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)if (get_oid ("HEAD" , &orig_head))oidclr (&orig_head); -if (!opt_rebase && opt_autostash !=-1 ) -die (_ ("--[no-]autostash option is only valid with --rebase." )); - autostash = config_autostash;if (opt_rebase) {if (opt_autostash !=-1 )
t/t5520-pull.sh +35 -8
55 lines not on this page · raw message
Signature · 1 line
2.26.0.159.g23e2136ad0
Hi Denton
Quoted 9 lines · Denton Liu
[...]
Chagnes since v4:* Drop 'sequencer: reuse
strbuf_trim()inread_oneliner()'* Fix commit message in 'sequencer: make
apply_autostash()accept a path'* Add more detail in commit message 'sequencer: implement
save_autostash()'
Quoted 8 lines
* In 'merge: teach --autostash option', improve documentation, remove
builtin/reset.cchanges, add agit merge --quittest case
[...]
Range-diff against v4:1: c26771e934 = 1: c26771e934 Makefile: ASCII-sort += lists 2: 059e0e8e43 = 2: 059e0e8e43 t7600: use test_write_lines() 3: 76585e5b13 = 3: 76585e5b13 sequencer: stop leaking buf 4: c7a3cfa200 < -: ---------- sequencer: reuse strbuf_trim() in read_oneliner()
I think it makes sense to drop that one, it was more trouble than it was worth in the end
Quoted 210 lines
5:
58da8e9555! 4:dc4674d222sequencer: make file exists check more efficient@@ Commit message check `errno` to see if the file doesn't exist. ## sequencer.c ## -@@ sequencer.c: static int write_message(const void *buf, size_t len, const char *filename, - static int read_oneliner(struct strbuf *buf, - const char *path, int skip_if_empty) +@@ sequencer.c: static int read_oneliner(struct strbuf *buf, { -- + int orig_len = buf->len; + - if (!file_exists(path)) - return 0; - - strbuf_reset(buf); if (strbuf_read_file(buf, path, 0) < 0) { - warning_errno(_("could not read '%s'"), path); + if (errno != ENOENT && errno != ENOTDIR)6:
6ead240957! 5:f30ad07823sequencer: makeread_oneliner()accept flags
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *f
+#defineREAD_ONELINER_SKIP_IF_EMPTY(1 << 0)
+
/*
- * Resets a strbuf then reads a file that was presumably written by a shell
- * script, i.e. with an end-of-line marker that needs to be stripped.
+ * Reads a file that was presumably written by a shell script, i.e. with an
+ * end-of-line marker that needs to be stripped.
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *filename,
* Returns 1 if the file was read, 0 if it could not be read or does not exist. */
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *f
- const char *path, intskip_if_empty)
+ const char *path, unsigned flags)
{
-strbuf_reset(buf);
- if (strbuf_read_file(buf, path, 0)< 0) {
+ intorig_len=buf->len;
+
@@sequencer.c: static int read_oneliner(struct strbuf *buf,
+ buf->buf[buf->len] = '\0';
+ }-
strbuf_trim(buf);
-
-- if (skip_if_empty&& !buf->len)
-+ if ((flags &READ_ONELINER_SKIP_IF_EMPTY) && !buf->len)
+- if (skip_if_empty&&buf->len==orig_len)
++ if ((flags &READ_ONELINER_SKIP_IF_EMPTY) &&buf->len==orig_len)
return 0;return 1;
@@sequencer.c: static intread_populate_opts(struct replay_opts *opts)
strbuf_reset(&buf);
else {
@@sequencer.c: static intread_populate_opts(struct replay_opts *opts)
- }
+strbuf_reset(&buf);
}- if (
read_oneliner(&buf, rebase_path_allow_rerere_autoupdate(), 1)) {
@@sequencer.c: static intread_populate_opts(struct replay_opts *opts)
opts->allow_rerere_auto=RERERE_AUTOUPDATE;
else if (!strcmp(buf.buf, "--no-rerere-autoupdate"))
@@sequencer.c: static intread_populate_opts(struct replay_opts *opts)
-read_strategy_opts(opts, &buf);
+strbuf_reset(&buf);if (read_oneliner(&opts->current_fixups,
-rebase_path_current_fixups(), 1)) {
7:2e7922b259! 6:8c504427e3sequencer: configurably warn on non-existent files
@@ Commit message
read_oneliner()will want the ability to output warnings in the event that thepathdoesn't exist. Introduce theREAD_ONELINER_WARN_MISSINGflag which, if active, would issue a
- warning when a file doesn't exist by skipping the!file_exists()check
- and lettingstrbuf_read_file()handle that case.
+ warning when a file doesn't exist by always executingwarning_errno()
+ in the case wherestrbuf_read_file()fails.##
sequencer.c##
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *filename,
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *f
+#defineREAD_ONELINER_WARN_MISSING(1 << 1)/*
- * Resets a strbuf then reads a file that was presumably written by a shell
+ * Reads a file that was presumably written by a shell script, i.e. with an @@sequencer.c: static int read_oneliner(struct strbuf *buf,
- {
-strbuf_reset(buf);
+ intorig_len=buf->len;
+
if (strbuf_read_file(buf, path, 0)< 0) {
- if (errno != ENOENT && errno != ENOTDIR)
+ if ((flags &READ_ONELINER_WARN_MISSING) ||
-+ (errno != ENOENT && errno != ENOTDIR))
++ (errno != ENOENT && errno != ENOTDIR))
warning_errno(_("could not read '%s'"), path);
return 0;
}
8:6c3c37994b! 7:50e93770e7sequencer: makeread_oneliner()extern
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *f
-#defineREAD_ONELINER_WARN_MISSING(1 << 1)
-
-/*
-- * Resets a strbuf then reads a file that was presumably written by a shell -- * script, i.e. with an end-of-line marker that needs to be stripped. +- * Reads a file that was presumably written by a shell script, i.e. with an +- * end-of-line marker that needs to be stripped.
- *
- * Note that only the last end-of-line marker is stripped, consistent with the
- * behavior of "$(cat path)" in a shell script.
@@sequencer.c: static int write_message(const void *buf,size_tlen, const char *f
+int read_oneliner(struct strbuf *buf,
const char *path, unsigned flags)
{
-strbuf_reset(buf);
+ intorig_len=buf->len;##
sequencer.h##
@@sequencer.h: void print_commit_summary(struct repository *repo,
@@sequencer.h: void print_commit_summary(struct repository *repo,
+#defineREAD_ONELINER_WARN_MISSING(1 << 1)
+
+/*
-+ * Resets a strbuf then reads a file that was presumably written by a shell -+ * script, i.e. with an end-of-line marker that needs to be stripped. ++ * Reads a file that was presumably written by a shell script, i.e. with an ++ * end-of-line marker that needs to be stripped.
+ *
+ * Note that only the last end-of-line marker is stripped, consistent with the
+ * behavior of "$(cat path)" in a shell script.
9:689f34a2a5! 8:0cc279fc14rebase: useread_oneliner()@@ Commit message Since in sequencer.c, read_one() basically duplicates the functionality of read_oneliner(), reduce code duplication by replacing read_one() with - read_oneliner(). Also, delete strbuf_reset() calls prior to - read_oneliner() as read_oneliner() already resets the strbuf. + read_oneliner(). - This was mostly done with the following Coccinelle script + This was done with the following Coccinelle script @@ expression a, b; @@ Commit message - read_one(a, b) + !read_oneliner(b, a, READ_ONELINER_WARN_NON_EXISTENCE) - Long lines were broken up and strbuf_reset()s were deleted manually. + and long lines were manually broken up. ## builtin/rebase.c ## @@ builtin/rebase.c: static const char *state_dir_path(const char *filename, struct rebase_options *o @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) opts->head_name = starts_with(head_name.buf, "refs/") ? xstrdup(head_name.buf) : NULL; @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) - * head. Fall back to reading from head to cover for the case that the - * user upgraded git with an ongoing interactive rebase. */ -- strbuf_reset(&buf); + strbuf_reset(&buf); if (file_exists(state_dir_path("orig-head", opts))) { - if (read_one(state_dir_path("orig-head", opts), &buf)) + if (!read_oneliner(&buf, state_dir_path("orig-head", opts), @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) if (get_oid(buf.buf, &opts->orig_head)) return error(_("invalid orig-head: '%s'"), buf.buf); @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) - } if (file_exists(state_dir_path("allow_rerere_autoupdate", opts))) { -- strbuf_reset(&buf); + strbuf_reset(&buf); - if (read_one(state_dir_path("allow_rerere_autoupdate", opts), - &buf)) + if (!read_oneliner(&buf, state_dir_path("allow_rerere_autoupdate", opts), @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) if (!strcmp(buf.buf, "--rerere-autoupdate")) opts->allow_rerere_autoupdate = RERERE_AUTOUPDATE; @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) - } if (file_exists(state_dir_path("gpg_sign_opt", opts))) { -- strbuf_reset(&buf); + strbuf_reset(&buf); - if (read_one(state_dir_path("gpg_sign_opt", opts), - &buf)) + if (!read_oneliner(&buf, state_dir_path("gpg_sign_opt", opts), @@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) return -1; free(opts->gpg_sign_opt); opts->gpg_sign_opt = xstrdup(buf.buf); - } +@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts) if (file_exists(state_dir_path("strategy", opts))) { -- strbuf_reset(&buf); + strbuf_reset(&buf); - if (read_one(state_dir_path("strategy", opts), &buf)) + if (!read_oneliner(&buf, state_dir_path("strategy", opts), + READ_ONELINER_WARN_MISSING)) return -1; free(opts->strategy); opts->strategy = xstrdup(buf.buf); - }
Quoted 8 lines
+@@ builtin/rebase.c: static int read_basic_state(struct rebase_options *opts)
if (file_exists(state_dir_path("strategy_opts", opts))) {
-- strbuf_reset(&buf);
+ strbuf_reset(&buf);
- if (read_one(state_dir_path("strategy_opts", opts), &buf))
+ if (!read_oneliner(&buf, state_dir_path("strategy_opts", opts),
+ READ_ONELINER_WARN_MISSING))So up to here I think all the changes since v4 are due to the dropped patch bar the rewording of the message in patch 7 that drops the reference to file_exists()
Junio pointed out a typo in the message to patch 9, apart from that it all looks good to me
Quoted 10 lines
10:
e0b8e409af! 9:3dbed2a832sequencer: makeapply_rebase()accept a path
@@ Metadata
Author: Denton Liu <[email protected]>## Commit message ##
- sequencer: makeapply_rebase()accept a path
+ sequencer: makeapply_autostash()accept a path- In order to make
apply_rebase()more generic for future extraction, make
+ In order to makeapply_autostash()more generic for future extraction, make
Thanks for fixing that
Quoted 20 lines
it accept a
pathargument so that the location from where to read the reference to the autostash commit can be customized. Remove theoptsargument since it was unused before anyway.11: a52583beff = 10: 937b4964d1 sequencer: rename stash_sha1 to stash_oid 12: 389b17df33 = 11: 98a7f5280c rebase: use apply_autostash() from sequencer.c 13: fe57e21dbc = 12: 67f3b5c225 rebase: generify reset_head() 14: 045668620d = 13: af5e808667 reset: extract reset_head() from rebase 15: d5f51cd80e = 14: 114d9a7655 rebase: extract create_autostash() 16: 9ab10d23d4 = 15: c2ed4cd39d rebase: generify create_autostash() 17: 26cca49be6 = 16: 3be4a27dfe sequencer: extract perform_autostash() from rebase 18: e703022fda = 17: 129d5d97ae sequencer: unlink autostash in apply_autostash() 19: 75dc3f10a1 ! 18: 7e04ce8d8e sequencer: implement save_autostash()@@ Commit message
function will be used in a future commit.The difference between
save_autostash()andapply_autostash()is that
- the latter does not try to apply the stash. It skips that step and
+ the former does not try to apply the stash. It skips that step and
just stores the created entry in the stash reflog.
Quoted 7 lines
+ This is useful in the case where we abort an operation when an autostash
+ is present but we don't want to dirty the worktree with the application
+ of the stash. For example, in a future commit, we will implement
+git merge --autostash. Since merges can be aborted using
+git reset --hard, we'd make use ofsave_autostash()to save the
+ autostash entry instead of applying it to the worktree thus keeping the
+ worktree undirtied.
The reason for the change is much clearer now thanks
Quoted 34 lines
+ ## sequencer.c ## @@ sequencer.c: void create_autostash(struct repository *r, const char *path, strbuf_release(&buf); 20: 598ddea6c1 = 19: 732b3f9945 sequencer: implement apply_autostash_oid() 21: 7adf794192 ! 20: f9f698b79a merge: teach --autostash option@@ Commit message
git reset --hard, the autostash is saved into the stash reflog instead keeping the worktree clean.+ Helped-by: Phillip Wood <[email protected]>
Suggested-by Alban Gruin <[email protected]>##
Documentation/config/merge.txt##
@@Documentation/git-merge.txt: will be appended to the specified message.
Abort the current conflict resolution process, and
- try to reconstruct the pre-merge state.
+ try to reconstruct the pre-merge state. If an autostash entry is
-+ present, apply it back to the worktree.
++ present, apply it to the worktree.
+
If there were uncommitted worktree changes present when the merge started, 'git merge --abort' will in some cases be unable to
@@Documentation/git-merge.txt: reconstruct these changes. It is therefore recommen
+
'git merge --abort' is equivalent to 'git reset --merge' when
-MERGE_HEADis present.
-+MERGE_HEADis present unlessMERGE_AUTOSTASHis also present where -+the stash entry is applied to the worktree.
++MERGE_HEADis present unlessMERGE_AUTOSTASHis also present in ++which case 'git merge --abort' applies the stash entry to the worktree ++whereas 'git reset --merge' will save the stashed changes in the stash ++reflog.
Quoted 5 lines
--quit::
Forget about the current merge in progress. Leave the index
+- and the working tree as-is.
++ and the working tree as-is. IfMERGE_AUTOSTASHis present, the
++ stash entry will be saved to the stash reflog.
Thanks for adding that
Quoted 4 lines
+
+ --continue::
+ After a 'git merge' stops due to conflicts you can conclude the
Quoted 8 lines
##
Documentation/merge-options.txt##
@@Documentation/merge-options.txt: ifndef::git-pull[]
@@builtin/merge.c: intcmd_merge(int argc, const char **argv, const char *prefix)
die(_("There is no merge to abort (MERGE_HEAD missing)."));+ if (read_oneliner(&stash_oid,
git_path_merge_autostash(the_repository),
-+READ_ONELINER_SKIP_IF_EMPTY)) {
-+strbuf_trim(&stash_oid);
Makes sense, we are in control of the file so don't need to trim the contents
Quoted 44 lines
++ READ_ONELINER_SKIP_IF_EMPTY))
+ unlink(git_path_merge_autostash(the_repository));
-+ }
+
/* Invoke 'git reset --merge' */
ret = cmd_reset(nargc, nargv, prefix);
@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix
N_("add exec lines after each commit of the "
"editable list")),
- ## builtin/reset.c ##
-@@
- #include "cache-tree.h"
- #include "submodule.h"
- #include "submodule-config.h"
-+#include "sequencer.h"
-
- #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
-
-@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)
- if (reset_type == HARD && !update_ref_status && !quiet)
- print_new_head_line(lookup_commit_reference(the_repository, &oid));
- }
-- if (!pathspec.nr)
-+ if (!pathspec.nr) {
- remove_branch_state(the_repository, 0);
-+ }
-
- return update_ref_status;
- }
-
## parse-options.h ##
@@ parse-options.h: int parse_opt_passthru_argv(const struct option *, const char *, int);
#define OPT_CLEANUP(v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))
@@ t/t7600-merge.sh: test_expect_success 'refresh the index before merging' '
+ git diff --exit-code
+'
+
++test_expect_success 'quit merge with --no-commit and --autostash' '
++ git reset --hard c1 &&
++ git merge-file file file.orig file.9 &&
++ git diff >expect &&
++ git merge --no-commit --autostash c2 &&
++ git stash show -p MERGE_AUTOSTASH >actual &&Quoted 8 lines
++ test_cmp expect actual &&
++ git diff HEAD >expect &&
++ git merge --quit 2>err &&
++ test_i18ngrep "Autostash exists; creating a new stash entry." err &&
++ git diff HEAD >actual &&
++ test_cmp expect actual
++'
++Thanks for the new test
Looking at the range-diff this series looks fine to me now apart from the typo Junio pointed out.
Thanks
Phillip
Quoted 6 lines
+test_expect_success 'merge with conflicted --autostash changes' '
+ git reset --hard c1 &&
+ git merge-file file file.orig file.9y &&
22: f1e54622fb = 21: 13f3fadbef t5520: make test_pull_autostash() accept expect_parent_num
23: 177c7e537b = 22: f4fec1e780 pull: pass --autostash to mergeQuoted 2 lines · Phillip Wood
Looking at the range-diff this series looks fine to me now apart from the typo Junio pointed out.
It is quicker for me to do s/NON_EXISTENCE/MISSING/ locally if there is nothing else. Thanks for giving the series another round of look-over.