diff mbox series

[5/6] builtin/stash: provide a way to import stashes from a ref

Message ID 20220310173236.4165310-6-sandals@crustytoothpaste.net (mailing list archive)
State Superseded
Headers show
Series Importing and exporting stashes to refs | expand

Commit Message

brian m. carlson March 10, 2022, 5:32 p.m. UTC
Now that we have a way to export stashes to a ref, let's provide a way
to import them from such a ref back to the stash.  This works much the
way the export code does, except that we strip off the first parent
chain commit and then store each resulting commit back to the stash.

We don't clear the stash first and instead add the specified stashes to
the top of the stash.  This is because users may want to export just a
few stashes, such as to share a small amount of work in progress with a
colleague, and it would be undesirable for the receiving user to lose
all of their data.  For users who do want to replace the stash, it's
easy to do to: simply run "git stash clear" first.

We specifically rely on the fact that we'll produce identical stash
commits on both sides in our tests.  This provides a cheap,
straightforward check for our tests and also makes it easy for users to
see if they already have the same data in both repositories.  Note that
the exported commits don't because we don't write a predictable base
commit, however.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/stash.c  | 125 +++++++++++++++++++++++++++++++++++++++++++++++
 t/t3903-stash.sh |  52 ++++++++++++++++++++
 2 files changed, 177 insertions(+)

Comments

