Skip to contentSkip to conversation

merge: learn --autostash

RFC PATCH Superseded Denton Liu 5 participants 152 messages Oct 16, 2019 – Apr 10, 2020 Page 2 of 2

Subsystems (2)
lore
Share & tools
lore
author
#113 lore raw

PATCH v4 17/23 sequencer: extract perform_autostash() from rebase

In reply to Denton Liu

Lib-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
index e9b05a6cd8..b800c8bfca 100644
@@ -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
index d6ba62a089..cf1284f9ed 100644
@@ -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
author
#114 lore raw

PATCH v4 18/23 sequencer: unlink autostash in apply_autostash()

In reply to Denton Liu

Explicitly 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
index 8efe0976fd..5dd1c2438e 100644
@@ -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
author
#115 lore raw

PATCH v4 19/23 sequencer: implement save_autostash()

In reply to Denton Liu

Extract 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
index 5dd1c2438e..fb52583bc2 100644
@@ -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
index cf1284f9ed..2d09c1ac0b 100644
@@ -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
#116 lore raw

Re: [PATCH v4 19/23] sequencer: implement save_autostash()

Hi Denton

Quoted 6 lines · Denton Liu

Extract 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.

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
---
  sequencer.c | 37 +++++++++++++++++++++++++++----------
  sequencer.h |  1 +
  2 files changed, 28 insertions(+), 10 deletions(-)
sequencer.c +27 -10
index 5dd1c2438e..fb52583bc2 100644
@@ -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
index cf1284f9ed..2d09c1ac0b 100644
@@ -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)
author
#117 lore raw

PATCH v4 20/23 sequencer: implement apply_autostash_oid()

In reply to Denton Liu

Split 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
index fb52583bc2..b4cab9632a 100644
@@ -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
index 2d09c1ac0b..a90b768b74 100644
@@ -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
author
#118 lore raw

PATCH v4 21/23 merge: teach --autostash option

In reply to Denton Liu

In 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
index 6a313937f8..cb2ed58907 100644
@@ -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
index 092529c619..9cb14f0059 100644
@@ -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
index 40dc4f5e8c..3985e6d4a9 100644
@@ -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
index 579494738a..2d9e7675a6 100644
@@ -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
index 7ba33a3bec..c1b25d2954 100644
@@ -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
index d127d2225f..f2f8069f85 100644
@@ -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
index b800c8bfca..e681ff58de 100644
@@ -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
index 18228c312e..0d155101f4 100644
@@ -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
index fece5ba628..46af942093 100644
@@ -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
index 88cf593007..d764738146 100644
@@ -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
index 14d6dcad16..1f1bf8f87a 100644
@@ -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
index d314599428..e29c284b9b 100755
@@ -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
#119 lore raw

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 --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
index 6a313937f8..cb2ed58907 100644
@@ -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
index 092529c619..9cb14f0059 100644

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
index 40dc4f5e8c..3985e6d4a9 100644
@@ -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
index 579494738a..2d9e7675a6 100644
@@ -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
index 7ba33a3bec..c1b25d2954 100644

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 -0
  	UNLEAK(err);
  	UNLEAK(sb);
  	return 0;
builtin/merge.c +2 -0
index d127d2225f..f2f8069f85 100644
@@ -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 -0
  	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"),

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
index b800c8bfca..e681ff58de 100644
@@ -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
index 18228c312e..0d155101f4 100644
@@ -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 -0
  	return update_ref_status;
  }
parse-options.h +1 -0
index fece5ba628..46af942093 100644
@@ -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
index 88cf593007..d764738146 100644
@@ -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
index 14d6dcad16..1f1bf8f87a 100644
@@ -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
index d314599428..e29c284b9b 100755
@@ -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
index 4fa0ef8e3b..d809310df1 100755
@@ -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
author
#120 lore raw

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 index

Should 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

#121 lore raw

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 index

Should 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.

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 --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.

That's a good idea, Thanks

Phillip

Quoted 4 lines

Thanks,

Denton

author
#122 lore raw

PATCH v4 22/23 t5520: make test_pull_autostash() accept expect_parent_num

In reply to Denton Liu

Before, 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
index 2f86fca042..f610dc14de 100755
@@ -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
author
#123 lore raw

PATCH v4 23/23 pull: pass --autostash to merge

In reply to Denton Liu

Before, --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
index dfb901f8b8..ba3772de9f 100644
@@ -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
index 3985e6d4a9..48bfcda084 100644
@@ -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
index 3e624d1e00..9beb4841d1 100644
@@ -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
author
#124 lore raw

v5 cover merge: learn --autostash

In reply to Denton Liu

Alban 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
author
#125 lore raw

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
index 9804a0758b..f709fc3f77 100644
@@ -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
author
#126 lore raw

PATCH v5 2/22 t7600: use test_write_lines()

In reply to Denton Liu

In 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
index 132608879a..4fa0ef8e3b 100755
@@ -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
author
#127 lore raw

PATCH v5 3/22 sequencer: stop leaking buf

In reply to Denton Liu

In 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
index e528225e78..faab0b13e8 100644
@@ -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
author
#128 lore raw

PATCH v5 4/22 sequencer: make file exists check more efficient

In reply to 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 errno to see if the file doesn't exist.

---
 sequencer.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
sequencer.c +2 -4
index faab0b13e8..a961cf5a9b 100644
@@ -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
#129 lore raw

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 errno to see if the file doesn't exist.

---
 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
index faab0b13e8..a961cf5a9b 100644
@@ -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;
 	}
author
#130 lore raw

PATCH v5 5/22 sequencer: make read_oneliner() accept flags

In reply to Denton Liu

