[PATCH 0/2] Prepare (auto)stash for reftable

[PATCH 0/2] Prepare (auto)stash for reftable

[Han-Wen Nienhuys via GitGitGadget]

Stashes are stored as hex OIDs in files under .git/ but are accessed as
refs.

This series consistently treats stashes (AUTOMERGE_STASH,
rebase-merge/autostash etc.) as refs.

Han-Wen Nienhuys (2):
  t3420: prepare for reftable
  Treat MERGE_AUTOSTASH and friends as a pseudoref

 branch.c                    |  2 +-
 builtin/commit.c            |  2 +-
 builtin/merge.c             | 29 +++++++++++------------
 builtin/rebase.c            | 34 ++++++++++++++++++++++-----
 path.c                      |  1 -
 path.h                      |  3 +--
 sequencer.c                 | 46 +++++++++++++++++--------------------
 t/t3420-rebase-autostash.sh | 31 +++++++++++++------------
 8 files changed, 82 insertions(+), 66 deletions(-)


base-commit: a9cd6a0c5981a7d3c9865cf1966243c82f93e58e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1214%2Fhanwen%2Freftable-stash-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1214/hanwen/reftable-stash-v1
Pull-Request: https://github.com/git/git/pull/1214
-- 
gitgitgadget

[PATCH 1/2] t3420: prepare for reftable

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Check for autostash existence using rev-parse; remove the autostash
using test-tool.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t3420-rebase-autostash.sh | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 43fcb68f27e..e6466290e03 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -79,7 +79,7 @@ create_expected_failure_merge () {
 
 testrebase () {
 	type=$1
-	dotest=$2
+	test_type=$2
 
 	test_expect_success "rebase$type: dirty worktree, --no-autostash" '
 		test_config rebase.autostash true &&
@@ -133,9 +133,10 @@ testrebase () {
 		test_when_finished git branch -D rebased-feature-branch &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
-		test_path_is_file $dotest/autostash &&
+		git rev-parse $test_type/autostash &&
 		test_path_is_missing file3 &&
-		rm -rf $dotest &&
+		test-tool ref-store main delete-refs REF_SKIP_REFNAME_VERIFICATION msg $test_type/autostash &&
+		rm -rf .git/$test_type &&
 		git reset --hard &&
 		git checkout feature-branch
 	'
@@ -147,12 +148,12 @@ testrebase () {
 		test_when_finished git branch -D rebased-feature-branch &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
-		test_path_is_file $dotest/autostash &&
+		git rev-parse $test_type/autostash &&
 		test_path_is_missing file3 &&
 		echo "conflicting-plus-goodbye" >file2 &&
 		git add file2 &&
 		git rebase --continue &&
-		test_path_is_missing $dotest/autostash &&
+		test_must_fail git rev-parse $test_type/autostash &&
 		grep dirty file3 &&
 		git checkout feature-branch
 	'
@@ -164,10 +165,10 @@ testrebase () {
 		test_when_finished git branch -D rebased-feature-branch &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
-		test_path_is_file $dotest/autostash &&
+		git rev-parse $test_type/autostash &&
 		test_path_is_missing file3 &&
 		git rebase --skip &&
-		test_path_is_missing $dotest/autostash &&
+		test_must_fail git rev-parse $test_type/autostash &&
 		grep dirty file3 &&
 		git checkout feature-branch
 	'
@@ -179,10 +180,10 @@ testrebase () {
 		test_when_finished git branch -D rebased-feature-branch &&
 		echo dirty >>file3 &&
 		test_must_fail git rebase$type related-onto-branch &&
-		test_path_is_file $dotest/autostash &&
+		git rev-parse $test_type/autostash &&
 		test_path_is_missing file3 &&
 		git rebase --abort &&
-		test_path_is_missing $dotest/autostash &&
+		test_must_fail git rev-parse $test_type/autostash &&
 		grep dirty file3 &&
 		git checkout feature-branch
 	'
@@ -195,11 +196,11 @@ testrebase () {
 		echo dirty >>file3 &&
 		git diff >expect &&
 		test_must_fail git rebase$type related-onto-branch &&
-		test_path_is_file $dotest/autostash &&
+		git rev-parse $test_type/autostash &&
 		test_path_is_missing file3 &&
 		git rebase --quit &&
 		test_when_finished git stash drop &&
-		test_path_is_missing $dotest/autostash &&
+		test_must_fail git rev-parse $test_type/autostash &&
 		! grep dirty file3 &&
 		git stash show -p >actual &&
 		test_cmp expect actual &&
@@ -214,7 +215,7 @@ testrebase () {
 		echo dirty >file4 &&
 		git add file4 &&
 		git rebase$type unrelated-onto-branch >actual 2>&1 &&
-		test_path_is_missing $dotest &&
+		test_must_fail git rev-parse $test_type/autostash &&
 		git reset --hard &&
 		grep unrelated file4 &&
 		! grep dirty file4 &&
@@ -257,9 +258,9 @@ test_expect_success "rebase: noop rebase" '
 	git checkout feature-branch
 '
 
-testrebase " --apply" .git/rebase-apply
-testrebase " --merge" .git/rebase-merge
-testrebase " --interactive" .git/rebase-merge
+testrebase " --apply" rebase-apply
+testrebase " --merge" rebase-merge
+testrebase " --interactive" rebase-merge
 
 test_expect_success 'abort rebase -i with --autostash' '
 	test_when_finished "git reset --hard" &&
-- 
gitgitgadget

[PATCH 2/2] Treat MERGE_AUTOSTASH and friends as a pseudoref

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

In t7600-merge.sh, we can see

        git stash show -p MERGE_AUTOSTASH

which yields

        error: MERGE_AUTOSTASH is not a valid reference

in reftable. stash.c further confirms the stash should be treated as a
revision, by calling setup_revisions() on the first argument to
show_stash()

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 branch.c         |  2 +-
 builtin/commit.c |  2 +-
 builtin/merge.c  | 29 ++++++++++++++---------------
 builtin/rebase.c | 34 ++++++++++++++++++++++++++++------
 path.c           |  1 -
 path.h           |  3 +--
 sequencer.c      | 46 +++++++++++++++++++++-------------------------
 7 files changed, 66 insertions(+), 51 deletions(-)

diff --git a/branch.c b/branch.c
index 6b31df539a5..f506f56de09 100644
--- a/branch.c
+++ b/branch.c
@@ -637,7 +637,7 @@ void remove_merge_branch_state(struct repository *r)
 	unlink(git_path_merge_msg(r));
 	unlink(git_path_merge_mode(r));
 	unlink(git_path_auto_merge(r));
-	save_autostash(git_path_merge_autostash(r));
+	save_autostash(MERGE_AUTOSTASH);
 }
 
 void remove_branch_state(struct repository *r, int verbose)
diff --git a/builtin/commit.c b/builtin/commit.c
index 33ca9e99c80..ef05ecf39df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1855,7 +1855,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 				     &oid, flags);
 	}
 
-	apply_autostash(git_path_merge_autostash(the_repository));
+	apply_autostash(MERGE_AUTOSTASH);
 
 	UNLEAK(err);
 	UNLEAK(sb);
diff --git a/builtin/merge.c b/builtin/merge.c
index a94a03384ae..81f4fa83c24 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -492,7 +492,7 @@ static void finish(struct commit *head_commit,
 	/* Run a post-merge hook */
 	run_hooks_l("post-merge", squash ? "1" : "0", NULL);
 
-	apply_autostash(git_path_merge_autostash(the_repository));
+	apply_autostash(MERGE_AUTOSTASH);
 	strbuf_release(&reflog_message);
 }
 
@@ -1318,8 +1318,8 @@ 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;
-
+		struct object_id stash_oid;
+		char hex[GIT_MAX_HEXSZ + 1] = { 0 };
 		if (orig_argc != 2)
 			usage_msg_opt(_("--abort expects no arguments"),
 			      builtin_merge_usage, builtin_merge_options);
@@ -1327,17 +1327,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));
+		if (!read_ref(MERGE_AUTOSTASH, &stash_oid)) {
+			oid_to_hex_r(hex, &stash_oid);
+			delete_ref("merge --abort", MERGE_AUTOSTASH, NULL, 0);
+		}
 
 		/* Invoke 'git reset --merge' */
 		ret = cmd_reset(nargc, nargv, prefix);
 
-		if (stash_oid.len)
-			apply_autostash_oid(stash_oid.buf);
+		if (hex[0])
+			apply_autostash_oid(hex);
 
-		strbuf_release(&stash_oid);
 		goto done;
 	}
 
@@ -1567,13 +1567,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		if (autostash)
-			create_autostash(the_repository,
-					 git_path_merge_autostash(the_repository));
+			create_autostash(the_repository, MERGE_AUTOSTASH);
 		if (checkout_fast_forward(the_repository,
 					  &head_commit->object.oid,
 					  &commit->object.oid,
 					  overwrite_ignore)) {
-			apply_autostash(git_path_merge_autostash(the_repository));
+			apply_autostash(MERGE_AUTOSTASH);
+
 			ret = 1;
 			goto done;
 		}
@@ -1638,8 +1638,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		die_ff_impossible();
 
 	if (autostash)
-		create_autostash(the_repository,
-				 git_path_merge_autostash(the_repository));
+		create_autostash(the_repository, MERGE_AUTOSTASH);
 
 	/* We are going to make a new commit. */
 	git_committer_info(IDENT_STRICT);
@@ -1722,7 +1721,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		else
 			fprintf(stderr, _("Merge with strategy %s failed.\n"),
 				use_strategies[0]->name);
-		apply_autostash(git_path_merge_autostash(the_repository));
+		apply_autostash(MERGE_AUTOSTASH);
 		ret = 2;
 		goto done;
 	} else if (best_strategy == wt_strategy)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index b29ad2b65e7..e84c21ab8fc 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -414,6 +414,17 @@ static const char *state_dir_path(const char *filename, struct rebase_options *o
 	return path.buf;
 }
 
+static const char *state_pseudoref(const char *suffix,
+				   struct rebase_options *opts)
+{
+	static struct strbuf pseudo_ref = STRBUF_INIT;
+	strbuf_reset(&pseudo_ref);
+	strbuf_addstr(&pseudo_ref, strrchr(opts->state_dir, '/') + 1);
+	strbuf_addstr(&pseudo_ref, "/");
+	strbuf_addstr(&pseudo_ref, suffix);
+	return pseudo_ref.buf;
+}
+
 /* Initialize the rebase options from the state directory. */
 static int read_basic_state(struct rebase_options *opts)
 {
@@ -544,10 +555,11 @@ static int finish_rebase(struct rebase_options *opts)
 {
 	struct strbuf dir = STRBUF_INIT;
 	int ret = 0;
+	const char *autostash_ref = state_pseudoref("autostash", opts);
 
 	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 	unlink(git_path_auto_merge(the_repository));
-	apply_autostash(state_dir_path("autostash", opts));
+	apply_autostash(autostash_ref);
 	/*
 	 * We ignore errors in 'git maintenance run --auto', since the
 	 * user should see them.
@@ -559,6 +571,9 @@ static int finish_rebase(struct rebase_options *opts)
 		replay.action = REPLAY_INTERACTIVE_REBASE;
 		ret = sequencer_remove_state(&replay);
 	} else {
+		if (delete_ref("cleanup autostash", autostash_ref, NULL,
+			       REF_SKIP_REFNAME_VERIFICATION))
+			die("failed cleaning up autostash ref");
 		strbuf_addstr(&dir, opts->state_dir);
 		if (remove_dir_recursively(&dir, 0))
 			ret = error(_("could not remove '%s'"),
@@ -760,8 +775,11 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)
 			finish_rebase(opts);
 	} else if (status == 2) {
 		struct strbuf dir = STRBUF_INIT;
-
-		apply_autostash(state_dir_path("autostash", opts));
+		const char *autostash_ref = state_pseudoref("autostash", opts);
+		apply_autostash(autostash_ref);
+		if (delete_ref("cleanup autostash", autostash_ref, NULL,
+			       REF_SKIP_REFNAME_VERIFICATION))
+			die("failed cleaning up autostash ref");
 		strbuf_addstr(&dir, opts->state_dir);
 		remove_dir_recursively(&dir, 0);
 		strbuf_release(&dir);
@@ -1310,13 +1328,18 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		goto cleanup;
 	}
 	case ACTION_QUIT: {
-		save_autostash(state_dir_path("autostash", &options));
+		const char *autostash_ref =
+			state_pseudoref("autostash", &options);
+		save_autostash(autostash_ref);
 		if (options.type == REBASE_MERGE) {
 			struct replay_opts replay = REPLAY_OPTS_INIT;
 
 			replay.action = REPLAY_INTERACTIVE_REBASE;
 			ret = sequencer_remove_state(&replay);
 		} else {
+			if (delete_ref("deleted autostash_ref", autostash_ref,
+				       NULL, REF_SKIP_REFNAME_VERIFICATION))
+				die("failed to cleanup autostash ref");
 			strbuf_reset(&buf);
 			strbuf_addstr(&buf, options.state_dir);
 			ret = remove_dir_recursively(&buf, 0);
@@ -1670,8 +1693,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
 	if (options.autostash)
 		create_autostash(the_repository,
-				 state_dir_path("autostash", &options));
-
+				 state_pseudoref("autostash", &options));
 
 	if (require_clean_work_tree(the_repository, "rebase",
 				    _("Please commit or stash them."), 1, 1)) {
diff --git a/path.c b/path.c
index 2c895471d90..f69a912a279 100644
--- a/path.c
+++ b/path.c
@@ -1552,7 +1552,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(auto_merge, "AUTO_MERGE")
 REPO_GIT_PATH_FUNC(fetch_head, "FETCH_HEAD")
 REPO_GIT_PATH_FUNC(shallow, "shallow")
diff --git a/path.h b/path.h
index b68691a86b8..e1cf1db98ba 100644
--- a/path.h
+++ b/path.h
@@ -175,7 +175,6 @@ struct path_cache {
 	const char *merge_rr;
 	const char *merge_mode;
 	const char *merge_head;
-	const char *merge_autostash;
 	const char *auto_merge;
 	const char *fetch_head;
 	const char *shallow;
@@ -188,11 +187,11 @@ 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_auto_merge(struct repository *r);
 const char *git_path_fetch_head(struct repository *r);
 const char *git_path_shallow(struct repository *r);
 
+#define MERGE_AUTOSTASH "MERGE_AUTOSTASH"
 
 int ends_with_path_components(const char *path, const char *components);
 
diff --git a/sequencer.c b/sequencer.c
index 35006c0cea6..d61ad1e215d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -160,7 +160,6 @@ static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet")
 static GIT_PATH_FUNC(rebase_path_signoff, "rebase-merge/signoff")
 static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
 static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
-static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash")
 static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
 static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
 static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
@@ -4085,7 +4084,7 @@ static enum todo_command peek_command(struct todo_list *todo_list, int offset)
 	return -1;
 }
 
-void create_autostash(struct repository *r, const char *path)
+void create_autostash(struct repository *r, const char *pseudoref)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct lock_file lock_file = LOCK_INIT;
@@ -4117,10 +4116,10 @@ void create_autostash(struct repository *r, const char *path)
 		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));
+		refs_update_ref(get_main_ref_store(r), "create_autostash",
+				pseudoref, &oid, null_oid(), 0,
+				UPDATE_REFS_DIE_ON_ERR);
+
 		printf(_("Created autostash: %s\n"), buf.buf);
 		if (reset_head(r, &ropts) < 0)
 			die(_("could not reset --hard"));
@@ -4174,33 +4173,30 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply)
 	return ret;
 }
 
-static int apply_save_autostash(const char *path, int attempt_apply)
+static int apply_save_autostash(const char *pseudoref, int attempt_apply)
 {
-	struct strbuf stash_oid = STRBUF_INIT;
+	struct object_id oid;
+	char hex[GIT_MAX_HEXSZ + 1] = { 0 };
 	int ret = 0;
 
-	if (!read_oneliner(&stash_oid, path,
-			   READ_ONELINER_SKIP_IF_EMPTY)) {
-		strbuf_release(&stash_oid);
+	if (read_ref(pseudoref, &oid))
 		return 0;
-	}
-	strbuf_trim(&stash_oid);
 
-	ret = apply_save_autostash_oid(stash_oid.buf, attempt_apply);
+	ret = apply_save_autostash_oid(oid_to_hex_r(hex, &oid), attempt_apply);
 
-	unlink(path);
-	strbuf_release(&stash_oid);
+	delete_ref("save autostash", pseudoref, NULL,
+		   REF_SKIP_REFNAME_VERIFICATION);
 	return ret;
 }
 
-int save_autostash(const char *path)
+int save_autostash(const char *pseudoref)
 {
-	return apply_save_autostash(path, 0);
+	return apply_save_autostash(pseudoref, 0);
 }
 
-int apply_autostash(const char *path)
+int apply_autostash(const char *pseudoref)
 {
-	return apply_save_autostash(path, 1);
+	return apply_save_autostash(pseudoref, 1);
 }
 
 int apply_autostash_oid(const char *stash_oid)
@@ -4222,7 +4218,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
 		.default_reflog_action = "rebase"
 	};
 	if (reset_head(r, &ropts)) {
-		apply_autostash(rebase_path_autostash());
+		apply_autostash("rebase-merge/autostash");
 		sequencer_remove_state(opts);
 		return error(_("could not detach HEAD"));
 	}
@@ -4556,7 +4552,7 @@ cleanup_head_ref:
 				run_command(&hook);
 			}
 		}
-		apply_autostash(rebase_path_autostash());
+		apply_autostash("rebase-merge/autostash");
 
 		if (!opts->quiet) {
 			if (!opts->verbose)
@@ -5635,7 +5631,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(rebase_path_autostash());
+		apply_autostash("rebase-merge/autostash");
 		sequencer_remove_state(opts);
 
 		return error(_("nothing to do"));
@@ -5646,12 +5642,12 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 	if (res == -1)
 		return -1;
 	else if (res == -2) {
-		apply_autostash(rebase_path_autostash());
+		apply_autostash("rebase-merge/autostash");
 		sequencer_remove_state(opts);
 
 		return -1;
 	} else if (res == -3) {
-		apply_autostash(rebase_path_autostash());
+		apply_autostash("rebase-merge/autostash");
 		sequencer_remove_state(opts);
 		todo_list_release(&new_todo);
 
-- 
gitgitgadget