Junio C Hamano March 16, 2022, 5:26 p.m. UTC | #1
"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> @@ -104,6 +109,7 @@ static struct strbuf stash_index_path = STRBUF_INIT;
>   * b_commit is set to the base commit
>   * i_commit is set to the commit containing the index tree
>   * u_commit is set to the commit containing the untracked files tree
> + * c_commit is set to the first parent (chain commit) when importing and is otherwise unset
>   * w_tree is set to the working tree
>   * b_tree is set to the base tree
>   * i_tree is set to the index tree
> @@ -114,6 +120,7 @@ struct stash_info {
>  	struct object_id b_commit;
>  	struct object_id i_commit;
>  	struct object_id u_commit;
> +	struct object_id c_commit;
>  	struct object_id w_tree;
>  	struct object_id b_tree;
>  	struct object_id i_tree;

With the redesign that an exported chain is a series of two-parent
merges, where the first parent is used to string them together in a
single strand of pearls and the second parent is the stash entry,
the above change becomes totally unnecessary, right?  The import
side will be doing a first-parent walk of the export, pushing the
second parent into reflog of refs/stash---we may want sanity check
these second parents with assert_stash_like(), but there is no need
to re-synthesize the stash entries anymore, which would simplify the
implementation quite a bit, right?
		
Namely:

> +static int do_import_stash(const char *rev)
> +{
> +	struct object_id oid;
> +	size_t nitems = 0, nalloc = 0;
> +	struct stash_info *items = NULL;
> +	int res = 0;
> +
> +	if (get_oid(rev, &oid))
> +		return error(_("not a valid revision: %s"), rev);
> +
> +	/*
> +	 * Walk the commit history, finding each stash entry, and load data into
> +	 * the array.
> +	 */
> +	for (size_t i = 0;; i++, nitems++) {
> +		int ret;
> +
> +		if (nalloc <= i) {
> +			size_t new = nalloc * 3 / 2 + 5;
> +			items = xrealloc(items, new * sizeof(*items));
> +			nalloc = new;
> +		}
> +		memset(&items[i], 0, sizeof(*items));
> +		/* We want this to be quiet because it might not exist. */
> +		ret = get_stash_info_for_import(&items[i], &oid);

The new helper function is not necessary; we can use vanilla
get_stash_info() on the second parent to get the same information,
and we do not really need to keep it in core.  We can sanity check
the shape of the imported stash entry right away and discard
everything except for the commit object name.

> +	/*
> +	 * Now, walk each entry, adding it to the stash as a normal stash
> +	 * commit.
> +	 */
> +	for (ssize_t i = nitems - 1; i >= 0; i--) {
> +		struct commit_list *parents = NULL;
> +		struct commit_list **next = &parents;
> +		struct object_id out;
> +		char *msg;
> +
> +		next = commit_list_append(lookup_commit_reference(the_repository, &items[i].b_commit), next);
> +		next = commit_list_append(lookup_commit_reference(the_repository, &items[i].i_commit), next);
> +		if (items[i].has_u)
> +			next = commit_list_append(lookup_commit_reference(the_repository,
> +								   &items[i].u_commit),
> +						  next);
> +
> +		msg = write_commit_with_parents(&out, &items[i], parents);

And this part becomes completely unnecessary if we reuse what the
origin repository already had, which directly can be ...

> +		if (!msg) {
> +			res = -1;
> +			goto out;
> +		}
> +		if (do_store_stash(&out, msg, 1)) {

... fed to this call.
brian m. carlson March 16, 2022, 9:50 p.m. UTC | #2
On 2022-03-16 at 17:26:28, Junio C Hamano wrote:
> > +static int do_import_stash(const char *rev)
> > +{
> > +	struct object_id oid;
> > +	size_t nitems = 0, nalloc = 0;
> > +	struct stash_info *items = NULL;
> > +	int res = 0;
> > +
> > +	if (get_oid(rev, &oid))
> > +		return error(_("not a valid revision: %s"), rev);
> > +
> > +	/*
> > +	 * Walk the commit history, finding each stash entry, and load data into
> > +	 * the array.
> > +	 */
> > +	for (size_t i = 0;; i++, nitems++) {
> > +		int ret;
> > +
> > +		if (nalloc <= i) {
> > +			size_t new = nalloc * 3 / 2 + 5;
> > +			items = xrealloc(items, new * sizeof(*items));
> > +			nalloc = new;
> > +		}
> > +		memset(&items[i], 0, sizeof(*items));
> > +		/* We want this to be quiet because it might not exist. */
> > +		ret = get_stash_info_for_import(&items[i], &oid);
> 
> The new helper function is not necessary; we can use vanilla
> get_stash_info() on the second parent to get the same information,
> and we do not really need to keep it in core.  We can sanity check
> the shape of the imported stash entry right away and discard
> everything except for the commit object name.

Yes, in the new approach, I think we can do that.  We may still need the
behavior which doesn't die on error, but I think we can centralize it.
diff mbox series

Patch

diff --git a/builtin/stash.c b/builtin/stash.c
index 582a04dbab..626e7b8531 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -34,6 +34,7 @@  static const char * const git_stash_usage[] = {
 	N_("git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [<message>]"),
 	N_("git stash export (--print | --to-ref <ref>) [<stashes>]"),
+	N_("git stash import <commit>"),
 	NULL
 };
 
@@ -95,6 +96,10 @@  static const char * const git_stash_export_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_import_usage[] = {
+	N_("git stash import <commit>"),
+	NULL
+};
 
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
@@ -104,6 +109,7 @@  static struct strbuf stash_index_path = STRBUF_INIT;
  * b_commit is set to the base commit
  * i_commit is set to the commit containing the index tree
  * u_commit is set to the commit containing the untracked files tree
+ * c_commit is set to the first parent (chain commit) when importing and is otherwise unset
  * w_tree is set to the working tree
  * b_tree is set to the base tree
  * i_tree is set to the index tree
@@ -114,6 +120,7 @@  struct stash_info {
 	struct object_id b_commit;
 	struct object_id i_commit;
 	struct object_id u_commit;
+	struct object_id c_commit;
 	struct object_id w_tree;
 	struct object_id b_tree;
 	struct object_id i_tree;
@@ -138,6 +145,32 @@  static void assert_stash_like(struct stash_info *info, const char *revision)
 		die(_("'%s' is not a stash-like commit"), revision);
 }
 
+static int get_stash_info_for_import(struct stash_info *info, const struct object_id *oid)
+{
+	int has_parents;
+	char hexoid[GIT_MAX_HEXSZ + 1];
+	oid_to_hex_r(hexoid, oid);
+
+	oidcpy(&info->w_commit, oid);
+	if (get_oidf(&info->w_tree, "%s:", hexoid))
+		return -1;
+
+	has_parents = !get_oidf(&info->c_commit, "%s^1", hexoid);
+
+	/* If this tree is the empty one and we have no parents, we've reached the end. */
+	if (oideq(&info->w_tree, the_hash_algo->empty_tree) && !has_parents)
+		return 1;
+	if (get_oidf(&info->b_commit, "%s^2", hexoid) ||
+	    get_oidf(&info->i_commit, "%s^3", hexoid) ||
+	    get_oidf(&info->b_tree, "%s^2:", hexoid) ||
+	    get_oidf(&info->i_tree, "%s^3:", hexoid))
+		return -1;
+
+	info->has_u = !get_oidf(&info->u_commit, "%s^4", hexoid) &&
+		      !get_oidf(&info->u_tree, "%s^4:", hexoid);
+	return 0;
+}
+
 static int get_stash_info_1(struct stash_info *info, const char *commit, int quiet)
 {
 	int ret;
@@ -1827,6 +1860,96 @@  static char *write_commit_with_parents(struct object_id *out, const struct stash
 	return msg;
 }
 
+static int do_import_stash(const char *rev)
+{
+	struct object_id oid;
+	size_t nitems = 0, nalloc = 0;
+	struct stash_info *items = NULL;
+	int res = 0;
+
+	if (get_oid(rev, &oid))
+		return error(_("not a valid revision: %s"), rev);
+
+	/*
+	 * Walk the commit history, finding each stash entry, and load data into
+	 * the array.
+	 */
+	for (size_t i = 0;; i++, nitems++) {
+		int ret;
+
+		if (nalloc <= i) {
+			size_t new = nalloc * 3 / 2 + 5;
+			items = xrealloc(items, new * sizeof(*items));
+			nalloc = new;
+		}
+		memset(&items[i], 0, sizeof(*items));
+		/* We want this to be quiet because it might not exist. */
+		ret = get_stash_info_for_import(&items[i], &oid);
+		if (ret < 0)
+			return error(_("%s is not a valid exported stash commit"), oid_to_hex(&oid));
+		if (ret)
+			break;
+		oidcpy(&oid, &items[i].c_commit);
+	}
+
+	/*
+	 * Now, walk each entry, adding it to the stash as a normal stash
+	 * commit.
+	 */
+	for (ssize_t i = nitems - 1; i >= 0; i--) {
+		struct commit_list *parents = NULL;
+		struct commit_list **next = &parents;
+		struct object_id out;
+		char *msg;
+
+		next = commit_list_append(lookup_commit_reference(the_repository, &items[i].b_commit), next);
+		next = commit_list_append(lookup_commit_reference(the_repository, &items[i].i_commit), next);
+		if (items[i].has_u)
+			next = commit_list_append(lookup_commit_reference(the_repository,
+								   &items[i].u_commit),
+						  next);
+
+		msg = write_commit_with_parents(&out, &items[i], parents);
+		if (!msg) {
+			res = -1;
+			goto out;
+		}
+		if (do_store_stash(&out, msg, 1)) {
+			free(msg);
+			res = -1;
+			error(_("Cannot save the current status"));
+			goto out;
+		}
+		free(msg);
+	}
+out:
+	for (size_t i = 0; i < nitems; i++) {
+		free_stash_info(&items[i]);
+	}
+	free(items);
+
+	return res;
+}
+
+static int import_stash(int argc, const char **argv, const char *prefix)
+{
+	int ret = 0;
+	struct option options[] = {
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_import_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc != 1)
+		return error(_("a revision to import from is required"));
+
+
+	ret = do_import_stash(argv[0]);
+	return ret;
+}
+
 static int do_export_stash(const char *ref, size_t argc, const char **argv)
 {
 	struct object_id base;
@@ -1996,6 +2119,8 @@  int cmd_stash(int argc, const char **argv, const char *prefix)
 		return !!save_stash(argc, argv, prefix);
 	else if (!strcmp(argv[0], "export"))
 		return !!export_stash(argc, argv, prefix);
+	else if (!strcmp(argv[0], "import"))
+		return !!import_stash(argc, argv, prefix);
 	else if (*argv[0] != '-')
 		usage_msg_optf(_("unknown subcommand: %s"),
 			       git_stash_usage, options, argv[0]);
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index b149e2af44..d2ddede9be 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1295,6 +1295,58 @@  test_expect_success 'stash --keep-index with file deleted in index does not resu
 	test_path_is_missing to-remove
 '
 
+test_expect_success 'stash export and import round-trip stashes' '
+	git reset &&
+	>untracked &&
+	>tracked1 &&
+	>tracked2 &&
+	git add tracked* &&
+	git stash -- &&
+	>subdir/untracked &&
+	>subdir/tracked1 &&
+	>subdir/tracked2 &&
+	git add subdir/tracked* &&
+	git stash -- subdir/ &&
+	stash0=$(git rev-parse --verify stash@{0}) &&
+	stash1=$(git rev-parse --verify stash@{1}) &&
+	simple=$(git stash export --print) &&
+	git stash clear &&
+	git stash import "$simple" &&
+	imported0=$(git rev-parse --verify stash@{0}) &&
+	imported1=$(git rev-parse --verify stash@{1}) &&
+	test "$imported0" = "$stash0" &&
+	test "$imported1" = "$stash1" &&
+	git stash export --to-ref refs/heads/foo &&
+	git stash clear &&
+	git stash import foo &&
+	imported0=$(git rev-parse --verify stash@{0}) &&
+	imported1=$(git rev-parse --verify stash@{1}) &&
+	test "$imported0" = "$stash0" &&
+	test "$imported1" = "$stash1"
+'
+
+test_expect_success 'stash import appends commits' '
+	git log --format=oneline -g refs/stash >actual &&
+	echo $(cat actual | wc -l) >count &&
+	git stash import refs/heads/foo &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = $(($(cat count) * 2)) actual
+'
+
+test_expect_success 'stash export can accept specified stashes' '
+	git stash clear &&
+	git stash import foo &&
+	git stash export --to-ref bar stash@{1} stash@{0} &&
+	git stash clear &&
+	git stash import bar &&
+	imported0=$(git rev-parse --verify stash@{0}) &&
+	imported1=$(git rev-parse --verify stash@{1}) &&
+	test "$imported1" = "$stash0" &&
+	test "$imported0" = "$stash1" &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = 2 actual
+'
+
 test_expect_success 'stash apply should succeed with unmodified file' '
 	echo base >file &&
 	git add file &&