In 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
index a961cf5a9b..6c4e8743ef 100644
@@ -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
#131 lore raw

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 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.

Good move. Looked sensible.

Thanks.

author
#132 lore raw

PATCH v5 6/22 sequencer: configurably warn on non-existent files

In reply to Denton Liu

In 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
index 6c4e8743ef..32cc289da3 100644
@@ -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
author
#133 lore raw

PATCH v5 7/22 sequencer: make read_oneliner() extern

In reply to Denton Liu

The 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
index 32cc289da3..0d98ddc33c 100644
@@ -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
index 718a07426d..ab43717fd6 100644
@@ -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
author
#134 lore raw

PATCH v5 8/22 rebase: use read_oneliner()

In reply to Denton Liu

Since 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
index bff53d5d16..57dbfd2a4f 100644
@@ -586,15 +586,6 @@ static const char *state_dir_path(const char *filename, struct rebase_options *o
 	return 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
#135 lore raw

Re: [PATCH v5 08/22] rebase: use read_oneliner()

Quoted 11 lines · Denton Liu

Since 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)

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.

author
#136 lore raw

PATCH v5 9/22 sequencer: make apply_autostash() accept a path

In reply to Denton Liu

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.

---
 sequencer.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
sequencer.c +7 -7
index 0d98ddc33c..4917c70a86 100644
@@ -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 fla
 		todo_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 fla
 	if (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
author
#137 lore raw

PATCH v5 10/22 sequencer: rename stash_sha1 to stash_oid

In reply to Denton Liu

The 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
index 4917c70a86..4a6812e90e 100644
@@ -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
author
#138 lore raw

PATCH v5 11/22 rebase: use apply_autostash() from sequencer.c

In reply to Denton Liu

The 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
index 57dbfd2a4f..cb0bdfc3e4 100644
@@ -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
index 4a6812e90e..f5bb1cc1fb 100644
@@ -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
index ab43717fd6..ab686146d1 100644
@@ -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
author
#139 lore raw

PATCH v5 12/22 rebase: generify reset_head()

In reply to Denton Liu

In 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
index cb0bdfc3e4..5532286142 100644
@@ -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
author
#140 lore raw

PATCH v5 13/22 reset: extract reset_head() from rebase

In reply to Denton Liu

Continue 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
index f709fc3f77..338fb55b73 100644
@@ -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
new file mode 100644
index 0000000000..12f83c78e2
@@ -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
author
#141 lore raw

PATCH v5 14/22 rebase: extract create_autostash()

In reply to Denton Liu

In 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
index 48ef36bd10..581ad3af7b 100644
@@ -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
author
#142 lore raw

PATCH v5 15/22 rebase: generify create_autostash()

In reply to Denton Liu

In 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
index 581ad3af7b..ae345c9e57 100644
@@ -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
author
#143 lore raw

PATCH v5 16/22 sequencer: extract perform_autostash() from rebase

In reply to Denton Liu

Lib-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
index ae345c9e57..08b89869b3 100644
@@ -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
index ab686146d1..9d1fe0ccfb 100644
@@ -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
author
#144 lore raw

PATCH v5 17/22 sequencer: unlink autostash in apply_autostash()

In reply to Denton Liu

Explicitly 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
index e0b8262521..645bc47ce3 100644
@@ -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
author
#145 lore raw

PATCH v5 18/22 sequencer: implement save_autostash()

In reply to Denton Liu

Extract 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
index 645bc47ce3..084ea4117e 100644
@@ -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
index 9d1fe0ccfb..306d4cecfb 100644
@@ -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
author
#146 lore raw

PATCH v5 19/22 sequencer: implement apply_autostash_oid()

In reply to Denton Liu

Split 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
index 084ea4117e..f1eba4cb1f 100644
@@ -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
index 306d4cecfb..5a85eeef92 100644
@@ -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
author
#147 lore raw

PATCH v5 20/22 merge: teach --autostash option

In reply to Denton Liu

In 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
index 6a313937f8..cb2ed58907 100644
@@ -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
index 092529c619..ec06b2f8c2 100644
@@ -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
index 40dc4f5e8c..3985e6d4a9 100644
@@ -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
index 579494738a..2d9e7675a6 100644
@@ -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
index 7ba33a3bec..c1b25d2954 100644
@@ -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
index d127d2225f..f86ae90bfc 100644
@@ -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
index 08b89869b3..bc4fc69906 100644
@@ -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
index fece5ba628..46af942093 100644
@@ -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
index 88cf593007..d764738146 100644
@@ -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
index 14d6dcad16..1f1bf8f87a 100644
@@ -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
index d314599428..e29c284b9b 100755
@@ -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
author
#148 lore raw

PATCH v5 21/22 t5520: make test_pull_autostash() accept expect_parent_num

In reply to Denton Liu

Before, 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
index 2f86fca042..f610dc14de 100755
@@ -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
author
#149 lore raw

PATCH v5 22/22 pull: pass --autostash to merge

In reply to Denton Liu

Before, --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
index dfb901f8b8..ba3772de9f 100644
@@ -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
index 3985e6d4a9..48bfcda084 100644
@@ -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
index 3e624d1e00..9beb4841d1 100644
@@ -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
#150 lore raw
In reply to Denton Liu

Hi Denton

Quoted 9 lines · Denton Liu

[...]
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()'

Quoted 8 lines

* In 'merge: teach --autostash option', improve documentation, remove builtin/reset.c changes, add a git merge --quit test 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: 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);
     - 	}
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: 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

Thanks for fixing that

Quoted 20 lines

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.

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 of save_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]>

## 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.

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. If MERGE_AUTOSTASH is 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: 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);

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 merge
#151 lore raw
Quoted 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.