[PATCH v5 0/4] Importing and exporting stashes to refs

[PATCH v5 0/4] Importing and exporting stashes to refs

[brian m. carlson]

Stashes are currently stored using the reflog in a given repository.
This is an interesting and novel way to handle them, but there is no way
to easily move a stash across machines.  For example, stashes cannot be
bundled, pushed, or fetched.

Let's solve this problem by allowing users to import and export stashes
to a chain of commits.  The commits used in a stash export contain two
parents: one which is the pointer to the next exported stash (or to an
empty commit with no parents if there are no more) and the second is the
stash commit that would normally be stored in the reflog.

Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>

Changes from v4:
* Fix another use of oid_array.
* Fix various memory leaks.
* Fix a segfault which appeared after a rebase.
* Use strstr for commits since we don't need to worry about NUL.
* Added some additional tests.
* Verify the ident values we're using to avoid using bad values.
* Various other code cleanups.
* Rebase on `master`.

Changes from v3:
* Fix strbuf handling to avoid leaks and generally be more sensible.
* Make use of the error return code more often.
* Use oid_array.
* Tidy various parts of the code and fix long lines.
* Simplify tests using git tag.
* Shorten and tidy tests.
* Add an additional test covering the base commit OID and importing and
  exporting empty stashes.

Changes from v2:
* Fix uninitialized strbuf.
* Avoid C99-style initializations.

Changes from v1:
* Change storage format as suggested by Junio.
* Rename to GIT_OID_GENTLY.
* Remove unnecessary initializations.
* Use ALLOC_GROW_BY.
* Ensure completely reproducible exports.
* Avoid size_t.
* Various other code cleanups.

brian m. carlson (4):
  object-name: make get_oid quietly return an error
  builtin/stash: factor out revision parsing into a function
  builtin/stash: provide a way to export stashes to a ref
  builtin/stash: provide a way to import stashes from a ref

 Documentation/git-stash.adoc |  29 ++-
 builtin/stash.c              | 339 +++++++++++++++++++++++++++++++++--
 hash.h                       |   1 +
 object-name.c                |   6 +-
 t/t3903-stash.sh             |  78 ++++++++
 5 files changed, 440 insertions(+), 13 deletions(-)

[PATCH v5 1/4] object-name: make get_oid quietly return an error

[brian m. carlson]

A reasonable person looking at the signature and usage of get_oid and
friends might conclude that in the event of an error, it always returns
-1.  However, this is not the case.  Instead, get_oid_basic dies if we
go too far back into the history of a reflog (or, when quiet, simply
exits).

This is not especially useful, since in many cases, we might want to
handle this error differently.  Let's add a flag here to make it just
return -1 like elsewhere in these code paths.

Note that we cannot make this behavior the default, since we have many
other codepaths that rely on the existing behavior, including in tests.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 hash.h        | 1 +
 object-name.c | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hash.h b/hash.h
index d6422ddf45..ec594c63a6 100644
--- a/hash.h
+++ b/hash.h
@@ -216,6 +216,7 @@ struct object_id {
 #define GET_OID_REQUIRE_PATH         010000
 #define GET_OID_HASH_ANY             020000
 #define GET_OID_SKIP_AMBIGUITY_CHECK 040000
+#define GET_OID_GENTLY              0100000
 
 #define GET_OID_DISAMBIGUATORS \
 	(GET_OID_COMMIT | GET_OID_COMMITTISH | \
diff --git a/object-name.c b/object-name.c
index 2c751a5352..3138103343 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 				 * still fill in the oid with the "old" value,
 				 * which we can use.
 				 */
-			} else {
+			} else if (!(flags & GET_OID_GENTLY)) {
 				if (flags & GET_OID_QUIETLY) {
 					exit(128);
 				}
 				die(_("log for '%.*s' only has %d entries"),
 				    len, str, co_cnt);
 			}
+			if (flags & GET_OID_GENTLY) {
+				free(real_ref);
+				return -1;
+			}
 		}
 	}
 

[PATCH v5 2/4] builtin/stash: factor out revision parsing into a function

[brian m. carlson]

We allow several special forms of stash names in this code.  In the
future, we'll want to allow these same forms without parsing a stash
commit, so let's refactor this code out into a function for reuse.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/stash.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index cfbd92852a..8ee6752efa 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -169,6 +169,25 @@ static void assert_stash_like(struct stash_info *info, const char *revision)
 		die(_("'%s' is not a stash-like commit"), revision);
 }
 
+static int parse_revision(struct strbuf *revision, const char *commit, int quiet)
+{
+	strbuf_reset(revision);
+	if (!commit) {
+		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
+			if (!quiet)
+				fprintf_ln(stderr, _("No stash entries found."));
+			return -1;
+		}
+
+		strbuf_addf(revision, "%s@{0}", ref_stash);
+	} else if (strspn(commit, "0123456789") == strlen(commit)) {
+		strbuf_addf(revision, "%s@{%s}", ref_stash, commit);
+	} else {
+		strbuf_addstr(revision, commit);
+	}
+	return 0;
+}
+
 static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 {
 	int ret;
@@ -196,17 +215,10 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 	if (argc == 1)
 		commit = argv[0];
 
-	if (!commit) {
-		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
-			fprintf_ln(stderr, _("No stash entries found."));
-			return -1;
-		}
-
-		strbuf_addf(&info->revision, "%s@{0}", ref_stash);
-	} else if (strspn(commit, "0123456789") == strlen(commit)) {
-		strbuf_addf(&info->revision, "%s@{%s}", ref_stash, commit);
-	} else {
-		strbuf_addstr(&info->revision, commit);
+	strbuf_init(&info->revision, 0);
+	if (parse_revision(&info->revision, commit, 0)) {
+		free_stash_info(info);
+		return -1;
 	}
 
 	revision = info->revision.buf;

[PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

[brian m. carlson]

A common user problem is how to sync in-progress work to another
machine.  Users currently must use some sort of transfer of the working
tree, which poses security risks and also necessarily causes the index
to become dirty.  The experience is suboptimal and frustrating for
users.

A reasonable idea is to use the stash for this purpose, but the stash is
stored in the reflog, not in a ref, and as such it cannot be pushed or
pulled.  This also means that it cannot be saved into a bundle or
preserved elsewhere, which is a problem when using throwaway development
environments.

Let's solve this problem by allowing the user to export the stash to a
ref (or, to just write it into the repository and print the hash, à la
git commit-tree).  Introduce git stash export, which writes a chain of
commits where the first parent is always a chain to the previous stash,
or to a single, empty commit (for the final item) and the second is the
stash commit normally written to the reflog.

Iterate over each stash from topmost to bottomost, looking up the data
for each one, and then create the chain from the single empty commit
back up in reverse order.  Generate a predictable empty commit so our
behavior is reproducible.  Create a useful commit message, preserving
the author and committer information, to help users identify stash
commits when viewing them as normal commits.

If the user has specified specific stashes they'd like to export
instead, use those instead of iterating over all of the stashes.

As part of this, specifically request quiet behavior when looking up the
OID for a revision because we will eventually hit a revision that
doesn't exist and we don't want to die when that occurs.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-stash.adoc |  22 +++-
 builtin/stash.c              | 202 +++++++++++++++++++++++++++++++++++
 2 files changed, 223 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 1a5177f498..e8efd43ba4 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -23,6 +23,7 @@ SYNOPSIS
 'git stash' clear
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
+'git stash' export (--print | --to-ref <ref>) [<stash>...]
 
 DESCRIPTION
 -----------
@@ -154,6 +155,12 @@ store::
 	reflog.  This is intended to be useful for scripts.  It is
 	probably not the command you want to use; see "push" above.
 
+export ( --print | --to-ref <ref> ) [<stash>...]::
+
+	Export the specified stashes, or all of them if none are specified, to
+	a chain of commits which can be transferred using the normal fetch and
+	push mechanisms, then imported using the `import` subcommand.
+
 OPTIONS
 -------
 -a::
@@ -242,6 +249,19 @@ literally (including newlines and quotes).
 +
 Quiet, suppress feedback messages.
 
+--print::
+	This option is only valid for `export`.
++
+Create the chain of commits representing the exported stashes without
+storing it anywhere in the ref namespace and print the object ID to
+standard output.  This is designed for scripts.
+
+--to-ref::
+	This option is only valid for `export`.
++
+Create the chain of commits representing the exported stashes and store
+it to the specified ref.
+
 \--::
 	This option is only valid for `push` command.
 +
@@ -259,7 +279,7 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 <stash>::
 	This option is only valid for `apply`, `branch`, `drop`, `pop`,
-	`show` commands.
+	`show`, and `export` commands.
 +
 A reference of the form `stash@{<revision>}`. When no `<stash>` is
 given, the latest stash is assumed (that is, `stash@{0}`).
diff --git a/builtin/stash.c b/builtin/stash.c
index 8ee6752efa..4a42126d02 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -29,6 +29,8 @@
 #include "diffcore.h"
 #include "reflog.h"
 #include "add-interactive.h"
+#include "oid-array.h"
+#include "commit.h"
 
 #define INCLUDE_ALL_FILES 2
 
@@ -56,6 +58,8 @@
 	   "          [-u | --include-untracked] [-a | --all] [<message>]")
 #define BUILTIN_STASH_CREATE_USAGE \
 	N_("git stash create [<message>]")
+#define BUILTIN_STASH_EXPORT_USAGE \
+	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -71,6 +75,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CLEAR_USAGE,
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
+	BUILTIN_STASH_EXPORT_USAGE,
 	NULL
 };
 
@@ -124,6 +129,12 @@ static const char * const git_stash_save_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_export_usage[] = {
+	BUILTIN_STASH_EXPORT_USAGE,
+	NULL
+};
+
+
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
 
@@ -1896,6 +1907,196 @@ static int save_stash(int argc, const char **argv, const char *prefix,
 	return ret;
 }
 
+static int write_commit_with_parents(struct repository *r,
+				     struct object_id *out,
+				     const struct object_id *oid,
+				     struct commit_list *parents)
+{
+	size_t author_len, committer_len;
+	struct commit *this;
+	const char *orig_author, *orig_committer;
+	char *author = NULL, *committer = NULL;
+	const char *buffer;
+	unsigned long bufsize;
+	const char *p;
+	struct strbuf msg = STRBUF_INIT;
+	int ret = 0;
+	struct ident_split id;
+
+	this = lookup_commit_reference(r, oid);
+	buffer = repo_get_commit_buffer(r, this, &bufsize);
+	orig_author = find_commit_header(buffer, "author", &author_len);
+	orig_committer = find_commit_header(buffer, "committer", &committer_len);
+	if (split_ident_line(&id, orig_author, author_len) < 0 ||
+	    split_ident_line(&id, orig_committer, committer_len) < 0) {
+		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
+		goto out;
+	}
+	p = strstr(buffer, "\n\n");
+
+	if (!orig_author || !orig_committer || !p) {
+		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
+		goto out;
+	}
+	/* Jump to message. */
+	p += 2;
+	strbuf_addstr(&msg, "git stash: ");
+	strbuf_add(&msg, p, bufsize - (p - buffer));
+
+	author = xmemdupz(orig_author, author_len);
+	committer = xmemdupz(orig_committer, committer_len);
+
+	if (commit_tree_extended(msg.buf, msg.len,
+				 r->hash_algo->empty_tree, parents,
+				 out, author, committer,
+				 NULL, NULL)) {
+		ret = error(_("could not write commit"));
+		goto out;
+	}
+out:
+	strbuf_release(&msg);
+	repo_unuse_commit_buffer(r, this, buffer);
+	free_commit_list(parents);
+	free(author);
+	free(committer);
+	return ret;
+}
+
+static int do_export_stash(struct repository *r,
+			   const char *ref,
+			   int argc,
+			   const char **argv)
+{
+	struct object_id base;
+	struct object_context unused;
+	struct commit *prev;
+	struct oid_array items = OID_ARRAY_INIT;
+	int res = 0;
+	int i;
+	struct strbuf revision = STRBUF_INIT;
+	const char *author, *committer;
+
+	/*
+	 * This is an arbitrary, fixed date, specifically the one used by git
+	 * format-patch.  The goal is merely to produce reproducible output.
+	 */
+	prepare_fallback_ident("git stash", "git@stash");
+	author = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			   "2001-09-17T00:00:00Z", 0);
+	committer = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			      "2001-09-17T00:00:00Z", 0);
+
+	/* First, we create a single empty commit. */
+	if (commit_tree_extended("", 0, r->hash_algo->empty_tree, NULL,
+				 &base, author, committer, NULL, NULL))
+		return error(_("unable to write base commit"));
+
+	prev = lookup_commit_reference(r, &base);
+
+	if (argc) {
+		/*
+		 * Find each specified stash, and load data into the array.
+		 */
+		for (i = 0; i < argc; i++) {
+			struct object_id oid;
+			if (parse_revision(&revision, argv[i], 1) ||
+			    get_oid_with_context(r, revision.buf,
+						 GET_OID_QUIETLY | GET_OID_GENTLY,
+						 &oid, &unused)) {
+				res = error(_("unable to find stash entry %s"), argv[i]);
+				goto out;
+			}
+			oid_array_append(&items, &oid);
+		}
+	} else {
+		/*
+		 * Walk the reflog, finding each stash entry, and load data into the
+		 * array.
+		 */
+		for (i = 0;; i++) {
+			char buf[32];
+			struct object_id oid;
+
+			snprintf(buf, sizeof(buf), "%d", i);
+			if (parse_revision(&revision, buf, 1) ||
+			    get_oid_with_context(r, revision.buf,
+						 GET_OID_QUIETLY | GET_OID_GENTLY,
+						 &oid, &unused))
+				break;
+			oid_array_append(&items, &oid);
+		}
+	}
+
+	/*
+	 * Now, create a set of commits identical to the regular stash commits,
+	 * but where their first parents form a chain to our original empty
+	 * base commit.
+	 */
+	for (i = items.nr - 1; i >= 0; i--) {
+		struct commit_list *parents = NULL;
+		struct commit_list **next = &parents;
+		struct object_id out;
+		const struct object_id *oid = items.oid + i;
+
+		next = commit_list_append(prev, next);
+		next = commit_list_append(lookup_commit_reference(r, oid), next);
+		res = write_commit_with_parents(r, &out, oid, parents);
+		if (res)
+			goto out;
+		prev = lookup_commit_reference(r, &out);
+	}
+	if (ref)
+		refs_update_ref(get_main_ref_store(r), NULL, ref,
+				&prev->object.oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	else
+		puts(oid_to_hex(&prev->object.oid));
+out:
+	strbuf_release(&revision);
+	oid_array_clear(&items);
+
+	return res;
+}
+
+enum export_action {
+	ACTION_NONE,
+	ACTION_PRINT,
+	ACTION_TO_REF,
+};
+
+static int export_stash(int argc,
+			const char **argv,
+			const char *prefix,
+			struct repository *repo)
+{
+	const char *ref = NULL;
+	enum export_action action = ACTION_NONE;
+	struct option options[] = {
+		OPT_CMDMODE(0, "print", &action,
+			    N_("print the object ID instead of writing it to a ref"),
+			    ACTION_PRINT),
+		OPT_CMDMODE(0, "to-ref", &action,
+			    N_("save the data to the given ref"),
+			    ACTION_TO_REF),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_export_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (action == ACTION_NONE) {
+		return error(_("exactly one of --print and --to-ref is required"));
+	} else if (action == ACTION_TO_REF) {
+		if (!argc)
+			return error(_("--to-ref requires an argument"));
+		ref = argv[0];
+		argc--;
+		argv++;
+	}
+
+	return do_export_stash(repo, ref, argc, argv);
+}
+
 int cmd_stash(int argc,
 	      const char **argv,
 	      const char *prefix,
@@ -1916,6 +2117,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("store", &fn, store_stash),
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
+		OPT_SUBCOMMAND("export", &fn, export_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};

[PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[brian m. carlson]

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.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-stash.adoc |   7 +++
 builtin/stash.c              | 103 +++++++++++++++++++++++++++++++++++
 t/t3903-stash.sh             |  78 ++++++++++++++++++++++++++
 3 files changed, 188 insertions(+)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index e8efd43ba4..f8193b712e 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -24,6 +24,7 @@ SYNOPSIS
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
 'git stash' export (--print | --to-ref <ref>) [<stash>...]
+'git stash' import <commit>
 
 DESCRIPTION
 -----------
@@ -161,6 +162,12 @@ export ( --print | --to-ref <ref> ) [<stash>...]::
 	a chain of commits which can be transferred using the normal fetch and
 	push mechanisms, then imported using the `import` subcommand.
 
+import <commit>::
+
+	Import the specified stashes from the specified commit, which must have been
+	created by `export`, and add them to the list of stashes.  To replace the
+	existing stashes, use `clear` first.
+
 OPTIONS
 -------
 -a::
diff --git a/builtin/stash.c b/builtin/stash.c
index 4a42126d02..d8332af401 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -60,6 +60,8 @@
 	N_("git stash create [<message>]")
 #define BUILTIN_STASH_EXPORT_USAGE \
 	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
+#define BUILTIN_STASH_IMPORT_USAGE \
+	N_("git stash import <commit>")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -76,6 +78,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
 	BUILTIN_STASH_EXPORT_USAGE,
+	BUILTIN_STASH_IMPORT_USAGE,
 	NULL
 };
 
@@ -134,6 +137,10 @@ static const char * const git_stash_export_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_import_usage[] = {
+	BUILTIN_STASH_IMPORT_USAGE,
+	NULL
+};
 
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
@@ -143,6 +150,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
@@ -153,6 +161,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;
@@ -1962,6 +1971,99 @@ static int write_commit_with_parents(struct repository *r,
 	return ret;
 }
 
+static int do_import_stash(struct repository *r, const char *rev)
+{
+	struct object_id chain;
+	struct oid_array items = OID_ARRAY_INIT;
+	int res = 0;
+	int i;
+	const char *buffer = NULL;
+	struct commit *this = NULL;
+	char *msg = NULL;
+
+	if (repo_get_oid(r, rev, &chain))
+		return error(_("not a valid revision: %s"), rev);
+
+	/*
+	 * Walk the commit history, finding each stash entry, and load data into
+	 * the array.
+	 */
+	for (i = 0;; i++) {
+		struct object_id tree, oid;
+		char revision[GIT_MAX_HEXSZ + 1];
+
+		oid_to_hex_r(revision, &chain);
+
+		if (get_oidf(&tree, "%s:", revision) ||
+		    !oideq(&tree, r->hash_algo->empty_tree)) {
+			res = error(_("%s is not a valid exported stash commit"), revision);
+			goto out;
+		}
+		if (get_oidf(&chain, "%s^1", revision) ||
+		    get_oidf(&oid, "%s^2", revision))
+			break;
+		oid_array_append(&items, &oid);
+	}
+
+	/*
+	 * Now, walk each entry, adding it to the stash as a normal stash
+	 * commit.
+	 */
+	for (i = items.nr - 1; i >= 0; i--) {
+		unsigned long bufsize;
+		const char *p;
+		const struct object_id *oid = items.oid + i;
+
+		this = lookup_commit_reference(r, oid);
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+		if (!buffer) {
+			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p += 2;
+		msg = xmemdupz(p, bufsize - (p - buffer));
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+
+		if (do_store_stash(oid, msg, 1)) {
+			res = error(_("cannot save the stash for %s"), oid_to_hex(oid));
+			goto out;
+		}
+		FREE_AND_NULL(msg);
+	}
+out:
+	if (this && buffer)
+		repo_unuse_commit_buffer(r, this, buffer);
+	oid_array_clear(&items);
+	free(msg);
+
+	return res;
+}
+
+static int import_stash(int argc, const char **argv, const char *prefix,
+			struct repository *repo)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_import_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc != 1)
+		usage_msg_opt("a revision is required", git_stash_import_usage, options);
+
+	return do_import_stash(repo, argv[0]);
+}
+
 static int do_export_stash(struct repository *r,
 			   const char *ref,
 			   int argc,
@@ -2118,6 +2220,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
 		OPT_SUBCOMMAND("export", &fn, export_stash),
+		OPT_SUBCOMMAND("import", &fn, import_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 74666ff3e4..2ab9e5d848 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -11,6 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-unique-files.sh
 
+test_expect_success 'setup' '
+	test_oid_cache <<-EOF
+	export_base sha1:73c9bab443d1f88ac61aa533d2eeaaa15451239c
+	export_base sha256:f210fa6346e3e2ce047bdb570426b17075980c1ac01fec8fc4b75bd3ab4bcfe4
+	EOF
+'
+
 test_expect_success 'usage on cmd and subcommand invalid option' '
 	test_expect_code 129 git stash --invalid-option 2>usage &&
 	grep "or: git stash" usage &&
@@ -1412,6 +1419,77 @@ test_expect_success 'stash --keep-index --include-untracked with empty tree' '
 	)
 '
 
+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 --include-untracked -- subdir/ &&
+	git tag t-stash0 stash@{0} &&
+	git tag t-stash1 stash@{1} &&
+	simple=$(git stash export --print) &&
+	git stash clear &&
+	git stash import "$simple" &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1 &&
+	git stash export --to-ref refs/heads/foo &&
+	test_cmp_rev "$(test_oid empty_tree)" foo: &&
+	test_cmp_rev "$(test_oid empty_tree)" foo^: &&
+	test_cmp_rev t-stash0 foo^2 &&
+	test_cmp_rev t-stash1 foo^^2 &&
+	git log --first-parent --format="%s" refs/heads/foo >log &&
+	grep "^git stash: " log >log2 &&
+	test_line_count = 13 log2 &&
+	git stash clear &&
+	git stash import foo &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1
+'
+
+test_expect_success 'stash import appends commits' '
+	git log --format=oneline -g refs/stash >out &&
+	cat out out >out2 &&
+	git stash import refs/heads/foo &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = $(wc -l <out2) 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 &&
+	test_cmp_rev stash@{1} t-stash0 &&
+	test_cmp_rev stash@{0} t-stash1 &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = 2 actual
+'
+
+test_expect_success 'stash can import and export zero stashes' '
+	git stash clear &&
+	git stash export --to-ref baz &&
+	test_cmp_rev "$(test_oid empty_tree)" baz: &&
+	test_cmp_rev "$(test_oid export_base)" baz &&
+	test_must_fail git rev-parse baz^1 &&
+	git stash import baz &&
+	test_must_fail git rev-parse refs/stash
+'
+
+test_expect_success 'stash rejects invalid attempts to import commits' '
+	git stash import foo &&
+	test_must_fail git stash import HEAD 2>output &&
+	oid=$(git rev-parse HEAD) &&
+	grep "$oid is not a valid exported stash commit" output &&
+	test_cmp_rev stash@{0} t-stash0
+'
+
 test_expect_success 'stash apply should succeed with unmodified file' '
 	echo base >file &&
 	git add file &&

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Stashes are currently stored using the reflog in a given repository.
> This is an interesting and novel way to handle them, but there is no way
> to easily move a stash across machines.  For example, stashes cannot be
> bundled, pushed, or fetched.
>
> Let's solve this problem by allowing users to import and export stashes
> to a chain of commits.  The commits used in a stash export contain two
> parents: one which is the pointer to the next exported stash (or to an
> empty commit with no parents if there are no more) and the second is the
> stash commit that would normally be stored in the reflog.
>
> Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>

Thanks for resurrecting the thread.  Once this part gets sorted out
and the outside-reflog representation of stash becomes stable, the
next logical step would be to teach the "git stash create" and
friends to directly work on the new format, perhaps?  But first
things first---export+import would certainly be a logical first
step.

Re: [PATCH v5 1/4] object-name: make get_oid quietly return an error

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> diff --git a/object-name.c b/object-name.c
> index 2c751a5352..3138103343 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
>  				 * still fill in the oid with the "old" value,
>  				 * which we can use.
>  				 */
> -			} else {
> +			} else if (!(flags & GET_OID_GENTLY)) {
>  				if (flags & GET_OID_QUIETLY) {
>  					exit(128);
>  				}
>  				die(_("log for '%.*s' only has %d entries"),
>  				    len, str, co_cnt);
>  			}
> +			if (flags & GET_OID_GENTLY) {
> +				free(real_ref);
> +				return -1;
> +			}
>  		}
>  	}
>  

Almost everybody else in this function hits "return -1" after
detecting that it cannot yield a valid object name, and this change
makes the oddball case do the same.

Ideally in a distant past, we might want to remove this _GENTLY
flag, together with the code path that is not so gentle, and adjust
the callers that depend on the current behaviour (which I somehow
doubt--- they need to be prepared to deal with the error return from
other parts of the same function already).  We might need to make it
possible for callers to tell which error condition we got (e.g., did
the input give it a non-existing ref?  did reflog walk run out?),
but these (including to the change to just lose "die" and always go
the GENTLY code path) are totally outside the scope of this series.

Thanks.

Re: [PATCH v5 2/4] builtin/stash: factor out revision parsing into a function

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> We allow several special forms of stash names in this code.  In the
> future, we'll want to allow these same forms without parsing a stash
> commit, so let's refactor this code out into a function for reuse.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/stash.c | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index cfbd92852a..8ee6752efa 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -169,6 +169,25 @@ static void assert_stash_like(struct stash_info *info, const char *revision)
>  		die(_("'%s' is not a stash-like commit"), revision);
>  }
>  
> +static int parse_revision(struct strbuf *revision, const char *commit, int quiet)
> +{
> +	strbuf_reset(revision);
> +	if (!commit) {
> +		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
> +			if (!quiet)
> +				fprintf_ln(stderr, _("No stash entries found."));
> +			return -1;
> +		}
> +
> +		strbuf_addf(revision, "%s@{0}", ref_stash);
> +	} else if (strspn(commit, "0123456789") == strlen(commit)) {
> +		strbuf_addf(revision, "%s@{%s}", ref_stash, commit);
> +	} else {
> +		strbuf_addstr(revision, commit);
> +	}
> +	return 0;
> +}
> +
>  static int get_stash_info(struct stash_info *info, int argc, const char **argv)
>  {
>  	int ret;
> @@ -196,17 +215,10 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
>  	if (argc == 1)
>  		commit = argv[0];
>  
> -	if (!commit) {
> -		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
> -			fprintf_ln(stderr, _("No stash entries found."));
> -			return -1;
> -		}
> -
> -		strbuf_addf(&info->revision, "%s@{0}", ref_stash);
> -	} else if (strspn(commit, "0123456789") == strlen(commit)) {
> -		strbuf_addf(&info->revision, "%s@{%s}", ref_stash, commit);
> -	} else {
> -		strbuf_addstr(&info->revision, commit);
> +	strbuf_init(&info->revision, 0);
> +	if (parse_revision(&info->revision, commit, 0)) {
> +		free_stash_info(info);
> +		return -1;
>  	}

Two comments:

 - It is file-scope static so it is not a huge deal, but it is a bit
   confusing that parse_revision() sounds like a helper function you
   would have in revision.c and call from everywhere.

 - The call to free_stash_info() from inside get_stash_info() is
   probably wrong.  The early error return when ref_stash is missing
   leaves info intact, and the callers of it share this pattern:

        if (get_stash_info(&info, argc - 1, argv + 1))
                goto cleanup;
        ...
    cleanup:
        free_stash_info(&info);
        return ret;

   Even if free_stah_info() happen to be idempotent right now, I do
   not think we want our code to rely on it.

Re: [PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> +static int write_commit_with_parents(struct repository *r,
> +				     struct object_id *out,
> +				     const struct object_id *oid,
> +				     struct commit_list *parents)
> +{
> +	size_t author_len, committer_len;
> +	struct commit *this;
> +	const char *orig_author, *orig_committer;
> +	char *author = NULL, *committer = NULL;
> +	const char *buffer;
> +	unsigned long bufsize;
> +	const char *p;
> +	struct strbuf msg = STRBUF_INIT;
> +	int ret = 0;
> +	struct ident_split id;
> +
> +	this = lookup_commit_reference(r, oid);
> +	buffer = repo_get_commit_buffer(r, this, &bufsize);
> +	orig_author = find_commit_header(buffer, "author", &author_len);
> +	orig_committer = find_commit_header(buffer, "committer", &committer_len);
> +	if (split_ident_line(&id, orig_author, author_len) < 0 ||
> +	    split_ident_line(&id, orig_committer, committer_len) < 0) {
> +		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +	p = strstr(buffer, "\n\n");
> +
> +	if (!orig_author || !orig_committer || !p) {
> +		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +		goto out;
> +	}

It is too late to check the NULL-ness of orig_author and
orig_committer here.  They have already been used without checking
for their NULL-ness to call split_ident_line() that happily will
dereference its second "const char *line" parameter, so we would
have already segfaulted.

As fsck.c::verify_headers() say, it is not a crime to lack the
"\n\n" after the last header item, if the commit truly lacks any
message.  So '!p' is a bit overly strict, but in practice I do not
think our tools saved a byte by omitting the empty line after the
header even when creating a commit with an empty message for a long
time, so this may be OK.  On the other hand, preparing for a stash
entry a third-party reimplementation prepared would not be too hard
to do here.

	if (!orig_author || !orig_committer) {
		ret = error(_("cannot parse..."));
		goto out;
	}
	if (split_ident_line(...) < 0 ||
	    split_ident_line(...) < 0) {
		ret = error(_("invalid au..."));
		goto out;
	}

	p = strstr(buffer, "\n\n");
	strbuf_addstr(&msg, "git stash: ");

	if (p)
		strbuf_add(&msg, p+2, bufsize - (p + 2 - buffer));
	strbuf_complete_line(&mesg);

or something.

> +	/* Jump to message. */
> +	p += 2;
> +	strbuf_addstr(&msg, "git stash: ");
> +	strbuf_add(&msg, p, bufsize - (p - buffer));

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[D. Ben Knoble]

On Thu, May 8, 2025 at 7:47 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> Stashes are currently stored using the reflog in a given repository.
> This is an interesting and novel way to handle them, but there is no way
> to easily move a stash across machines.  For example, stashes cannot be
> bundled, pushed, or fetched.

I'm probably missing something here after having skimmed the original
cover letter at
<20220310173236.4165310-1-sandals@crustytoothpaste.net>, but:

- Can't we "git push <remote> stash[@{n}]:<branch>" to share a stash somewhere?
- And then, doesn't "git stash apply [--index] <arbitrary sha>" work?
(At which point you could presumably create a new stash, though I'll
admit that's cumbersome relative to dedicated export/import.)

I can see how that doesn't help you quickly export a whole _chain_ of
stashes, so I'm not saying "this series seems like the wrong tack"
(far be it from me, who doesn't understand your use case, to say
that!). Rather, I'm confused about the inability to move a (single)
stash across machines.

Unrelated question: Can we import arbitrary refs into stashes? That
is, what happens if the commit structure doesn't look right? (Maybe I
should go read the tests and see.)

-- 
D. Ben Knoble

Re: [PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> +static int do_export_stash(struct repository *r,
> +			   const char *ref,
> +			   int argc,
> +			   const char **argv)
> +{
> +...
> +	if (argc) {
> +		/*
> +		 * Find each specified stash, and load data into the array.
> +		 */
> +		for (i = 0; i < argc; i++) {
> +			struct object_id oid;
> +			if (parse_revision(&revision, argv[i], 1) ||
> +			    get_oid_with_context(r, revision.buf,
> +						 GET_OID_QUIETLY | GET_OID_GENTLY,
> +						 &oid, &unused)) {
> +				res = error(_("unable to find stash entry %s"), argv[i]);
> +				goto out;
> +			}
> +			oid_array_append(&items, &oid);
> +		}

Grabbing individual reflog entry given on the command line looks
trivial, but ...

> +	} else {
> +		/*
> +		 * Walk the reflog, finding each stash entry, and load data into the
> +		 * array.
> +		 */
> +		for (i = 0;; i++) {
> +			char buf[32];
> +			struct object_id oid;
> +
> +			snprintf(buf, sizeof(buf), "%d", i);
> +			if (parse_revision(&revision, buf, 1) ||
> +			    get_oid_with_context(r, revision.buf,
> +						 GET_OID_QUIETLY | GET_OID_GENTLY,
> +						 &oid, &unused))
> +				break;
> +			oid_array_append(&items, &oid);
> +		}

... have you considered reusing reflog-walk.c:read_complete_reflog()
as a helper function?  

Doing so of would be more efficient than going from int to string
back to int to call read_ref_at() and iterate over the same reflog
entries with refs_for_each_reflog_ent().

> +	}
> +
> +	/*
> +	 * Now, create a set of commits identical to the regular stash commits,
> +	 * but where their first parents form a chain to our original empty
> +	 * base commit.
> +	 */
> +	for (i = items.nr - 1; i >= 0; i--) {
> +		struct commit_list *parents = NULL;
> +		struct commit_list **next = &parents;
> +		struct object_id out;
> +		const struct object_id *oid = items.oid + i;
> +
> +		next = commit_list_append(prev, next);
> +		next = commit_list_append(lookup_commit_reference(r, oid), next);

The individual-reflog-entry mode above was fairly strict in that a
list of reflog entries with even one unreadable commit caused the
whole command to fail, but reflog-walk mode assumed that a failure
to read an entry must always be due to reflog entries running out
due to the index incremented to a large enough number.  I suspect
get_oid_with_context() can give you oid obtained out of a reflog
entry without actually parsing the object or checking if it exists.

Should we be a bit more defensinve here in lookup_commit_reference()
call, which would silently throw a NULL back at us if the commit
cannot be found without complaining?

Re: [PATCH v5 1/4] object-name: make get_oid quietly return an error

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

On 2025-05-09 at 01:55:23, Junio C Hamano wrote:
> Almost everybody else in this function hits "return -1" after
> detecting that it cannot yield a valid object name, and this change
> makes the oddball case do the same.
> 
> Ideally in a distant past, we might want to remove this _GENTLY
> flag, together with the code path that is not so gentle, and adjust
> the callers that depend on the current behaviour (which I somehow
> doubt--- they need to be prepared to deal with the error return from
> other parts of the same function already).  We might need to make it
> possible for callers to tell which error condition we got (e.g., did
> the input give it a non-existing ref?  did reflog walk run out?),
> but these (including to the change to just lose "die" and always go
> the GENTLY code path) are totally outside the scope of this series.

Yes, I think we may have discussed doing this in the past and I
originally considered doing so, only to realize that was an entire
series to itself and would be pretty disruptive.  Last I looked, we
unfortunately relied on this case more than you might imagine.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> +static int do_import_stash(struct repository *r, const char *rev)
> +{
> +	struct object_id chain;
> +	struct oid_array items = OID_ARRAY_INIT;
> +	int res = 0;
> +	int i;
> +	const char *buffer = NULL;
> +	struct commit *this = NULL;
> +	char *msg = NULL;
> +
> +	if (repo_get_oid(r, rev, &chain))
> +		return error(_("not a valid revision: %s"), rev);
> +
> +	/*
> +	 * Walk the commit history, finding each stash entry, and load data into
> +	 * the array.
> +	 */
> +	for (i = 0;; i++) {
> +		struct object_id tree, oid;
> +		char revision[GIT_MAX_HEXSZ + 1];
> +
> +		oid_to_hex_r(revision, &chain);
> +
> +		if (get_oidf(&tree, "%s:", revision) ||
> +		    !oideq(&tree, r->hash_algo->empty_tree)) {
> +			res = error(_("%s is not a valid exported stash commit"), revision);
> +			goto out;
> +		}
> +		if (get_oidf(&chain, "%s^1", revision) ||
> +		    get_oidf(&oid, "%s^2", revision))
> +			break;

This is to stop at the sentinel commit at the end.  Don't we want to
make sure that it actually has the expected shape of the sentinel?

IOW, how robust do we try to be against being fed a random mergy
commit history (e.g., our 'master') and mistakenly adding nonsense
stash entries as refs/stash@{<n>}?

> +	/*
> +	 * Now, walk each entry, adding it to the stash as a normal stash
> +	 * commit.
> +	 */
> +	for (i = items.nr - 1; i >= 0; i--) {
> +		unsigned long bufsize;
> +		const char *p;
> +		const struct object_id *oid = items.oid + i;
> +
> +		this = lookup_commit_reference(r, oid);
> +		buffer = repo_get_commit_buffer(r, this, &bufsize);
> +		if (!buffer) {
> +			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
> +			goto out;
> +		}
> +
> +		p = strstr(buffer, "\n\n");
> +		if (!p) {
> +			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +			goto out;
> +		}
> +
> +		p += 2;
> +		msg = xmemdupz(p, bufsize - (p - buffer));

Here, we could check "git stash: " string to make sure that it is as
expected in an exported stash we previously made, and abort, just
like the above "cannot parse" case.

> +		repo_unuse_commit_buffer(r, this, buffer);
> +		buffer = NULL;
> +
> +		if (do_store_stash(oid, msg, 1)) {

Should we be making sure that the object named by "oid" does look
like a stash, like what is done in builtin/stash.c:get_stash_info(),
or is assert_stash_like() called from do_store_stash() sufficient?

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]

On 2025-05-09 at 16:53:17, D. Ben Knoble wrote:
> - Can't we "git push <remote> stash[@{n}]:<branch>" to share a stash somewhere?
> - And then, doesn't "git stash apply [--index] <arbitrary sha>" work?
> (At which point you could presumably create a new stash, though I'll
> admit that's cumbersome relative to dedicated export/import.)

I haven't tried, but it does certainly seem plausible that you can
import and export them in that way.

> I can see how that doesn't help you quickly export a whole _chain_ of
> stashes, so I'm not saying "this series seems like the wrong tack"
> (far be it from me, who doesn't understand your use case, to say
> that!). Rather, I'm confused about the inability to move a (single)
> stash across machines.

Let me explain the intended use case here.  At work, many people use
GitHub Codespaces, which are throwaway development environments.  Since
one's whole set of stashes cannot be imported or exported, it's hard to
use stashes effectively in such a case, since they'd be deleted when you
destroyed the environment.  I like stashes a lot (my personal Git
development repository has 153), so I want to synchronize all of them
across.

The other case is people who routinely work on multiple machines.
(Remote or throwaway development environments, like GitHub Codespaces or
Devcontainers, are just a special case of this.)  Many of these users
want to keep their working tree and other state across machines and lots
of them rely on cloud syncing services, such as Dropbox, to do this,
which often ends up corrupting the repository (as outlined in the FAQ).
Providing a way to quickly and easily synchronize the working tree
across systems, including any stashes, is really important to encourage
best practices that don't result in data loss or have unpleasant
security issues (such as untrusted local config).

> Unrelated question: Can we import arbitrary refs into stashes? That
> is, what happens if the commit structure doesn't look right? (Maybe I
> should go read the tests and see.)

That doesn't work because the commit used here has to have a fixed
number of parents, since we need to keep track of the index and the
working tree.  Stash commits, even the regular ones used in the reflog,
always have to have a certain structure.

If you try to do that anyway, you get this message:

    % git stash import HEAD
    error: 3bf235c35ef51d01663f2ab9665026b05b8af1dd is not a valid exported stash commit

I did try to avoid people accidentally destroying data.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On 2025-05-09 at 01:10:59, Junio C Hamano wrote:
> Thanks for resurrecting the thread.  Once this part gets sorted out
> and the outside-reflog representation of stash becomes stable, the
> next logical step would be to teach the "git stash create" and
> friends to directly work on the new format, perhaps?  But first
> things first---export+import would certainly be a logical first
> step.

That could certainly be a follow-up, yes.  I don't currently have any
plans to do that, but of course someone else could or I could be
convinced that it's something I should pick up.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

[PATCH] Makefile: avoid constant rebuilds with compilation database

[brian m. carlson]

Many contributors to software use a Language Server Protocol
implementation to allow their editor to learn structural information
about the code they write and provide additional features, such as
jumping to the declaration or definition of a function or type.  In C,
the usual implementation is clangd, which requires compiling with clang.

Because C and C++ projects lack a standard file system layout and build
system, unlike languages such as Rust and Go, clangd requires a
compilation database to be generated by the clang compiler in order to
pass the proper compilation flags and discover all of the files
necessary to make the LSP work.  This is done by setting
GENERATE_COMPILATION_DATABASE to "yes".

However, when that's enabled and the user runs "make" a second time,
all of the files are re-compiled, which is inconvenient for contributors
to Git, since it makes small changes or rebases recompile the entirety
of the codebase.  This happens because the directory holding the
compilation database is updated anytime an object is built, so its
modification date will always be newer than the first object built.

To solve this, use the same trick we do just above for the .depend
directory and filter the compilation database directory out if it
already exists, which avoids making it a target to be built.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Helped-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8a7f1c7654..8f38de8ebf 100644
--- a/Makefile
+++ b/Makefile
@@ -2804,7 +2804,7 @@ endif
 compdb_dir = compile_commands
 
 ifeq ($(GENERATE_COMPILATION_DATABASE),yes)
-missing_compdb_dir = $(compdb_dir)
+missing_compdb_dir = $(filter-out $(wildcard $(compdb_dir)), $(compdb_dir))
 $(missing_compdb_dir):
 	@mkdir -p $@
 

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Jeff King]

On Thu, May 08, 2025 at 11:44:57PM +0000, brian m. carlson wrote:

> +test_expect_success 'stash export and import round-trip stashes' '
> [...]
> +	git stash export --to-ref refs/heads/foo &&

Here we export to a name in the refs/heads/ namespace...

> +test_expect_success 'stash export can accept specified stashes' '
> [...]
> +	git stash export --to-ref bar stash@{1} stash@{0} &&

...but here we are writing to the top-level .git/bar. We do currently
allow that, but there's been discussion of locking this down a bit
further (requiring BAR or even BAR_HEAD at the top-level). Should this
be refs/heads/bar?

> +test_expect_success 'stash can import and export zero stashes' '
> [...]
> +	git stash export --to-ref baz &&

Ditto here.

I noticed because I have a patch series from last summer tightening
these rules (it got derailed by some conflicting work, and I've been
meaning to pick it back up). I can certainly adjust these tests as part
of that series, but if you're re-rolling anyway, it might be nice to do
it now.

-Peff

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[D. Ben Knoble]

On Fri, May 9, 2025 at 4:15 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2025-05-09 at 16:53:17, D. Ben Knoble wrote:
> > - Can't we "git push <remote> stash[@{n}]:<branch>" to share a stash somewhere?
> > - And then, doesn't "git stash apply [--index] <arbitrary sha>" work?
> > (At which point you could presumably create a new stash, though I'll
> > admit that's cumbersome relative to dedicated export/import.)
>
> I haven't tried, but it does certainly seem plausible that you can
> import and export them in that way.
>
> > I can see how that doesn't help you quickly export a whole _chain_ of
> > stashes, so I'm not saying "this series seems like the wrong tack"
> > (far be it from me, who doesn't understand your use case, to say
> > that!). Rather, I'm confused about the inability to move a (single)
> > stash across machines.
>
> Let me explain the intended use case here.  At work, many people use
> GitHub Codespaces, which are throwaway development environments.  Since
> one's whole set of stashes cannot be imported or exported, it's hard to
> use stashes effectively in such a case, since they'd be deleted when you
> destroyed the environment.  I like stashes a lot (my personal Git
> development repository has 153), so I want to synchronize all of them
> across.
>
> The other case is people who routinely work on multiple machines.
> (Remote or throwaway development environments, like GitHub Codespaces or
> Devcontainers, are just a special case of this.)  Many of these users
> want to keep their working tree and other state across machines and lots
> of them rely on cloud syncing services, such as Dropbox, to do this,
> which often ends up corrupting the repository (as outlined in the FAQ).
> Providing a way to quickly and easily synchronize the working tree
> across systems, including any stashes, is really important to encourage
> best practices that don't result in data loss or have unpleasant
> security issues (such as untrusted local config).
>
> > Unrelated question: Can we import arbitrary refs into stashes? That
> > is, what happens if the commit structure doesn't look right? (Maybe I
> > should go read the tests and see.)
>
> That doesn't work because the commit used here has to have a fixed
> number of parents, since we need to keep track of the index and the
> working tree.  Stash commits, even the regular ones used in the reflog,
> always have to have a certain structure.
>
> If you try to do that anyway, you get this message:
>
>     % git stash import HEAD
>     error: 3bf235c35ef51d01663f2ab9665026b05b8af1dd is not a valid exported stash commit
>
> I did try to avoid people accidentally destroying data.
> --
> brian m. carlson (they/them)
> Toronto, Ontario, CA

You've answered all my questions, thanks!

-- 
D. Ben Knoble

Re: [PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

[Jeff King]

On Thu, May 08, 2025 at 11:44:56PM +0000, brian m. carlson wrote:

> +static int write_commit_with_parents(struct repository *r,
> +				     struct object_id *out,
> +				     const struct object_id *oid,
> +				     struct commit_list *parents)
> +{
> +	size_t author_len, committer_len;
> +	struct commit *this;
> +	const char *orig_author, *orig_committer;
> +	char *author = NULL, *committer = NULL;
> +	const char *buffer;
> +	unsigned long bufsize;
> +	const char *p;
> +	struct strbuf msg = STRBUF_INIT;
> +	int ret = 0;
> +	struct ident_split id;
> +
> +	this = lookup_commit_reference(r, oid);
> +	buffer = repo_get_commit_buffer(r, this, &bufsize);
> +	orig_author = find_commit_header(buffer, "author", &author_len);
> +	orig_committer = find_commit_header(buffer, "committer", &committer_len);
> +	if (split_ident_line(&id, orig_author, author_len) < 0 ||
> +	    split_ident_line(&id, orig_committer, committer_len) < 0) {
> +		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +	p = strstr(buffer, "\n\n");
> +
> +	if (!orig_author || !orig_committer || !p) {
> +		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +		goto out;
> +	}

Coverity flagged this as a potential NULL deref. We check that
orig_author and orig_committer aren't NULL here, but we'll already have
looked at them via split_ident_line() above. Probably the error checks
should be reordered?

-Peff

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Jeff King]

On Thu, May 08, 2025 at 11:44:57PM +0000, brian m. carlson wrote:

> +static int do_import_stash(struct repository *r, const char *rev)
> [...]
> +	struct oid_array items = OID_ARRAY_INIT;
> +	int i;
> [...]
> +	/*
> +	 * Walk the commit history, finding each stash entry, and load data into
> +	 * the array.
> +	 */
> +	for (i = 0;; i++) {
> +		struct object_id tree, oid;
> +		char revision[GIT_MAX_HEXSZ + 1];
> +
> +		oid_to_hex_r(revision, &chain);
> +
> +		if (get_oidf(&tree, "%s:", revision) ||
> +		    !oideq(&tree, r->hash_algo->empty_tree)) {
> +			res = error(_("%s is not a valid exported stash commit"), revision);
> +			goto out;
> +		}
> +		if (get_oidf(&chain, "%s^1", revision) ||
> +		    get_oidf(&oid, "%s^2", revision))
> +			break;
> +		oid_array_append(&items, &oid);
> +	}
> +
> +	/*
> +	 * Now, walk each entry, adding it to the stash as a normal stash
> +	 * commit.
> +	 */
> +	for (i = items.nr - 1; i >= 0; i--) {

Coverity complains about possible integer overflow here. It's an
interesting case. items.nr is a size_t, coming from the oid_array, and
so it's unsigned. You use a signed int to iterate, which is needed to
catch walking past the zero. But in that initial assignment, the
subtraction of 1 is done on an unsigned value. If items.nr is zero, then
it wraps around to a big (usually 64-bit) number, which is then
truncated and forced into a signed 32-bit int.

I _think_ that usually works out, because the overflowed size_t is going
to be all-bits-1, and then the truncation to int is also all-bits-1,
which taken as a signed value is -1.

Probably there's some light violation of the standard there, but I think
it should be OK. But I thought I'd mention it in case I'm missing
something.

-Peff

PS Sorry for the flurry of emails on a v5; this hit jch, so it got
   sucked into my usual testing / analysis flow.

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3169 bytes --]

On 2025-05-09 at 19:54:12, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > +static int do_import_stash(struct repository *r, const char *rev)
> > +{
> > +	struct object_id chain;
> > +	struct oid_array items = OID_ARRAY_INIT;
> > +	int res = 0;
> > +	int i;
> > +	const char *buffer = NULL;
> > +	struct commit *this = NULL;
> > +	char *msg = NULL;
> > +
> > +	if (repo_get_oid(r, rev, &chain))
> > +		return error(_("not a valid revision: %s"), rev);
> > +
> > +	/*
> > +	 * Walk the commit history, finding each stash entry, and load data into
> > +	 * the array.
> > +	 */
> > +	for (i = 0;; i++) {
> > +		struct object_id tree, oid;
> > +		char revision[GIT_MAX_HEXSZ + 1];
> > +
> > +		oid_to_hex_r(revision, &chain);
> > +
> > +		if (get_oidf(&tree, "%s:", revision) ||
> > +		    !oideq(&tree, r->hash_algo->empty_tree)) {
> > +			res = error(_("%s is not a valid exported stash commit"), revision);
> > +			goto out;
> > +		}
> > +		if (get_oidf(&chain, "%s^1", revision) ||
> > +		    get_oidf(&oid, "%s^2", revision))
> > +			break;
> 
> This is to stop at the sentinel commit at the end.  Don't we want to
> make sure that it actually has the expected shape of the sentinel?
> 
> IOW, how robust do we try to be against being fed a random mergy
> commit history (e.g., our 'master') and mistakenly adding nonsense
> stash entries as refs/stash@{<n>}?

I'll try to make this more robust.  The advantage of the current
approach is that we don't have to parse the buffer, but once we need to
do that, we might as well do the entire thing.

> > +	/*
> > +	 * Now, walk each entry, adding it to the stash as a normal stash
> > +	 * commit.
> > +	 */
> > +	for (i = items.nr - 1; i >= 0; i--) {
> > +		unsigned long bufsize;
> > +		const char *p;
> > +		const struct object_id *oid = items.oid + i;
> > +
> > +		this = lookup_commit_reference(r, oid);
> > +		buffer = repo_get_commit_buffer(r, this, &bufsize);
> > +		if (!buffer) {
> > +			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
> > +			goto out;
> > +		}
> > +
> > +		p = strstr(buffer, "\n\n");
> > +		if (!p) {
> > +			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
> > +			goto out;
> > +		}
> > +
> > +		p += 2;
> > +		msg = xmemdupz(p, bufsize - (p - buffer));
> 
> Here, we could check "git stash: " string to make sure that it is as
> expected in an exported stash we previously made, and abort, just
> like the above "cannot parse" case.

This part doesn't have the prefix.  That's the part above, but I'll add
the check there.  This part instead is the actual stash commit and we're
extracting the message to put into the reflog.

> Should we be making sure that the object named by "oid" does look
> like a stash, like what is done in builtin/stash.c:get_stash_info(),
> or is assert_stash_like() called from do_store_stash() sufficient?

I think `assert_stash_like` should be fine.  I don't have any better
checking to do than that, since we don't have any more checks to do.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

[Phillip Wood]

Hi brian

On 09/05/2025 00:44, brian m. carlson wrote:
> A common user problem is how to sync in-progress work to another
> machine.  Users currently must use some sort of transfer of the working
> tree, which poses security risks and also necessarily causes the index
> to become dirty.  The experience is suboptimal and frustrating for
> users.
> 
> A reasonable idea is to use the stash for this purpose, but the stash is
> stored in the reflog, not in a ref, and as such it cannot be pushed or
> pulled. 

I think the commit message would be more convincing if it concentrated
on the need to import / export chains of stashes and the convenience of
having a dedicated command to import a stash as one can export a single
stash with

     git push <remote> refs/stash@{<n>}:refs/exported-stash

and then import it with

     git pull <remote> refs/exported-stash
     git update-ref refs/stash FETCH_HEAD

Having said that I do agree that adding these new commands is a good
idea.

> @@ -154,6 +155,12 @@ store::
>   	reflog.  This is intended to be useful for scripts.  It is
>   	probably not the command you want to use; see "push" above.
>   
> +export ( --print | --to-ref <ref> ) [<stash>...]::

I was surprised to see that --to-ref does not take an argument. I think
that is confusing and it would be better to use OPT_STRING() rather than
OPT_CMDMODE() and manually check that only one of "--print" or
"--to-ref" is given. In the end it is no more work because now you have
to manually check that we have a ref when "--to-ref" is given.

I was also surprised to find that "git stash export HEAD" did not error
out. I was looking for a function that checked the topology of a stash
commit and could not find one so I thought I see how difficult it was to
add that. The fixup commit below adds a new function to check that a
commit looks like a stash and refuses to export anything else. The patch
adds a new function rather than using assert_stash_like() because
despite its name that function is pretty lax (any merge commit will
satisfy it) and is more concerned about filling out a stash_info struct.

Best Wishes

Phillip

---- >8 ----
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH] fixup! builtin/stash: provide a way to export stashes to a
  ref

Reject commits that do not look like stashes.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
  builtin/stash.c | 42 ++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 42 insertions(+)

diff --git a/builtin/stash.c b/builtin/stash.c
index d8332af4017..02cf344ed96 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -180,6 +180,33 @@ static void free_stash_info(struct stash_info *info)
  	strbuf_release(&info->revision);
  }

+static int check_stash_topology(struct repository *r, struct commit *stash)
+{
+	struct commit *p1, *p2, *p3 = NULL;
+
+	/* stash must have two or three parents */
+	if (!stash->parents || !stash->parents->next ||
+	    (stash->parents->next->next && stash->parents->next->next->next))
+		return -1;
+	p1 = stash->parents->item;
+	p2 = stash->parents->next->item;
+	if (stash->parents->next->next)
+		p3 = stash->parents->next->next->item;
+	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
+	    (p3 && repo_parse_commit(r, p3)))
+		return -1;
+	/* p2 must have a single parent, p3 must have no parents */
+	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
+		return -1;
+	if (repo_parse_commit(r, p2->parents->item))
+		return -1;
+	/* p2^1 must equal p1 */
+	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
+		return -1;
+
+	return 0;
+}
+
  static void assert_stash_like(struct stash_info *info, const char *revision)
  {
  	if (get_oidf(&info->b_commit, "%s^1", revision) ||
@@ -2101,13 +2128,21 @@ static int do_export_stash(struct repository *r,
  		 */
  		for (i = 0; i < argc; i++) {
  			struct object_id oid;
+			struct commit *stash;
+
  			if (parse_revision(&revision, argv[i], 1) ||
  			    get_oid_with_context(r, revision.buf,
  						 GET_OID_QUIETLY | GET_OID_GENTLY,
  						 &oid, &unused)) {
  				res = error(_("unable to find stash entry %s"), argv[i]);
  				goto out;
  			}
+			stash = lookup_commit_reference(r, &oid);
+			if (!stash || check_stash_topology(r, stash)) {
+				res = error(_("%s does not look like a stash commit"),
+					    argv[i]);
+				goto out;
+			}
  			oid_array_append(&items, &oid);
  		}
  	} else {
@@ -2118,13 +2153,20 @@ static int do_export_stash(struct repository *r,
  		for (i = 0;; i++) {
  			char buf[32];
  			struct object_id oid;
+			struct commit *stash;

  			snprintf(buf, sizeof(buf), "%d", i);
  			if (parse_revision(&revision, buf, 1) ||
  			    get_oid_with_context(r, revision.buf,
  						 GET_OID_QUIETLY | GET_OID_GENTLY,
  						 &oid, &unused))
  				break;
+			stash = lookup_commit_reference(r, &oid);
+			if (!stash || check_stash_topology(r, stash)) {
+				res = error(_("%s does not look like a stash commit"),
+					    revision.buf);
+				goto out;
+			}
  			oid_array_append(&items, &oid);
  		}
  	}
-- 
2.49.0.300.g813f75aecee

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Phillip Wood]

Hi brian

On 09/05/2025 00:44, brian m. carlson wrote:
> 
> @@ -1962,6 +1971,99 @@ static int write_commit_with_parents(struct repository *r,
>   	return ret;
>   }
>   
> +static int do_import_stash(struct repository *r, const char *rev)
> +{
> +	struct object_id chain;
> +	struct oid_array items = OID_ARRAY_INIT;
> +	int res = 0;
> +	int i;
> +	const char *buffer = NULL;
> +	struct commit *this = NULL;
> +	char *msg = NULL;
> +
> +	if (repo_get_oid(r, rev, &chain))
> +		return error(_("not a valid revision: %s"), rev);
> +
> +	/*
> +	 * Walk the commit history, finding each stash entry, and load data into
> +	 * the array.
> +	 */
> +	for (i = 0;; i++) {
> +		struct object_id tree, oid;
> +		char revision[GIT_MAX_HEXSZ + 1];
> +
> +		oid_to_hex_r(revision, &chain);
> +
> +		if (get_oidf(&tree, "%s:", revision) ||
> +		    !oideq(&tree, r->hash_algo->empty_tree)) {
> +			res = error(_("%s is not a valid exported stash commit"), revision);
> +			goto out;
> +		}
> +		if (get_oidf(&chain, "%s^1", revision) ||
> +		    get_oidf(&oid, "%s^2", revision))
> +			break;
> +		oid_array_append(&items, &oid);
> +	}

This loop could use some improvement - it does not use the loop
variable, it accepts any commit with an empty tree as a valid exported
stash, it is pretty lax about checking that the commits in the chain
have either zero or two parents, it does not check if the second parent
looks like a stash and it is forever converting between strings and
object_ids. I think it would be better to loop over commits rather than
object ids then you can walk the history using the pointers to the
parent commits. Something like

	if (repo_get_oid(r, rev, &chain))
		return error(_("not a valid revision: %s"), rev);

	this = lookup_commit_reference(r, &chain);
	if (!this)
		return error(_("not a commit: %s"), rev);
	/*
	 * Walk the commit history, finding each stash entry, and load data into
	 * the array.
	 */
	for (;;) {
		struct commit *stash;
		struct tree *tree = repo_get_commit_tree(r, this);

		if (!tree ||
		    !oideq(&tree->object.oid, r->hash_algo->empty_tree) ||
		    (this->parents &&
		     (!this->parents->next || this->parents->next->next))) {
			res = error(_("%s is not a valid exported stash commit"),
				    oid_to_hex(&this->object.oid));
			goto out;
		}
		if (!this->parents)
			break;
		stash = this->parents->next->item;
		if (repo_parse_commit(r, this->parents->item) ||
		    repo_parse_commit(r, stash)) {
			res = error(_("cannot parse parents of commit: %s"),
				     oid_to_hex(&this->object.oid));
			goto out;
		}
		if (check_stash_topology(r, stash)) {
			res = error(_("%s does not look like a stash commit"),
				    oid_to_hex(&stash->object.oid));
			goto out;
		}
		/* TODO:
		 *  - store commits, not objects
		 */
		oid_array_append(&items, &this->parents->next->item->object.oid);
		this = this->parents->item;
	}

I've appended a fixup commit below that applies on top of your
patch. The fixups for this patch and the previous one can be fetched
with

   git fetch https://github.com/phillipwood/git.git bc/stash-import-export-fixups

if you want to use them.

Best Wishes

Phillip

---- >8 ----
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH] fixup! builtin/stash: provide a way to import stashes from a
  ref

Strengthen the checks on imported commits to ensure that the chain of
imported stashes consists of commits with exactly two parents where the
first parent is either the root commit or another imported stash commit
and the second parent looks like a stash commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
  builtin/stash.c | 43 +++++++++++++++++++++++++++++++------------
  1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 02cf344ed9..7d3a8c03a0 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -2011,25 +2011,44 @@ static int do_import_stash(struct repository *r, const char *rev)
  	if (repo_get_oid(r, rev, &chain))
  		return error(_("not a valid revision: %s"), rev);
  
+	this = lookup_commit_reference(r, &chain);
+	if (!this)
+		return error(_("not a commit: %s"), rev);
  	/*
  	 * Walk the commit history, finding each stash entry, and load data into
  	 * the array.
  	 */
-	for (i = 0;; i++) {
-		struct object_id tree, oid;
-		char revision[GIT_MAX_HEXSZ + 1];
-
-		oid_to_hex_r(revision, &chain);
-
-		if (get_oidf(&tree, "%s:", revision) ||
-		    !oideq(&tree, r->hash_algo->empty_tree)) {
-			res = error(_("%s is not a valid exported stash commit"), revision);
+	for (;;) {
+		struct commit *stash;
+		struct tree *tree = repo_get_commit_tree(r, this);
+
+		if (!tree ||
+		    !oideq(&tree->object.oid, r->hash_algo->empty_tree) ||
+		    (this->parents &&
+		     (!this->parents->next || this->parents->next->next))) {
+			res = error(_("%s is not a valid exported stash commit"),
+				    oid_to_hex(&this->object.oid));
  			goto out;
  		}
-		if (get_oidf(&chain, "%s^1", revision) ||
-		    get_oidf(&oid, "%s^2", revision))
+		if (!this->parents)
  			break;
-		oid_array_append(&items, &oid);
+		stash = this->parents->next->item;
+		if (repo_parse_commit(r, this->parents->item) ||
+		    repo_parse_commit(r, stash)) {
+			res = error(_("cannot parse parents of commit: %s"),
+				     oid_to_hex(&this->object.oid));
+			goto out;
+		}
+		if (check_stash_topology(r, stash)) {
+			res = error(_("%s does not look like a stash commit"),
+				    oid_to_hex(&stash->object.oid));
+			goto out;
+		}
+		/* TODO:
+		 *  - store commits, not objects
+		 */
+		oid_array_append(&items, &this->parents->next->item->object.oid);
+		this = this->parents->item;
  	}
  
  	/*
-- 
2.49.0.300.g813f75aecee

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, May 08, 2025 at 11:44:57PM +0000, brian m. carlson wrote:
>
>> +test_expect_success 'stash export and import round-trip stashes' '
>> [...]
>> +	git stash export --to-ref refs/heads/foo &&
>
> Here we export to a name in the refs/heads/ namespace...
>
>> +test_expect_success 'stash export can accept specified stashes' '
>> [...]
>> +	git stash export --to-ref bar stash@{1} stash@{0} &&
>
> ...but here we are writing to the top-level .git/bar. We do currently
> allow that, but there's been discussion of locking this down a bit
> further (requiring BAR or even BAR_HEAD at the top-level). Should this
> be refs/heads/bar?
>
>> +test_expect_success 'stash can import and export zero stashes' '
>> [...]
>> +	git stash export --to-ref baz &&
>
> Ditto here.
>
> I noticed because I have a patch series from last summer tightening
> these rules (it got derailed by some conflicting work, and I've been
> meaning to pick it back up). I can certainly adjust these tests as part
> of that series, but if you're re-rolling anyway, it might be nice to do
> it now.

True, and exported one is a sort-of-normal-looking isolated history,
so it does not have any strong reason to be at the top level.  But I
am curious what your plans are to deal with .git/refs/stash itself?

Thanks.

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Jeff King]

On Mon, May 12, 2025 at 05:42:08AM -0700, Junio C Hamano wrote:

> > I noticed because I have a patch series from last summer tightening
> > these rules (it got derailed by some conflicting work, and I've been
> > meaning to pick it back up). I can certainly adjust these tests as part
> > of that series, but if you're re-rolling anyway, it might be nice to do
> > it now.
> 
> True, and exported one is a sort-of-normal-looking isolated history,
> so it does not have any strong reason to be at the top level.  But I
> am curious what your plans are to deal with .git/refs/stash itself?

My series is only about the absolute top-level, outside of refs/. So
"refs/stash" is OK, but "stash" is not.

-Peff

Re: [PATCH v5 3/4] builtin/stash: provide a way to export stashes to a ref

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I think the commit message would be more convincing if it concentrated
> on the need to import / export chains of stashes and the convenience of
> having a dedicated command to import a stash as one can export a single
> stash with
>
>     git push <remote> refs/stash@{<n>}:refs/exported-stash
>
> and then import it with
>
>     git pull <remote> refs/exported-stash
>     git update-ref refs/stash FETCH_HEAD

Oooh, then as long as the other side has enabled reflog, you could
even do

	for n in $(seq 0 N)
	do
		git push <remote> +refs/stash@{$n}:refs/stash
	done

and we can call it done?  ;-)

> Having said that I do agree that adding these new commands is a good
> idea.

Yeah, I do not think there is no reason to oppose having an export/import
pair.

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, May 12, 2025 at 05:42:08AM -0700, Junio C Hamano wrote:
>
>> > I noticed because I have a patch series from last summer tightening
>> > these rules (it got derailed by some conflicting work, and I've been
>> > meaning to pick it back up). I can certainly adjust these tests as part
>> > of that series, but if you're re-rolling anyway, it might be nice to do
>> > it now.
>> 
>> True, and exported one is a sort-of-normal-looking isolated history,
>> so it does not have any strong reason to be at the top level.  But I
>> am curious what your plans are to deal with .git/refs/stash itself?
>
> My series is only about the absolute top-level, outside of refs/. So
> "refs/stash" is OK, but "stash" is not.

Ah, OK.  It would be good to forbid things outside refs/, and
tightly control the top level, so $GIT_DIR/config and
$GIT_DIR/objects/[0-9a-f]{2}/[0-9a-f]{38} won't be clobbered.

refs/stash is probably fine.  Creating refs/head to nuke all local
branches might also be something we want to protect against, though.

Thanks.

Re: [PATCH v5 4/4] builtin/stash: provide a way to import stashes from a ref

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On 2025-05-10 at 17:21:07, Jeff King wrote:
> I noticed because I have a patch series from last summer tightening
> these rules (it got derailed by some conflicting work, and I've been
> meaning to pick it back up). I can certainly adjust these tests as part
> of that series, but if you're re-rolling anyway, it might be nice to do
> it now.

I've fixed these in my branch and will include them in v6.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

[PATCH] Makefile: avoid constant rebuilds with compilation database

[brian m. carlson]

Many contributors to software use a Language Server Protocol
implementation to allow their editor to learn structural information
about the code they write and provide additional features, such as
jumping to the declaration or definition of a function or type.  In C,
the usual implementation is clangd, which requires compiling with clang.

Because C and C++ projects lack a standard file system layout and build
system, unlike languages such as Rust and Go, clangd requires a
compilation database to be generated by the clang compiler in order to
pass the proper compilation flags and discover all of the files
necessary to make the LSP work.  This is done by setting
GENERATE_COMPILATION_DATABASE to "yes".

However, when that's enabled and the user runs "make" a second time,
all of the files are re-compiled, which is inconvenient for contributors
to Git, since it makes small changes or rebases recompile the entirety
of the codebase.  This happens because the directory holding the
compilation database is updated anytime an object is built, so its
modification date will always be newer than the first object built.

To solve this, use the same trick we do just above for the .depend
directory and filter the compilation database directory out if it
already exists, which avoids making it a target to be built.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Helped-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8a7f1c7654..8f38de8ebf 100644
--- a/Makefile
+++ b/Makefile
@@ -2804,7 +2804,7 @@ endif
 compdb_dir = compile_commands
 
 ifeq ($(GENERATE_COMPILATION_DATABASE),yes)
-missing_compdb_dir = $(compdb_dir)
+missing_compdb_dir = $(filter-out $(wildcard $(compdb_dir)), $(compdb_dir))
 $(missing_compdb_dir):
 	@mkdir -p $@
 

[PATCH v6 0/5] Importing and exporting stashes to refs

[brian m. carlson]

Stashes are currently stored using the reflog in a given repository.
This is an interesting and novel way to handle them, but there is no way
to easily move a set of stashes across machines.  For example, groups of
stashes cannot be bundled, pushed, or fetched.

Let's solve this problem by allowing users to import and export stashes
to a chain of commits.  The commits used in a stash export contain two
parents: one which is the pointer to the next exported stash (or to an
empty commit with no parents if there are no more) and the second is the
stash commit that would normally be stored in the reflog.

I will need Phillip's sign-off for the last patch, since his fixup patch
didn't include it.

Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>

Changes from v5:
* Rename `parse_revision`.
* Remove extra call to `free_stash_info`.
* Fix parsing of existing commit.
* Add more validation of imported stash commits.
* Add more tests for improved validation of imported stash commits.
* Explicitly cast `items.nr` and make the iteration counter an `ssize_t`
  to avoid casting problems.
* Don't require a trailing `\n\n` in commit messages.
* Use `read_complete_reflog` to walk reflogs.
* Be more defensive when using `lookup_commit_reference`.
* Apply parts of Phillip's patches for improved robustness.
* Update commit message to explain additional use cases.
* Use `OPT_STRING` for `--to-ref`.

Changes from v4:
* Fix another use of oid_array.
* Fix various memory leaks.
* Fix a segfault which appeared after a rebase.
* Use strstr for commits since we don't need to worry about NUL.
* Added some additional tests.
* Verify the ident values we're using to avoid using bad values.
* Various other code cleanups.
* Rebase on `master`.

Changes from v3:
* Fix strbuf handling to avoid leaks and generally be more sensible.
* Make use of the error return code more often.
* Use oid_array.
* Tidy various parts of the code and fix long lines.
* Simplify tests using git tag.
* Shorten and tidy tests.
* Add an additional test covering the base commit OID and importing and
  exporting empty stashes.

Changes from v2:
* Fix uninitialized strbuf.
* Avoid C99-style initializations.

Changes from v1:
* Change storage format as suggested by Junio.
* Rename to GIT_OID_GENTLY.
* Remove unnecessary initializations.
* Use ALLOC_GROW_BY.
* Ensure completely reproducible exports.
* Avoid size_t.
* Various other code cleanups.

brian m. carlson (5):
  object-name: make get_oid quietly return an error
  reflog-walk: expose read_complete_reflog
  builtin/stash: factor out revision parsing into a function
  builtin/stash: provide a way to export stashes to a ref
  builtin/stash: provide a way to import stashes from a ref

 Documentation/git-stash.adoc |  29 ++-
 builtin/stash.c              | 450 ++++++++++++++++++++++++++++++++++-
 hash.h                       |   1 +
 object-name.c                |   6 +-
 reflog-walk.c                |  17 +-
 reflog-walk.h                |  18 ++
 t/t3903-stash.sh             |  94 ++++++++
 7 files changed, 587 insertions(+), 28 deletions(-)

[PATCH v6 1/5] object-name: make get_oid quietly return an error

[brian m. carlson]

A reasonable person looking at the signature and usage of get_oid and
friends might conclude that in the event of an error, it always returns
-1.  However, this is not the case.  Instead, get_oid_basic dies if we
go too far back into the history of a reflog (or, when quiet, simply
exits).

This is not especially useful, since in many cases, we might want to
handle this error differently.  Let's add a flag here to make it just
return -1 like elsewhere in these code paths.

Note that we cannot make this behavior the default, since we have many
other codepaths that rely on the existing behavior, including in tests.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 hash.h        | 1 +
 object-name.c | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hash.h b/hash.h
index d6422ddf45..ec594c63a6 100644
--- a/hash.h
+++ b/hash.h
@@ -216,6 +216,7 @@ struct object_id {
 #define GET_OID_REQUIRE_PATH         010000
 #define GET_OID_HASH_ANY             020000
 #define GET_OID_SKIP_AMBIGUITY_CHECK 040000
+#define GET_OID_GENTLY              0100000
 
 #define GET_OID_DISAMBIGUATORS \
 	(GET_OID_COMMIT | GET_OID_COMMITTISH | \
diff --git a/object-name.c b/object-name.c
index 9288b2dd24..851858975f 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 				 * still fill in the oid with the "old" value,
 				 * which we can use.
 				 */
-			} else {
+			} else if (!(flags & GET_OID_GENTLY)) {
 				if (flags & GET_OID_QUIETLY) {
 					exit(128);
 				}
 				die(_("log for '%.*s' only has %d entries"),
 				    len, str, co_cnt);
 			}
+			if (flags & GET_OID_GENTLY) {
+				free(real_ref);
+				return -1;
+			}
 		}
 	}
 

[PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[brian m. carlson]

In a future commit, we'll use this function and the corresponding free
function to read the entire reflog.  Expose it in the header so we can
do so.

Include the appropriate header files so that our header is complete.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 reflog-walk.c | 17 ++---------------
 reflog-walk.h | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index c7070b13b0..b7a9d70966 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -9,19 +9,6 @@
 #include "string-list.h"
 #include "reflog-walk.h"
 
-struct complete_reflogs {
-	char *ref;
-	char *short_ref;
-	struct reflog_info {
-		struct object_id ooid, noid;
-		char *email;
-		timestamp_t timestamp;
-		int tz;
-		char *message;
-	} *items;
-	int nr, alloc;
-};
-
 static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
 		const char *email, timestamp_t timestamp, int tz,
 		const char *message, void *cb_data)
@@ -41,7 +28,7 @@ static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static void free_complete_reflog(struct complete_reflogs *array)
+void free_complete_reflog(struct complete_reflogs *array)
 {
 	int i;
 
@@ -64,7 +51,7 @@ static void complete_reflogs_clear(void *util, const char *str UNUSED)
 	free_complete_reflog(array);
 }
 
-static struct complete_reflogs *read_complete_reflog(const char *ref)
+struct complete_reflogs *read_complete_reflog(const char *ref)
 {
 	struct complete_reflogs *reflogs =
 		xcalloc(1, sizeof(struct complete_reflogs));
diff --git a/reflog-walk.h b/reflog-walk.h
index 989583dc55..8f0640f662 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -1,9 +1,24 @@
 #ifndef REFLOG_WALK_H
 #define REFLOG_WALK_H
 
+#include "git-compat-util.h"
+#include "hash.h"
+
 struct commit;
 struct reflog_walk_info;
 struct date_mode;
+struct complete_reflogs {
+	char *ref;
+	char *short_ref;
+	struct reflog_info {
+		struct object_id ooid, noid;
+		char *email;
+		timestamp_t timestamp;
+		int tz;
+		char *message;
+	} *items;
+	int nr, alloc;
+};
 
 void init_reflog_walk(struct reflog_walk_info **info);
 void reflog_walk_info_release(struct reflog_walk_info *info);
@@ -24,4 +39,7 @@ int reflog_walk_empty(struct reflog_walk_info *walk);
 
 struct commit *next_reflog_entry(struct reflog_walk_info *reflog_info);
 
+void free_complete_reflog(struct complete_reflogs *array);
+struct complete_reflogs *read_complete_reflog(const char *ref);
+
 #endif

[PATCH v6 3/5] builtin/stash: factor out revision parsing into a function

[brian m. carlson]

We allow several special forms of stash names in this code.  In the
future, we'll want to allow these same forms without parsing a stash
commit, so let's refactor this code out into a function for reuse.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/stash.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index cfbd92852a..ab491d5ff6 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -169,6 +169,25 @@ static void assert_stash_like(struct stash_info *info, const char *revision)
 		die(_("'%s' is not a stash-like commit"), revision);
 }
 
+static int parse_stash_revision(struct strbuf *revision, const char *commit, int quiet)
+{
+	strbuf_reset(revision);
+	if (!commit) {
+		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
+			if (!quiet)
+				fprintf_ln(stderr, _("No stash entries found."));
+			return -1;
+		}
+
+		strbuf_addf(revision, "%s@{0}", ref_stash);
+	} else if (strspn(commit, "0123456789") == strlen(commit)) {
+		strbuf_addf(revision, "%s@{%s}", ref_stash, commit);
+	} else {
+		strbuf_addstr(revision, commit);
+	}
+	return 0;
+}
+
 static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 {
 	int ret;
@@ -196,17 +215,9 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 	if (argc == 1)
 		commit = argv[0];
 
-	if (!commit) {
-		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
-			fprintf_ln(stderr, _("No stash entries found."));
-			return -1;
-		}
-
-		strbuf_addf(&info->revision, "%s@{0}", ref_stash);
-	} else if (strspn(commit, "0123456789") == strlen(commit)) {
-		strbuf_addf(&info->revision, "%s@{%s}", ref_stash, commit);
-	} else {
-		strbuf_addstr(&info->revision, commit);
+	strbuf_init(&info->revision, 0);
+	if (parse_stash_revision(&info->revision, commit, 0)) {
+		return -1;
 	}
 
 	revision = info->revision.buf;

[PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref

[brian m. carlson]

A common user problem is how to sync in-progress work to another
machine.  Users currently must use some sort of transfer of the working
tree, which poses security risks and also necessarily causes the index
to become dirty.  The experience is suboptimal and frustrating for
users.

A reasonable idea is to use the stash for this purpose, but the stash is
stored in the reflog, not in a ref, and as such it cannot be pushed or
pulled.  This also means that it cannot be saved into a bundle or
preserved elsewhere, which is a problem when using throwaway development
environments.

In addition, users often want to replicate stashes across machines, such
as when they must use multiple machines or when they use throwaway dev
environments, such as those based on the Devcontainer spec, where they
might otherwise lose various in-progress work.

Let's solve this problem by allowing the user to export the stash to a
ref (or, to just write it into the repository and print the hash, à la
git commit-tree).  Introduce git stash export, which writes a chain of
commits where the first parent is always a chain to the previous stash,
or to a single, empty commit (for the final item) and the second is the
stash commit normally written to the reflog.

Iterate over each stash from topmost to bottomost, looking up the data
for each one, and then create the chain from the single empty commit
back up in reverse order.  Generate a predictable empty commit so our
behavior is reproducible.  Create a useful commit message, preserving
the author and committer information, to help users identify stash
commits when viewing them as normal commits.

If the user has specified specific stashes they'd like to export
instead, use those instead of iterating over all of the stashes.

As part of this, specifically request quiet behavior when looking up the
OID for a revision because we will eventually hit a revision that
doesn't exist and we don't want to die when that occurs.

When exporting stashes, be sure to verify that they look like valid
stashes and don't contain invalid data.  This will help avoid failures
on import or problems due to attempting to export invalid refs that are
not stashes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 Documentation/git-stash.adoc |  22 ++-
 builtin/stash.c              | 250 +++++++++++++++++++++++++++++++++++
 2 files changed, 271 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 1a5177f498..e8efd43ba4 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -23,6 +23,7 @@ SYNOPSIS
 'git stash' clear
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
+'git stash' export (--print | --to-ref <ref>) [<stash>...]
 
 DESCRIPTION
 -----------
@@ -154,6 +155,12 @@ store::
 	reflog.  This is intended to be useful for scripts.  It is
 	probably not the command you want to use; see "push" above.
 
+export ( --print | --to-ref <ref> ) [<stash>...]::
+
+	Export the specified stashes, or all of them if none are specified, to
+	a chain of commits which can be transferred using the normal fetch and
+	push mechanisms, then imported using the `import` subcommand.
+
 OPTIONS
 -------
 -a::
@@ -242,6 +249,19 @@ literally (including newlines and quotes).
 +
 Quiet, suppress feedback messages.
 
+--print::
+	This option is only valid for `export`.
++
+Create the chain of commits representing the exported stashes without
+storing it anywhere in the ref namespace and print the object ID to
+standard output.  This is designed for scripts.
+
+--to-ref::
+	This option is only valid for `export`.
++
+Create the chain of commits representing the exported stashes and store
+it to the specified ref.
+
 \--::
 	This option is only valid for `push` command.
 +
@@ -259,7 +279,7 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 <stash>::
 	This option is only valid for `apply`, `branch`, `drop`, `pop`,
-	`show` commands.
+	`show`, and `export` commands.
 +
 A reference of the form `stash@{<revision>}`. When no `<stash>` is
 given, the latest stash is assumed (that is, `stash@{0}`).
diff --git a/builtin/stash.c b/builtin/stash.c
index ab491d5ff6..ca00663e36 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -28,7 +28,10 @@
 #include "log-tree.h"
 #include "diffcore.h"
 #include "reflog.h"
+#include "reflog-walk.h"
 #include "add-interactive.h"
+#include "oid-array.h"
+#include "commit.h"
 
 #define INCLUDE_ALL_FILES 2
 
@@ -56,6 +59,8 @@
 	   "          [-u | --include-untracked] [-a | --all] [<message>]")
 #define BUILTIN_STASH_CREATE_USAGE \
 	N_("git stash create [<message>]")
+#define BUILTIN_STASH_EXPORT_USAGE \
+	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -71,6 +76,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CLEAR_USAGE,
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
+	BUILTIN_STASH_EXPORT_USAGE,
 	NULL
 };
 
@@ -124,6 +130,12 @@ static const char * const git_stash_save_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_export_usage[] = {
+	BUILTIN_STASH_EXPORT_USAGE,
+	NULL
+};
+
+
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
 
@@ -160,6 +172,33 @@ static void free_stash_info(struct stash_info *info)
 	strbuf_release(&info->revision);
 }
 
+static int check_stash_topology(struct repository *r, struct commit *stash)
+{
+	struct commit *p1, *p2, *p3 = NULL;
+
+	/* stash must have two or three parents */
+	if (!stash->parents || !stash->parents->next ||
+			(stash->parents->next->next && stash->parents->next->next->next))
+		return -1;
+	p1 = stash->parents->item;
+	p2 = stash->parents->next->item;
+	if (stash->parents->next->next)
+		p3 = stash->parents->next->next->item;
+	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
+			(p3 && repo_parse_commit(r, p3)))
+		return -1;
+	/* p2 must have a single parent, p3 must have no parents */
+	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
+		return -1;
+	if (repo_parse_commit(r, p2->parents->item))
+		return -1;
+	/* p2^1 must equal p1 */
+	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
+		return -1;
+
+	return 0;
+}
+
 static void assert_stash_like(struct stash_info *info, const char *revision)
 {
 	if (get_oidf(&info->b_commit, "%s^1", revision) ||
@@ -1895,6 +1934,216 @@ static int save_stash(int argc, const char **argv, const char *prefix,
 	return ret;
 }
 
+static int write_commit_with_parents(struct repository *r,
+				     struct object_id *out,
+				     const struct object_id *oid,
+				     struct commit_list *parents)
+{
+	size_t author_len, committer_len;
+	struct commit *this;
+	const char *orig_author, *orig_committer;
+	char *author = NULL, *committer = NULL;
+	const char *buffer;
+	unsigned long bufsize;
+	const char *p;
+	struct strbuf msg = STRBUF_INIT;
+	int ret = 0;
+	struct ident_split id;
+
+	this = lookup_commit_reference(r, oid);
+	buffer = repo_get_commit_buffer(r, this, &bufsize);
+	orig_author = find_commit_header(buffer, "author", &author_len);
+	orig_committer = find_commit_header(buffer, "committer", &committer_len);
+
+	if (!orig_author || !orig_committer) {
+		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	if (split_ident_line(&id, orig_author, author_len) < 0 ||
+	    split_ident_line(&id, orig_committer, committer_len) < 0) {
+		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	p = strstr(buffer, "\n\n");
+	strbuf_addstr(&msg, "git stash: ");
+
+	if (p)
+		strbuf_add(&msg, p + 2, bufsize - (p + 2 - buffer));
+	strbuf_complete_line(&msg);
+
+	author = xmemdupz(orig_author, author_len);
+	committer = xmemdupz(orig_committer, committer_len);
+
+	if (commit_tree_extended(msg.buf, msg.len,
+				 r->hash_algo->empty_tree, parents,
+				 out, author, committer,
+				 NULL, NULL)) {
+		ret = error(_("could not write commit"));
+		goto out;
+	}
+out:
+	strbuf_release(&msg);
+	repo_unuse_commit_buffer(r, this, buffer);
+	free_commit_list(parents);
+	free(author);
+	free(committer);
+	return ret;
+}
+
+static int do_export_stash(struct repository *r,
+			   const char *ref,
+			   int argc,
+			   const char **argv)
+{
+	struct object_id base;
+	struct object_context unused;
+	struct commit *prev;
+	struct oid_array items = OID_ARRAY_INIT;
+	int res = 0;
+	ssize_t i;
+	struct strbuf revision = STRBUF_INIT;
+	const char *author, *committer;
+
+	/*
+	 * This is an arbitrary, fixed date, specifically the one used by git
+	 * format-patch.  The goal is merely to produce reproducible output.
+	 */
+	prepare_fallback_ident("git stash", "git@stash");
+	author = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			   "2001-09-17T00:00:00Z", 0);
+	committer = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			      "2001-09-17T00:00:00Z", 0);
+
+	/* First, we create a single empty commit. */
+	if (commit_tree_extended("", 0, r->hash_algo->empty_tree, NULL,
+				 &base, author, committer, NULL, NULL))
+		return error(_("unable to write base commit"));
+
+	prev = lookup_commit_reference(r, &base);
+
+	if (argc) {
+		/*
+		 * Find each specified stash, and load data into the array.
+		 */
+		for (i = 0; i < argc; i++) {
+			struct object_id oid;
+			struct commit *stash;
+
+			if (parse_stash_revision(&revision, argv[i], 1) ||
+			    get_oid_with_context(r, revision.buf,
+						 GET_OID_QUIETLY | GET_OID_GENTLY,
+						 &oid, &unused)) {
+				res = error(_("unable to find stash entry %s"), argv[i]);
+				goto out;
+			}
+
+			stash = lookup_commit_reference(r, &oid);
+			if (!stash || check_stash_topology(r, stash)) {
+				res = error(_("%s does not look like a stash commit"),
+						revision.buf);
+				goto out;
+			}
+			oid_array_append(&items, &oid);
+		}
+	} else {
+		/*
+		 * Walk the reflog, finding each stash entry, and load data into the
+		 * array.
+		 */
+		struct complete_reflogs *crl = read_complete_reflog("refs/stash");
+		for (i = 0; i < crl->nr; i++) {
+			/*
+			 * Iterate from the latest item in the reflog to the
+			 * earliest.  crl->items starts from the earliest first.
+			 */
+			struct reflog_info *item = crl->items + (crl->nr - i - 1);
+			struct commit *stash;
+
+			stash = lookup_commit_reference(r, &item->noid);
+			if (!stash || check_stash_topology(r, stash)) {
+				res = error(_("%s does not look like a stash commit"),
+						revision.buf);
+				goto out;
+			}
+			oid_array_append(&items, &item->noid);
+		}
+		free_complete_reflog(crl);
+	}
+
+	/*
+	 * Now, create a set of commits identical to the regular stash commits,
+	 * but where their first parents form a chain to our original empty
+	 * base commit.
+	 */
+	for (i = (ssize_t)items.nr - 1; i >= 0; i--) {
+		struct commit_list *parents = NULL;
+		struct commit_list **next = &parents;
+		struct object_id out;
+		const struct object_id *oid = items.oid + i;
+		struct commit *stash;
+
+		next = commit_list_append(prev, next);
+		stash = lookup_commit_reference(r, oid);
+		if (!stash) {
+			res = error(_("%s is not a valid commit"),
+					revision.buf);
+			goto out;
+		}
+		next = commit_list_append(stash, next);
+		res = write_commit_with_parents(r, &out, oid, parents);
+		if (res)
+			goto out;
+		prev = lookup_commit_reference(r, &out);
+	}
+	if (ref)
+		refs_update_ref(get_main_ref_store(r), NULL, ref,
+				&prev->object.oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	else
+		puts(oid_to_hex(&prev->object.oid));
+out:
+	strbuf_release(&revision);
+	oid_array_clear(&items);
+
+	return res;
+}
+
+enum export_action {
+	ACTION_NONE,
+	ACTION_PRINT,
+	ACTION_TO_REF,
+};
+
+static int export_stash(int argc,
+			const char **argv,
+			const char *prefix,
+			struct repository *repo)
+{
+	const char *ref = NULL;
+	enum export_action action = ACTION_NONE;
+	struct option options[] = {
+		OPT_CMDMODE(0, "print", &action,
+			    N_("print the object ID instead of writing it to a ref"),
+			    ACTION_PRINT),
+		OPT_STRING(0, "to-ref", &ref, "ref",
+			    N_("save the data to the given ref")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_export_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (ref && action == ACTION_NONE)
+		action = ACTION_TO_REF;
+
+	if (action == ACTION_NONE)
+		return error(_("exactly one of --print and --to-ref is required"));
+
+	return do_export_stash(repo, ref, argc, argv);
+}
+
 int cmd_stash(int argc,
 	      const char **argv,
 	      const char *prefix,
@@ -1915,6 +2164,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("store", &fn, store_stash),
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
+		OPT_SUBCOMMAND("export", &fn, export_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};

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

[brian m. carlson]

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.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
I'll need Phillip's sign-off for this patch.

 Documentation/git-stash.adoc |   7 ++
 builtin/stash.c              | 167 +++++++++++++++++++++++++++++++++++
 t/t3903-stash.sh             |  94 ++++++++++++++++++++
 3 files changed, 268 insertions(+)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index e8efd43ba4..f8193b712e 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -24,6 +24,7 @@ SYNOPSIS
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
 'git stash' export (--print | --to-ref <ref>) [<stash>...]
+'git stash' import <commit>
 
 DESCRIPTION
 -----------
@@ -161,6 +162,12 @@ export ( --print | --to-ref <ref> ) [<stash>...]::
 	a chain of commits which can be transferred using the normal fetch and
 	push mechanisms, then imported using the `import` subcommand.
 
+import <commit>::
+
+	Import the specified stashes from the specified commit, which must have been
+	created by `export`, and add them to the list of stashes.  To replace the
+	existing stashes, use `clear` first.
+
 OPTIONS
 -------
 -a::
diff --git a/builtin/stash.c b/builtin/stash.c
index ca00663e36..59fcb461eb 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -61,6 +61,8 @@
 	N_("git stash create [<message>]")
 #define BUILTIN_STASH_EXPORT_USAGE \
 	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
+#define BUILTIN_STASH_IMPORT_USAGE \
+	N_("git stash import <commit>")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -77,6 +79,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
 	BUILTIN_STASH_EXPORT_USAGE,
+	BUILTIN_STASH_IMPORT_USAGE,
 	NULL
 };
 
@@ -135,6 +138,10 @@ static const char * const git_stash_export_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_import_usage[] = {
+	BUILTIN_STASH_IMPORT_USAGE,
+	NULL
+};
 
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
@@ -144,6 +151,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
@@ -154,6 +162,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;
@@ -1992,6 +2001,163 @@ static int write_commit_with_parents(struct repository *r,
 	return ret;
 }
 
+static int do_import_stash(struct repository *r, const char *rev)
+{
+	struct object_id chain;
+	struct oid_array items = OID_ARRAY_INIT;
+	int res = 0;
+	ssize_t i;
+	const char *buffer = NULL;
+	unsigned long bufsize;
+	struct commit *this = NULL;
+	char *msg = NULL;
+
+	if (repo_get_oid(r, rev, &chain))
+		return error(_("not a valid revision: %s"), rev);
+
+	this = lookup_commit_reference(r, &chain);
+	if (!this)
+		return error(_("not a commit: %s"), rev);
+
+	/*
+	 * Walk the commit history, finding each stash entry, and load data into
+	 * the array.
+	 */
+	for (;;) {
+		const char *author, *committer;
+		size_t author_len, committer_len;
+		const char *p;
+		const char *expected = "git stash <git@stash> 1000684800 +0000";
+		const char *prefix = "git stash: ";
+		struct commit *stash;
+		struct tree *tree = repo_get_commit_tree(r, this);
+
+		if (!tree ||
+		    !oideq(&tree->object.oid, r->hash_algo->empty_tree) ||
+		    (this->parents &&
+		     (!this->parents->next || this->parents->next->next))) {
+			res = error(_("%s is not a valid exported stash commit"),
+					oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+
+		if (!this->parents) {
+			/*
+			 * We don't have any parents.  Make sure this is our
+			 * root commit.
+			 */
+			author = find_commit_header(buffer, "author", &author_len);
+			committer = find_commit_header(buffer, "committer", &committer_len);
+
+			if (!author || !committer) {
+				error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
+				goto out;
+			}
+
+			if (author_len != strlen(expected) ||
+			    committer_len != strlen(expected) ||
+			    memcmp(author, expected, author_len) ||
+			    memcmp(committer, expected, committer_len)) {
+				res = error(_("found root commit %s with invalid data"), oid_to_hex(&this->object.oid));
+				goto out;
+			}
+			break;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		p += 2;
+		if (((size_t)(bufsize - (p - buffer)) < strlen(prefix)) ||
+		    memcmp(prefix, p, strlen(prefix))) {
+			res = error(_("found stash commit %s with unexpected prefix"), oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		stash = this->parents->next->item;
+
+		if (repo_parse_commit(r, this->parents->item) ||
+		    repo_parse_commit(r, stash)) {
+			res = error(_("cannot parse parents of commit: %s"),
+					oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		if (check_stash_topology(r, stash)) {
+			res = error(_("%s does not look like a stash commit"),
+					oid_to_hex(&stash->object.oid));
+			goto out;
+		}
+
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+		oid_array_append(&items, &stash->object.oid);
+		this = this->parents->item;
+	}
+
+	/*
+	 * Now, walk each entry, adding it to the stash as a normal stash
+	 * commit.
+	 */
+	for (i = (ssize_t)items.nr - 1; i >= 0; i--) {
+		const char *p;
+		const struct object_id *oid = items.oid + i;
+
+		this = lookup_commit_reference(r, oid);
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+		if (!buffer) {
+			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p += 2;
+		msg = xmemdupz(p, bufsize - (p - buffer));
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+
+		if (do_store_stash(oid, msg, 1)) {
+			res = error(_("cannot save the stash for %s"), oid_to_hex(oid));
+			goto out;
+		}
+		FREE_AND_NULL(msg);
+	}
+out:
+	if (this && buffer)
+		repo_unuse_commit_buffer(r, this, buffer);
+	oid_array_clear(&items);
+	free(msg);
+
+	return res;
+}
+
+static int import_stash(int argc, const char **argv, const char *prefix,
+			struct repository *repo)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_import_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc != 1)
+		usage_msg_opt("a revision is required", git_stash_import_usage, options);
+
+	return do_import_stash(repo, argv[0]);
+}
+
 static int do_export_stash(struct repository *r,
 			   const char *ref,
 			   int argc,
@@ -2165,6 +2331,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
 		OPT_SUBCOMMAND("export", &fn, export_stash),
+		OPT_SUBCOMMAND("import", &fn, import_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 74666ff3e4..c827709468 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -11,6 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-unique-files.sh
 
+test_expect_success 'setup' '
+	test_oid_cache <<-EOF
+	export_base sha1:73c9bab443d1f88ac61aa533d2eeaaa15451239c
+	export_base sha256:f210fa6346e3e2ce047bdb570426b17075980c1ac01fec8fc4b75bd3ab4bcfe4
+	EOF
+'
+
 test_expect_success 'usage on cmd and subcommand invalid option' '
 	test_expect_code 129 git stash --invalid-option 2>usage &&
 	grep "or: git stash" usage &&
@@ -1412,6 +1419,93 @@ test_expect_success 'stash --keep-index --include-untracked with empty tree' '
 	)
 '
 
+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 --include-untracked -- subdir/ &&
+	git tag t-stash0 stash@{0} &&
+	git tag t-stash1 stash@{1} &&
+	simple=$(git stash export --print) &&
+	git stash clear &&
+	git stash import "$simple" &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1 &&
+	git stash export --to-ref refs/heads/foo &&
+	test_cmp_rev "$(test_oid empty_tree)" foo: &&
+	test_cmp_rev "$(test_oid empty_tree)" foo^: &&
+	test_cmp_rev t-stash0 foo^2 &&
+	test_cmp_rev t-stash1 foo^^2 &&
+	git log --first-parent --format="%s" refs/heads/foo >log &&
+	grep "^git stash: " log >log2 &&
+	test_line_count = 13 log2 &&
+	git stash clear &&
+	git stash import foo &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1
+'
+
+test_expect_success 'stash import appends commits' '
+	git log --format=oneline -g refs/stash >out &&
+	cat out out >out2 &&
+	git stash import refs/heads/foo &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = $(wc -l <out2) actual
+'
+
+test_expect_success 'stash export can accept specified stashes' '
+	git stash clear &&
+	git stash import foo &&
+	git stash export --to-ref refs/heads/bar stash@{1} stash@{0} &&
+	git stash clear &&
+	git stash import refs/heads/bar &&
+	test_cmp_rev stash@{1} t-stash0 &&
+	test_cmp_rev stash@{0} t-stash1 &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = 2 actual
+'
+
+test_expect_success 'stash can import and export zero stashes' '
+	git stash clear &&
+	git stash export --to-ref refs/heads/baz &&
+	test_cmp_rev "$(test_oid empty_tree)" baz: &&
+	test_cmp_rev "$(test_oid export_base)" baz &&
+	test_must_fail git rev-parse baz^1 &&
+	git stash import baz &&
+	test_must_fail git rev-parse refs/stash
+'
+
+test_expect_success 'stash rejects invalid attempts to import commits' '
+	git stash import foo &&
+	test_must_fail git stash import HEAD 2>output &&
+	oid=$(git rev-parse HEAD) &&
+	grep "$oid is not a valid exported stash commit" output &&
+	test_cmp_rev stash@{0} t-stash0 &&
+
+	git checkout --orphan orphan &&
+	git commit-tree $(test_oid empty_tree) -p "$oid" -p "$oid^" -m "" >fake-commit &&
+	git update-ref refs/heads/orphan "$(cat fake-commit)" &&
+	oid=$(git rev-parse HEAD) &&
+	test_must_fail git stash import orphan 2>output &&
+	grep "found stash commit $oid with unexpected prefix" output &&
+	test_cmp_rev stash@{0} t-stash0 &&
+
+	git checkout --orphan orphan2 &&
+	git commit-tree $(test_oid empty_tree) -m "" >fake-commit &&
+	git update-ref refs/heads/orphan2 "$(cat fake-commit)" &&
+	oid=$(git rev-parse HEAD) &&
+	test_must_fail git stash import orphan2 2>output &&
+	grep "found root commit $oid with invalid data" output &&
+	test_cmp_rev stash@{0} t-stash0
+'
+
 test_expect_success 'stash apply should succeed with unmodified file' '
 	echo base >file &&
 	git add file &&

Re: [PATCH] Makefile: avoid constant rebuilds with compilation database

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On 2025-05-22 at 18:55:18, brian m. carlson wrote:
> Many contributors to software use a Language Server Protocol
> implementation to allow their editor to learn structural information
> about the code they write and provide additional features, such as
> jumping to the declaration or definition of a function or type.  In C,
> the usual implementation is clangd, which requires compiling with clang.

Oops, my apologies.  My patch directory was unclean.  Junio already
picked this up, I believe.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH] Makefile: avoid constant rebuilds with compilation database

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2025-05-22 at 18:55:18, brian m. carlson wrote:
>> Many contributors to software use a Language Server Protocol
>> implementation to allow their editor to learn structural information
>> about the code they write and provide additional features, such as
>> jumping to the declaration or definition of a function or type.  In C,
>> the usual implementation is clangd, which requires compiling with clang.
>
> Oops, my apologies.  My patch directory was unclean.  Junio already
> picked this up, I believe.

Don't fret--mistakes happen.  880146ae (Makefile: avoid constant
rebuilds with compilation database, 2025-05-09) is already in
'master' as of a few days ago.

Re: [PATCH v6 1/5] object-name: make get_oid quietly return an error

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> @@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
>  				 * still fill in the oid with the "old" value,
>  				 * which we can use.
>  				 */

So far in this if/elseif cascade, we covered the case where we found
a reflog entry we are looking for before running out.  So ...

> -			} else {
> +			} else if (!(flags & GET_OID_GENTLY)) {
>  				if (flags & GET_OID_QUIETLY) {
>  					exit(128);
>  				}
>  				die(_("log for '%.*s' only has %d entries"),
>  				    len, str, co_cnt);
>  			}

... existing code chose between a silent exit or die based on
GET_OID_QUIETLY bit in the flags word.  In the updated code, this
block is entered only when the caller did not ask for
GET_OID_GENTLY.  But the point is that if we do not say GENTLY,
we no longer do this "choose between exit or die, either way we are
dead at this point".

OK.

> +			if (flags & GET_OID_GENTLY) {
> +				free(real_ref);
> +				return -1;
> +			}

I am confused.

Imagine that one of the if/elseif cascade handled the situation.
e.g. "The caller asked Nth, we found exactly N entries, so instead
of usual new side of the N-1th, we can give the old side of the Nth"
case is ready to return a success.  Why should the caller in such a
case instead get a failure only because the caller said "do not die
on me; I will handle failures myself"?

Shouldn't it be made the final "} else {" of the if/elseif cascade
instead?

Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> +	strbuf_init(&info->revision, 0);
> +	if (parse_stash_revision(&info->revision, commit, 0)) {
> +		return -1;
>  	}

It does not look like this series add more code inside this block in
a later step, so let's lose the unnecessary {braces} around a single
statement "return -1" here.


By the way, what is "pwodd" I saw in the e-mail header?

Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> +static int check_stash_topology(struct repository *r, struct commit *stash)
> +{
> +	struct commit *p1, *p2, *p3 = NULL;
> +
> +	/* stash must have two or three parents */
> +	if (!stash->parents || !stash->parents->next ||
> +			(stash->parents->next->next && stash->parents->next->next->next))
> +		return -1;
> +	p1 = stash->parents->item;
> +	p2 = stash->parents->next->item;
> +	if (stash->parents->next->next)
> +		p3 = stash->parents->next->next->item;
> +	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
> +			(p3 && repo_parse_commit(r, p3)))
> +		return -1;
> +	/* p2 must have a single parent, p3 must have no parents */
> +	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
> +		return -1;
> +	if (repo_parse_commit(r, p2->parents->item))
> +		return -1;
> +	/* p2^1 must equal p1 */
> +	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
> +		return -1;
> +
> +	return 0;
> +}

This is much more elaborate than the existing "does this commit look
like a stash entry" test done elsewhere, isn't it?  Very nicely done.

> +	this = lookup_commit_reference(r, oid);
> +	buffer = repo_get_commit_buffer(r, this, &bufsize);
> +	orig_author = find_commit_header(buffer, "author", &author_len);
> +	orig_committer = find_commit_header(buffer, "committer", &committer_len);
> +
> +	if (!orig_author || !orig_committer) {
> +		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +
> +	if (split_ident_line(&id, orig_author, author_len) < 0 ||
> +	    split_ident_line(&id, orig_committer, committer_len) < 0) {
> +		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
> +		goto out;
> +	}
> +
> +	p = strstr(buffer, "\n\n");
> +	strbuf_addstr(&msg, "git stash: ");
> +
> +	if (p)
> +		strbuf_add(&msg, p + 2, bufsize - (p + 2 - buffer));
> +	strbuf_complete_line(&msg);
> +
> +	author = xmemdupz(orig_author, author_len);
> +	committer = xmemdupz(orig_committer, committer_len);
> +
> +	if (commit_tree_extended(msg.buf, msg.len,
> +				 r->hash_algo->empty_tree, parents,
> +				 out, author, committer,
> +				 NULL, NULL)) {
> +		ret = error(_("could not write commit"));
> +		goto out;
> +	}
> +out:
> +	strbuf_release(&msg);
> +	repo_unuse_commit_buffer(r, this, buffer);
> +	free_commit_list(parents);
> +	free(author);
> +	free(committer);
> +	return ret;
> +}

Nice.

> +	if (argc) {
> +		/*
> +		 * Find each specified stash, and load data into the array.
> +		 */
> +		for (i = 0; i < argc; i++) {
> +			struct object_id oid;
> +			struct commit *stash;
> +
> +			if (parse_stash_revision(&revision, argv[i], 1) ||
> +			    get_oid_with_context(r, revision.buf,
> +						 GET_OID_QUIETLY | GET_OID_GENTLY,
> +						 &oid, &unused)) {
> +				res = error(_("unable to find stash entry %s"), argv[i]);
> +				goto out;
> +			}
> +
> +			stash = lookup_commit_reference(r, &oid);
> +			if (!stash || check_stash_topology(r, stash)) {
> +				res = error(_("%s does not look like a stash commit"),
> +						revision.buf);
> +				goto out;
> +			}
> +			oid_array_append(&items, &oid);
> +		}
> +	} else {
> +		/*
> +		 * Walk the reflog, finding each stash entry, and load data into the
> +		 * array.
> +		 */
> +		struct complete_reflogs *crl = read_complete_reflog("refs/stash");
> +		for (i = 0; i < crl->nr; i++) {
> +			/*
> +			 * Iterate from the latest item in the reflog to the
> +			 * earliest.  crl->items starts from the earliest first.
> +			 */
> +			struct reflog_info *item = crl->items + (crl->nr - i - 1);
> +			struct commit *stash;
> +
> +			stash = lookup_commit_reference(r, &item->noid);
> +			if (!stash || check_stash_topology(r, stash)) {
> +				res = error(_("%s does not look like a stash commit"),
> +						revision.buf);
> +				goto out;
> +			}
> +			oid_array_append(&items, &item->noid);
> +		}
> +		free_complete_reflog(crl);
> +	}

Hmph.  As we work with the commit objects in the above part already,
would it still make sense to use an oid array?  That would force us
to look up the same set of commits using the object names again in
the later loop.  I would have thought we might use commit_list or
something to accumulate commits, and then consume in the loop below.

Not a huge deal, though.  But I somehow find it more natural.  

In addition, we would not have to worry about the ssize_t cast and
other complications if we used commit_list to hold these incoming
stash entries.

Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

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

Very sensible.

> +static int do_import_stash(struct repository *r, const char *rev)
> +{
> +	struct object_id chain;
> +	struct oid_array items = OID_ARRAY_INIT;
> +	int res = 0;
> +	ssize_t i;
> +	const char *buffer = NULL;
> +	unsigned long bufsize;
> +	struct commit *this = NULL;
> +	char *msg = NULL;
> +
> +	if (repo_get_oid(r, rev, &chain))
> +		return error(_("not a valid revision: %s"), rev);
> +
> +	this = lookup_commit_reference(r, &chain);
> +	if (!this)
> +		return error(_("not a commit: %s"), rev);
> +
> +	/*
> +	 * Walk the commit history, finding each stash entry, and load data into
> +	 * the array.
> +	 */
> +	for (;;) {
> +		const char *author, *committer;
> +		size_t author_len, committer_len;
> +		const char *p;
> +		const char *expected = "git stash <git@stash> 1000684800 +0000";

Makes me wonder if we can somehow share this with [4/5] where we
establish the author and committer ident (timestamp spelled in a
different form).  Three strings that has to stay in sync is not a
huge deal, though.

> +		const char *prefix = "git stash: ";
> +		struct commit *stash;
> +		struct tree *tree = repo_get_commit_tree(r, this);
> +
> +		if (!tree ||
> +		    !oideq(&tree->object.oid, r->hash_algo->empty_tree) ||
> +		    (this->parents &&
> +		     (!this->parents->next || this->parents->next->next))) {
> +			res = error(_("%s is not a valid exported stash commit"),
> +					oid_to_hex(&this->object.oid));
> +			goto out;
> +		}

OK.  "buffer" is initialized to NULL before entering this loop, and
cleared to NULL at the end of this loop when "this" is moved, so
jumping to "out:" would not confuse the call to unuse_buffer().

> +		buffer = repo_get_commit_buffer(r, this, &bufsize);

And "buffer" becomes non-NULL and holds data associated with "this";
any "goto out" in the error path would do the right thing.  Very
nicely designed loop.

> +		if (!this->parents) {
> +			/*
> +			 * We don't have any parents.  Make sure this is our
> +			 * root commit.
> +			 */
> +			author = find_commit_header(buffer, "author", &author_len);
> +			committer = find_commit_header(buffer, "committer", &committer_len);
> +
> +			if (!author || !committer) {
> +				error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
> +				goto out;
> +			}
> +
> +			if (author_len != strlen(expected) ||
> +			    committer_len != strlen(expected) ||
> +			    memcmp(author, expected, author_len) ||
> +			    memcmp(committer, expected, committer_len)) {
> +				res = error(_("found root commit %s with invalid data"), oid_to_hex(&this->object.oid));
> +				goto out;
> +			}
> +			break;
> +		}

OK.

> +		p = strstr(buffer, "\n\n");
> +		if (!p) {
> +			res = error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
> +			goto out;
> +		}
> +
> +		p += 2;
> +		if (((size_t)(bufsize - (p - buffer)) < strlen(prefix)) ||
> +		    memcmp(prefix, p, strlen(prefix))) {
> +			res = error(_("found stash commit %s with unexpected prefix"), oid_to_hex(&this->object.oid));
> +			goto out;
> +		}

I'd call that "without expected prefix" (the difference being "we
expected 'git stash:' prefix but you have 'git stosh:' prefix" vs
"your commit title is 'hello world'"), but OK.  The important thing
is that we are being careful not to get confused.

It is unfortunate historical practice to measure almost everything
in "unsigned long" and bufsize here is one of them, but do we need
that cast?  We know strlen(prefix) is a small known constant
integer, we know p is not smaller than buffer, we know (p-buffer) is
not larger than bufsize.

> +		stash = this->parents->next->item;
> +
> +		if (repo_parse_commit(r, this->parents->item) ||
> +		    repo_parse_commit(r, stash)) {
> +			res = error(_("cannot parse parents of commit: %s"),
> +					oid_to_hex(&this->object.oid));
> +			goto out;
> +		}
> +
> +		if (check_stash_topology(r, stash)) {
> +			res = error(_("%s does not look like a stash commit"),
> +					oid_to_hex(&stash->object.oid));
> +			goto out;
> +		}
> +
> +		repo_unuse_commit_buffer(r, this, buffer);
> +		buffer = NULL;
> +		oid_array_append(&items, &stash->object.oid);
> +		this = this->parents->item;
> +	}
> +
> +	/*
> +	 * Now, walk each entry, adding it to the stash as a normal stash
> +	 * commit.
> +	 */
> +	for (i = (ssize_t)items.nr - 1; i >= 0; i--) {
> +		const char *p;
> +		const struct object_id *oid = items.oid + i;
> +
> +		this = lookup_commit_reference(r, oid);
> +		buffer = repo_get_commit_buffer(r, this, &bufsize);
> +		if (!buffer) {
> +			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
> +			goto out;
> +		}
> +
> +		p = strstr(buffer, "\n\n");
> +		if (!p) {
> +			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
> +			goto out;
> +		}
> +
> +		p += 2;
> +		msg = xmemdupz(p, bufsize - (p - buffer));
> +		repo_unuse_commit_buffer(r, this, buffer);
> +		buffer = NULL;
> +
> +		if (do_store_stash(oid, msg, 1)) {
> +			res = error(_("cannot save the stash for %s"), oid_to_hex(oid));
> +			goto out;
> +		}
> +		FREE_AND_NULL(msg);
> +	}
> +out:
> +	if (this && buffer)
> +		repo_unuse_commit_buffer(r, this, buffer);
> +	oid_array_clear(&items);
> +	free(msg);
> +
> +	return res;
> +}

OK.  Again, I suspect that commit_list may be a more natural thing
to use to accumulate the stash entries, but that is not a huge deal.

Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> 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.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> I'll need Phillip's sign-off for this patch.

Let's Cc'em, then.

Ahh, it occurs to me that your pwodd may be a typo of that address?

Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[Ramsay Jones]

On 22/05/2025 19:55, brian m. carlson wrote:
> In a future commit, we'll use this function and the corresponding free
> function to read the entire reflog.  Expose it in the header so we can
> do so.
> 
> Include the appropriate header files so that our header is complete.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  reflog-walk.c | 17 ++---------------
>  reflog-walk.h | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/reflog-walk.c b/reflog-walk.c
> index c7070b13b0..b7a9d70966 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -9,19 +9,6 @@
>  #include "string-list.h"
>  #include "reflog-walk.h"
>  
> -struct complete_reflogs {
> -	char *ref;
> -	char *short_ref;
> -	struct reflog_info {
> -		struct object_id ooid, noid;
> -		char *email;
> -		timestamp_t timestamp;
> -		int tz;
> -		char *message;
> -	} *items;
> -	int nr, alloc;
> -};
> -
>  static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
>  		const char *email, timestamp_t timestamp, int tz,
>  		const char *message, void *cb_data)
> @@ -41,7 +28,7 @@ static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
>  	return 0;
>  }
>  
> -static void free_complete_reflog(struct complete_reflogs *array)
> +void free_complete_reflog(struct complete_reflogs *array)
>  {
>  	int i;
>  
> @@ -64,7 +51,7 @@ static void complete_reflogs_clear(void *util, const char *str UNUSED)
>  	free_complete_reflog(array);
>  }
>  
> -static struct complete_reflogs *read_complete_reflog(const char *ref)
> +struct complete_reflogs *read_complete_reflog(const char *ref)
>  {
>  	struct complete_reflogs *reflogs =
>  		xcalloc(1, sizeof(struct complete_reflogs));
> diff --git a/reflog-walk.h b/reflog-walk.h
> index 989583dc55..8f0640f662 100644
> --- a/reflog-walk.h
> +++ b/reflog-walk.h
> @@ -1,9 +1,24 @@
>  #ifndef REFLOG_WALK_H
>  #define REFLOG_WALK_H
>  
> +#include "git-compat-util.h"

Why? 'git-compat-util.h' must be #include-d in any compilation
unit before 'reflog-walk.h', so ...

(Sorry, I have not studied these patches, they were just floating
in front of my eyes ... so, please ignore me if the reason is
obvious! :) ).

ATB,
Ramsay Jones

> +#include "hash.h"
> +
>  struct commit;
>  struct reflog_walk_info;
>  struct date_mode;
> +struct complete_reflogs {
> +	char *ref;
> +	char *short_ref;
> +	struct reflog_info {
> +		struct object_id ooid, noid;
> +		char *email;
> +		timestamp_t timestamp;
> +		int tz;
> +		char *message;
> +	} *items;
> +	int nr, alloc;
> +};
>  
>  void init_reflog_walk(struct reflog_walk_info **info);
>  void reflog_walk_info_release(struct reflog_walk_info *info);
> @@ -24,4 +39,7 @@ int reflog_walk_empty(struct reflog_walk_info *walk);
>  
>  struct commit *next_reflog_entry(struct reflog_walk_info *reflog_info);
>  
> +void free_complete_reflog(struct complete_reflogs *array);
> +struct complete_reflogs *read_complete_reflog(const char *ref);
> +
>  #endif
> 

Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref

[Phillip Wood]

On 22/05/2025 22:15, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
>> I'll need Phillip's sign-off for this patch.
> 
> Let's Cc'em, then.

Thanks, I'm glad the fixups were useful, sorry I forgot to add my sign 
off to the last one - here it is

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>

I'll take a proper look at this series next week

Thanks

Phillip

> Ahh, it occurs to me that your pwodd may be a typo of that address?

Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

On 2025-05-22 at 21:53:27, Ramsay Jones wrote:
> 
> 
> On 22/05/2025 19:55, brian m. carlson wrote:
> > diff --git a/reflog-walk.h b/reflog-walk.h
> > index 989583dc55..8f0640f662 100644
> > --- a/reflog-walk.h
> > +++ b/reflog-walk.h
> > @@ -1,9 +1,24 @@
> >  #ifndef REFLOG_WALK_H
> >  #define REFLOG_WALK_H
> >  
> > +#include "git-compat-util.h"
> 
> Why? 'git-compat-util.h' must be #include-d in any compilation
> unit before 'reflog-walk.h', so ...

One of the CI jobs fails if we don't include `hash.h` for `struct
object_id`.  I don't remember which one.  The rule is that we always
include that header before any other header, so that's what I did here.
It is also needed for `timestamp_t`.

I will also mention that having complete headers makes clangd and other
LSPs work better because then they don't warn about undefined types and
they can actually warn when we have failed to include the relevant type,
and since we actually both headers here, I decided that was the right
thing to do.

If we're dead set against it, I can remove it.

> (Sorry, I have not studied these patches, they were just floating
> in front of my eyes ... so, please ignore me if the reason is
> obvious! :) ).
> 
> ATB,
> Ramsay Jones
> 
> > +#include "hash.h"
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On 2025-05-22 at 20:34:16, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > +	strbuf_init(&info->revision, 0);
> > +	if (parse_stash_revision(&info->revision, commit, 0)) {
> > +		return -1;
> >  	}
> 
> It does not look like this series add more code inside this block in
> a later step, so let's lose the unnecessary {braces} around a single
> statement "return -1" here.

Sounds good.  I'll fix that in a v7.

> By the way, what is "pwodd" I saw in the e-mail header?

A typo for "pwood", which is my alias for Phillip Wood, who I intended
to CC.  A typo which I unfortunately didn't catch before sending out
patches (mostly because I was sitting in an airport lounge when I sent
them out instead of using my giant screens at home).
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> A typo for "pwood", which is my alias for Phillip Wood, who I intended
> to CC.  A typo which I unfortunately didn't catch before sending out
> patches (mostly because I was sitting in an airport lounge when I sent
> them out instead of using my giant screens at home).

Should git-send-email be able to (optionally) catch a typo like
this?  E.g., we have an address without @ in it, and if we feed it
to alias expansion, it comes back without a change.  It could be a
local address but is more likely a typo if you are sending the
message also to an external recipients with @ in their addresses.

Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[Ramsay Jones]

On 24/05/2025 00:22, brian m. carlson wrote:
> On 2025-05-22 at 21:53:27, Ramsay Jones wrote:
>>
>>
>> On 22/05/2025 19:55, brian m. carlson wrote:
>>> diff --git a/reflog-walk.h b/reflog-walk.h
>>> index 989583dc55..8f0640f662 100644
>>> --- a/reflog-walk.h
>>> +++ b/reflog-walk.h
>>> @@ -1,9 +1,24 @@
>>>  #ifndef REFLOG_WALK_H
>>>  #define REFLOG_WALK_H
>>>  
>>> +#include "git-compat-util.h"
>>
>> Why? 'git-compat-util.h' must be #include-d in any compilation
>> unit before 'reflog-walk.h', so ...
> 
> One of the CI jobs fails if we don't include `hash.h` for `struct
> object_id`.  I don't remember which one.  The rule is that we always
> include that header before any other header, so that's what I did here.
> It is also needed for `timestamp_t`.

The rule refers to '*.c' files not '*.h' files. ie. we #include
'git-compat-util.h' at the beginning of all C files, so at the
start of all compilation units (well Documentation/CodingGuidelines
has some exceptions, but ...).

So, at this point, 'git-compat-util.h' will already have been seen
by the compiler (so it won't cause any problem, but equally it is
not needed). In order to determine if the header can stand alone
you should just need to:

    $ make reflog-walk.hco

[That is what the hdr-check target is for].

So, yes, include 'hash.h' to make the header file stand alone,
but 'git-compat-util.h' should not be needed.

Thanks.

ATB,
Ramsay Jones

Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On 2025-05-24 at 00:23:06, Junio C Hamano wrote:
> Should git-send-email be able to (optionally) catch a typo like
> this?  E.g., we have an address without @ in it, and if we feed it
> to alias expansion, it comes back without a change.  It could be a
> local address but is more likely a typo if you are sending the
> message also to an external recipients with @ in their addresses.

That seems like it might be a good idea.  We could override it with a
command-line flag in the unlikely situation that the user really wanted
that.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]

On 2025-05-22 at 20:51:20, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > +static int check_stash_topology(struct repository *r, struct commit *stash)
> > +{
> > +	struct commit *p1, *p2, *p3 = NULL;
> > +
> > +	/* stash must have two or three parents */
> > +	if (!stash->parents || !stash->parents->next ||
> > +			(stash->parents->next->next && stash->parents->next->next->next))
> > +		return -1;
> > +	p1 = stash->parents->item;
> > +	p2 = stash->parents->next->item;
> > +	if (stash->parents->next->next)
> > +		p3 = stash->parents->next->next->item;
> > +	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
> > +			(p3 && repo_parse_commit(r, p3)))
> > +		return -1;
> > +	/* p2 must have a single parent, p3 must have no parents */
> > +	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
> > +		return -1;
> > +	if (repo_parse_commit(r, p2->parents->item))
> > +		return -1;
> > +	/* p2^1 must equal p1 */
> > +	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
> > +		return -1;
> > +
> > +	return 0;
> > +}
> 
> This is much more elaborate than the existing "does this commit look
> like a stash entry" test done elsewhere, isn't it?  Very nicely done.

This is Phillip Wood's code, but yes, it is very nice.  I looked at the
patch he provided and it seemed very sensible, so I adopted it.

> Hmph.  As we work with the commit objects in the above part already,
> would it still make sense to use an oid array?  That would force us
> to look up the same set of commits using the object names again in
> the later loop.  I would have thought we might use commit_list or
> something to accumulate commits, and then consume in the loop below.

I can do that in v7.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On 2025-05-24 at 01:09:49, Ramsay Jones wrote:
> So, at this point, 'git-compat-util.h' will already have been seen
> by the compiler (so it won't cause any problem, but equally it is
> not needed). In order to determine if the header can stand alone
> you should just need to:
> 
>     $ make reflog-walk.hco
> 
> [That is what the hdr-check target is for].
> 
> So, yes, include 'hash.h' to make the header file stand alone,
> but 'git-compat-util.h' should not be needed.

Okay, I'll change that here, but it does make clangd much less useful
because `hash.h` does actually depend on `git-compat-util.h` and so
clangd flags lots of missing types.  Perhaps we should revisit that in
the future.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On 2025-05-22 at 21:09:23, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> > +		const char *author, *committer;
> > +		size_t author_len, committer_len;
> > +		const char *p;
> > +		const char *expected = "git stash <git@stash> 1000684800 +0000";
> 
> Makes me wonder if we can somehow share this with [4/5] where we
> establish the author and committer ident (timestamp spelled in a
> different form).  Three strings that has to stay in sync is not a
> huge deal, though.

I'll take a look and see if I can come up with a nice way to solve this.

> > +		p = strstr(buffer, "\n\n");
> > +		if (!p) {
> > +			res = error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
> > +			goto out;
> > +		}
> > +
> > +		p += 2;
> > +		if (((size_t)(bufsize - (p - buffer)) < strlen(prefix)) ||
> > +		    memcmp(prefix, p, strlen(prefix))) {
> > +			res = error(_("found stash commit %s with unexpected prefix"), oid_to_hex(&this->object.oid));
> > +			goto out;
> > +		}
> 
> I'd call that "without expected prefix" (the difference being "we
> expected 'git stash:' prefix but you have 'git stosh:' prefix" vs
> "your commit title is 'hello world'"), but OK.  The important thing
> is that we are being careful not to get confused.

Will fix.

> It is unfortunate historical practice to measure almost everything
> in "unsigned long" and bufsize here is one of them, but do we need
> that cast?  We know strlen(prefix) is a small known constant
> integer, we know p is not smaller than buffer, we know (p-buffer) is
> not larger than bufsize.

Yes, we do, because Windows will complain about a comparison between
signed and unsigned integer.  `p - buffer` is `ptrdiff_t`, which is 64
bits, but `unsigned long` is only 32 bite, so they both get promoted to
long long, which is 64 bits and signed, and then the comparison is to
`size_t`, which is 64 bits and unsigned.  The CI jobs fail without this.

I'll make a note in the commit message to that effect.

> OK.  Again, I suspect that commit_list may be a more natural thing
> to use to accumulate the stash entries, but that is not a huge deal.

I'll try to fix that in v7.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref

[Phillip Wood]

Hi brian

This is looking pretty nice, I don't have much to add to Junio's comments.

On 22/05/2025 19:55, brian m. carlson wrote:
> [...]
> +out:
> +	strbuf_release(&msg);
> +	repo_unuse_commit_buffer(r, this, buffer);
> +	free_commit_list(parents);

I found this confusing when I was reading the caller as I couldn't see 
where the list of parent commits was being free()d. I think it would be 
easier to follow if this function did not take ownership of the list.

> +	/*
> +	 * Now, create a set of commits identical to the regular stash commits,
> +	 * but where their first parents form a chain to our original empty
> +	 * base commit.
> +	 */
> +	for (i = (ssize_t)items.nr - 1; i >= 0; i--) {

This cast is pretty horrible. ssize_t is not guaranteed to be the same 
width as size_t (see [1] for a bug fix on NonStop caused by this). I 
think we'd normally write this as

	for (size_t i = items.nr; i > 0; i--) {
		const struct object_id *oid = &items[i - 1];

[1] c14e5a1a501 (transport-helper: use xread instead of read, 2019-01-03)

> +		struct commit_list *parents = NULL;
> +		struct commit_list **next = &parents;
> +		struct object_id out;
> +		const struct object_id *oid = items.oid + i;
> [...]
> +static int export_stash(int argc,
> +			const char **argv,
> +			const char *prefix,
> +			struct repository *repo)
> +{
> +	const char *ref = NULL;
> +	enum export_action action = ACTION_NONE;
> +	struct option options[] = {
> +		OPT_CMDMODE(0, "print", &action,
> +			    N_("print the object ID instead of writing it to a ref"),
> +			    ACTION_PRINT),
> +		OPT_STRING(0, "to-ref", &ref, "ref",
> +			    N_("save the data to the given ref")),
> +		OPT_END()
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, options,
> +			     git_stash_export_usage,
> +			     PARSE_OPT_KEEP_DASHDASH);
> +
> +	if (ref && action == ACTION_NONE)
> +		action = ACTION_TO_REF;
> +
> +	if (action == ACTION_NONE)
> +		return error(_("exactly one of --print and --to-ref is required"));
I don't think this protects against 'git stash export --print --to-ref'. 
It is a bit odd to have a single option marked with OPT_CMDMODE() it 
might be worth changing that to OPT_SET_INT().

Best Wishes

Phillip

Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[Phillip Wood]

On 22/05/2025 19:55, brian m. carlson wrote:
> In a future commit, we'll use this function and the corresponding free
> function to read the entire reflog.  Expose it in the header so we can
> do so.

We already have refs_for_each_reflog_entry() and 
refs_for_each_reflog_entry_reverse() for traversing the reflog entries 
so I'm a bit confused as to why we need to make the internal details public.

Thanks

Phillip
> Include the appropriate header files so that our header is complete.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>   reflog-walk.c | 17 ++---------------
>   reflog-walk.h | 18 ++++++++++++++++++
>   2 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/reflog-walk.c b/reflog-walk.c
> index c7070b13b0..b7a9d70966 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -9,19 +9,6 @@
>   #include "string-list.h"
>   #include "reflog-walk.h"
>   
> -struct complete_reflogs {
> -	char *ref;
> -	char *short_ref;
> -	struct reflog_info {
> -		struct object_id ooid, noid;
> -		char *email;
> -		timestamp_t timestamp;
> -		int tz;
> -		char *message;
> -	} *items;
> -	int nr, alloc;
> -};
> -
>   static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
>   		const char *email, timestamp_t timestamp, int tz,
>   		const char *message, void *cb_data)
> @@ -41,7 +28,7 @@ static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
>   	return 0;
>   }
>   
> -static void free_complete_reflog(struct complete_reflogs *array)
> +void free_complete_reflog(struct complete_reflogs *array)
>   {
>   	int i;
>   
> @@ -64,7 +51,7 @@ static void complete_reflogs_clear(void *util, const char *str UNUSED)
>   	free_complete_reflog(array);
>   }
>   
> -static struct complete_reflogs *read_complete_reflog(const char *ref)
> +struct complete_reflogs *read_complete_reflog(const char *ref)
>   {
>   	struct complete_reflogs *reflogs =
>   		xcalloc(1, sizeof(struct complete_reflogs));
> diff --git a/reflog-walk.h b/reflog-walk.h
> index 989583dc55..8f0640f662 100644
> --- a/reflog-walk.h
> +++ b/reflog-walk.h
> @@ -1,9 +1,24 @@
>   #ifndef REFLOG_WALK_H
>   #define REFLOG_WALK_H
>   
> +#include "git-compat-util.h"
> +#include "hash.h"
> +
>   struct commit;
>   struct reflog_walk_info;
>   struct date_mode;
> +struct complete_reflogs {
> +	char *ref;
> +	char *short_ref;
> +	struct reflog_info {
> +		struct object_id ooid, noid;
> +		char *email;
> +		timestamp_t timestamp;
> +		int tz;
> +		char *message;
> +	} *items;
> +	int nr, alloc;
> +};
>   
>   void init_reflog_walk(struct reflog_walk_info **info);
>   void reflog_walk_info_release(struct reflog_walk_info *info);
> @@ -24,4 +39,7 @@ int reflog_walk_empty(struct reflog_walk_info *walk);
>   
>   struct commit *next_reflog_entry(struct reflog_walk_info *reflog_info);
>   
> +void free_complete_reflog(struct complete_reflogs *array);
> +struct complete_reflogs *read_complete_reflog(const char *ref);
> +
>   #endif
> 

Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 22/05/2025 19:55, brian m. carlson wrote:
>> In a future commit, we'll use this function and the corresponding free
>> function to read the entire reflog.  Expose it in the header so we can
>> do so.
>
> We already have refs_for_each_reflog_entry() and
> refs_for_each_reflog_entry_reverse() for traversing the reflog entries
> so I'm a bit confused as to why we need to make the internal details
> public.

The suggestion for using this function is largely fault of mine, who
did not think of using the for-each helpers.

Looking at the function signature of for-each-reflog-ent callback

        typedef int each_reflog_ent_fn(
                        struct object_id *old_oid, struct object_id *new_oid,
                        const char *committer, timestamp_t timestamp,
                        int tz, const char *msg, void *cb_data);

I think it gives us the same piece of information, and is
preferrable.

Thanks for a dose of sanity.

[PATCH v7 0/4] Importing and exporting stashes to refs

[brian m. carlson]

Stashes are currently stored using the reflog in a given repository.
This is an interesting and novel way to handle them, but there is no way
to easily move a set of stashes across machines.  For example, groups of
stashes cannot be bundled, pushed, or fetched.

Let's solve this problem by allowing users to import and export stashes
to a chain of commits.  The commits used in a stash export contain two
parents: one which is the pointer to the next exported stash (or to an
empty commit with no parents if there are no more) and the second is the
stash commit that would normally be stored in the reflog.

Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>

Changes from v6:
* Add Phillip's sign-off to the last patch.
* Use `commit_list` for tracking commits.
* Use reflog entry walker.
* Fix some commit messages for improved legibility.
* Rephrase some error messages for precision.
* Drop the patch that exposes `read_complete_reflog` since it is no longer necessary.

Changes from v5:
* Rename `parse_revision`.
* Remove extra call to `free_stash_info`.
* Fix parsing of existing commit.
* Add more validation of imported stash commits.
* Add more tests for improved validation of imported stash commits.
* Explicitly cast `items.nr` and make the iteration counter an `ssize_t`
  to avoid casting problems.
* Don't require a trailing `\n\n` in commit messages.
* Use `read_complete_reflog` to walk reflogs.
* Be more defensive when using `lookup_commit_reference`.
* Apply parts of Phillip's patches for improved robustness.
* Update commit message to explain additional use cases.
* Use `OPT_STRING` for `--to-ref`.

Changes from v4:
* Fix another use of oid_array.
* Fix various memory leaks.
* Fix a segfault which appeared after a rebase.
* Use strstr for commits since we don't need to worry about NUL.
* Added some additional tests.
* Verify the ident values we're using to avoid using bad values.
* Various other code cleanups.
* Rebase on `master`.

Changes from v3:
* Fix strbuf handling to avoid leaks and generally be more sensible.
* Make use of the error return code more often.
* Use oid_array.
* Tidy various parts of the code and fix long lines.
* Simplify tests using git tag.
* Shorten and tidy tests.
* Add an additional test covering the base commit OID and importing and
  exporting empty stashes.

Changes from v2:
* Fix uninitialized strbuf.
* Avoid C99-style initializations.

Changes from v1:
* Change storage format as suggested by Junio.
* Rename to GIT_OID_GENTLY.
* Remove unnecessary initializations.
* Use ALLOC_GROW_BY.
* Ensure completely reproducible exports.
* Avoid size_t.
* Various other code cleanups.

brian m. carlson (4):
  object-name: make get_oid quietly return an error
  builtin/stash: factor out revision parsing into a function
  builtin/stash: provide a way to export stashes to a ref
  builtin/stash: provide a way to import stashes from a ref

 Documentation/git-stash.adoc |  29 ++-
 builtin/stash.c              | 460 ++++++++++++++++++++++++++++++++++-
 hash.h                       |   1 +
 object-name.c                |   6 +-
 t/t3903-stash.sh             |  94 +++++++
 5 files changed, 577 insertions(+), 13 deletions(-)

[PATCH v7 1/4] object-name: make get_oid quietly return an error

[brian m. carlson]

A reasonable person looking at the signature and usage of get_oid and
friends might conclude that in the event of an error, it always returns
-1.  However, this is not the case.  Instead, get_oid_basic dies if we
go too far back into the history of a reflog (or, when quiet, simply
exits).

This is not especially useful, since in many cases, we might want to
handle this error differently.  Let's add a flag here to make it just
return -1 like elsewhere in these code paths.

Note that we cannot make this behavior the default, since we have many
other codepaths that rely on the existing behavior, including in tests.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 hash.h        | 1 +
 object-name.c | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hash.h b/hash.h
index d6422ddf45..ec594c63a6 100644
--- a/hash.h
+++ b/hash.h
@@ -216,6 +216,7 @@ struct object_id {
 #define GET_OID_REQUIRE_PATH         010000
 #define GET_OID_HASH_ANY             020000
 #define GET_OID_SKIP_AMBIGUITY_CHECK 040000
+#define GET_OID_GENTLY              0100000
 
 #define GET_OID_DISAMBIGUATORS \
 	(GET_OID_COMMIT | GET_OID_COMMITTISH | \
diff --git a/object-name.c b/object-name.c
index 9288b2dd24..851858975f 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 				 * still fill in the oid with the "old" value,
 				 * which we can use.
 				 */
-			} else {
+			} else if (!(flags & GET_OID_GENTLY)) {
 				if (flags & GET_OID_QUIETLY) {
 					exit(128);
 				}
 				die(_("log for '%.*s' only has %d entries"),
 				    len, str, co_cnt);
 			}
+			if (flags & GET_OID_GENTLY) {
+				free(real_ref);
+				return -1;
+			}
 		}
 	}
 

[PATCH v7 2/4] builtin/stash: factor out revision parsing into a function

[brian m. carlson]

We allow several special forms of stash names in this code.  In the
future, we'll want to allow these same forms without parsing a stash
commit, so let's refactor this code out into a function for reuse.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/stash.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index cfbd92852a..ab491d5ff6 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -169,6 +169,25 @@ static void assert_stash_like(struct stash_info *info, const char *revision)
 		die(_("'%s' is not a stash-like commit"), revision);
 }
 
+static int parse_stash_revision(struct strbuf *revision, const char *commit, int quiet)
+{
+	strbuf_reset(revision);
+	if (!commit) {
+		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
+			if (!quiet)
+				fprintf_ln(stderr, _("No stash entries found."));
+			return -1;
+		}
+
+		strbuf_addf(revision, "%s@{0}", ref_stash);
+	} else if (strspn(commit, "0123456789") == strlen(commit)) {
+		strbuf_addf(revision, "%s@{%s}", ref_stash, commit);
+	} else {
+		strbuf_addstr(revision, commit);
+	}
+	return 0;
+}
+
 static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 {
 	int ret;
@@ -196,17 +215,9 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 	if (argc == 1)
 		commit = argv[0];
 
-	if (!commit) {
-		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
-			fprintf_ln(stderr, _("No stash entries found."));
-			return -1;
-		}
-
-		strbuf_addf(&info->revision, "%s@{0}", ref_stash);
-	} else if (strspn(commit, "0123456789") == strlen(commit)) {
-		strbuf_addf(&info->revision, "%s@{%s}", ref_stash, commit);
-	} else {
-		strbuf_addstr(&info->revision, commit);
+	strbuf_init(&info->revision, 0);
+	if (parse_stash_revision(&info->revision, commit, 0)) {
+		return -1;
 	}
 
 	revision = info->revision.buf;

[PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref

[brian m. carlson]

A common user problem is how to sync in-progress work to another
machine.  Users currently must use some sort of transfer of the working
tree, which poses security risks and also necessarily causes the index
to become dirty.  The experience is suboptimal and frustrating for
users.

A reasonable idea is to use the stash for this purpose, but the stash is
stored in the reflog, not in a ref, and as such it cannot be pushed or
pulled.  This also means that it cannot be saved into a bundle or
preserved elsewhere, which is a problem when using throwaway development
environments.

In addition, users often want to replicate stashes across machines, such
as when they must use multiple machines or when they use throwaway dev
environments, such as those based on the Devcontainer spec, where they
might otherwise lose various in-progress work.

Let's solve this problem by allowing the user to export the stash to a
ref (or, to just write it into the repository and print the hash, à la
git commit-tree).  Introduce git stash export, which writes a chain of
commits where the first parent is always a chain to the previous stash,
or to a single, empty commit (for the final item) and the second is the
stash commit normally written to the reflog.

Iterate over each stash from top to bottom, looking up the data for each
one, and then create the chain from the single empty commit back up in
reverse order.  Generate a predictable empty commit so our behavior is
reproducible.  Create a useful commit message, preserving the author and
committer information, to help users identify stash commits when viewing
them as normal commits.

If the user has specified specific stashes they'd like to export
instead, use those instead of iterating over all of the stashes.

As part of this, specifically request quiet behavior when looking up the
OID for a revision because we will eventually hit a revision that
doesn't exist and we don't want to die when that occurs.

When exporting stashes, be sure to verify that they look like valid
stashes and don't contain invalid data.  This will help avoid failures
on import or problems due to attempting to export invalid refs that are
not stashes.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-stash.adoc |  22 ++-
 builtin/stash.c              | 260 +++++++++++++++++++++++++++++++++++
 2 files changed, 281 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 1a5177f498..e8efd43ba4 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -23,6 +23,7 @@ SYNOPSIS
 'git stash' clear
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
+'git stash' export (--print | --to-ref <ref>) [<stash>...]
 
 DESCRIPTION
 -----------
@@ -154,6 +155,12 @@ store::
 	reflog.  This is intended to be useful for scripts.  It is
 	probably not the command you want to use; see "push" above.
 
+export ( --print | --to-ref <ref> ) [<stash>...]::
+
+	Export the specified stashes, or all of them if none are specified, to
+	a chain of commits which can be transferred using the normal fetch and
+	push mechanisms, then imported using the `import` subcommand.
+
 OPTIONS
 -------
 -a::
@@ -242,6 +249,19 @@ literally (including newlines and quotes).
 +
 Quiet, suppress feedback messages.
 
+--print::
+	This option is only valid for `export`.
++
+Create the chain of commits representing the exported stashes without
+storing it anywhere in the ref namespace and print the object ID to
+standard output.  This is designed for scripts.
+
+--to-ref::
+	This option is only valid for `export`.
++
+Create the chain of commits representing the exported stashes and store
+it to the specified ref.
+
 \--::
 	This option is only valid for `push` command.
 +
@@ -259,7 +279,7 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 <stash>::
 	This option is only valid for `apply`, `branch`, `drop`, `pop`,
-	`show` commands.
+	`show`, and `export` commands.
 +
 A reference of the form `stash@{<revision>}`. When no `<stash>` is
 given, the latest stash is assumed (that is, `stash@{0}`).
diff --git a/builtin/stash.c b/builtin/stash.c
index ab491d5ff6..bebe004e4d 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -28,7 +28,10 @@
 #include "log-tree.h"
 #include "diffcore.h"
 #include "reflog.h"
+#include "reflog-walk.h"
 #include "add-interactive.h"
+#include "oid-array.h"
+#include "commit.h"
 
 #define INCLUDE_ALL_FILES 2
 
@@ -56,6 +59,8 @@
 	   "          [-u | --include-untracked] [-a | --all] [<message>]")
 #define BUILTIN_STASH_CREATE_USAGE \
 	N_("git stash create [<message>]")
+#define BUILTIN_STASH_EXPORT_USAGE \
+	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -71,6 +76,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CLEAR_USAGE,
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
+	BUILTIN_STASH_EXPORT_USAGE,
 	NULL
 };
 
@@ -124,6 +130,12 @@ static const char * const git_stash_save_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_export_usage[] = {
+	BUILTIN_STASH_EXPORT_USAGE,
+	NULL
+};
+
+
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
 
@@ -160,6 +172,33 @@ static void free_stash_info(struct stash_info *info)
 	strbuf_release(&info->revision);
 }
 
+static int check_stash_topology(struct repository *r, struct commit *stash)
+{
+	struct commit *p1, *p2, *p3 = NULL;
+
+	/* stash must have two or three parents */
+	if (!stash->parents || !stash->parents->next ||
+			(stash->parents->next->next && stash->parents->next->next->next))
+		return -1;
+	p1 = stash->parents->item;
+	p2 = stash->parents->next->item;
+	if (stash->parents->next->next)
+		p3 = stash->parents->next->next->item;
+	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
+			(p3 && repo_parse_commit(r, p3)))
+		return -1;
+	/* p2 must have a single parent, p3 must have no parents */
+	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
+		return -1;
+	if (repo_parse_commit(r, p2->parents->item))
+		return -1;
+	/* p2^1 must equal p1 */
+	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
+		return -1;
+
+	return 0;
+}
+
 static void assert_stash_like(struct stash_info *info, const char *revision)
 {
 	if (get_oidf(&info->b_commit, "%s^1", revision) ||
@@ -1895,6 +1934,226 @@ static int save_stash(int argc, const char **argv, const char *prefix,
 	return ret;
 }
 
+static int write_commit_with_parents(struct repository *r,
+				     struct object_id *out,
+				     const struct object_id *oid,
+				     struct commit_list *parents)
+{
+	size_t author_len, committer_len;
+	struct commit *this;
+	const char *orig_author, *orig_committer;
+	char *author = NULL, *committer = NULL;
+	const char *buffer;
+	unsigned long bufsize;
+	const char *p;
+	struct strbuf msg = STRBUF_INIT;
+	int ret = 0;
+	struct ident_split id;
+
+	this = lookup_commit_reference(r, oid);
+	buffer = repo_get_commit_buffer(r, this, &bufsize);
+	orig_author = find_commit_header(buffer, "author", &author_len);
+	orig_committer = find_commit_header(buffer, "committer", &committer_len);
+
+	if (!orig_author || !orig_committer) {
+		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	if (split_ident_line(&id, orig_author, author_len) < 0 ||
+	    split_ident_line(&id, orig_committer, committer_len) < 0) {
+		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	p = strstr(buffer, "\n\n");
+	strbuf_addstr(&msg, "git stash: ");
+
+	if (p)
+		strbuf_add(&msg, p + 2, bufsize - (p + 2 - buffer));
+	strbuf_complete_line(&msg);
+
+	author = xmemdupz(orig_author, author_len);
+	committer = xmemdupz(orig_committer, committer_len);
+
+	if (commit_tree_extended(msg.buf, msg.len,
+				 r->hash_algo->empty_tree, parents,
+				 out, author, committer,
+				 NULL, NULL)) {
+		ret = error(_("could not write commit"));
+		goto out;
+	}
+out:
+	strbuf_release(&msg);
+	repo_unuse_commit_buffer(r, this, buffer);
+	free_commit_list(parents);
+	free(author);
+	free(committer);
+	return ret;
+}
+
+struct stash_entry_data {
+	struct repository *r;
+	struct commit_list **items;
+	size_t count;
+};
+
+static int collect_stash_entries(struct object_id *old_oid UNUSED,
+				 struct object_id *new_oid,
+				 const char *committer UNUSED,
+				 timestamp_t timestamp UNUSED,
+				 int tz UNUSED, const char *msg UNUSED,
+				 void *cb_data)
+{
+	struct stash_entry_data *data = cb_data;
+	struct commit *stash;
+
+	data->count++;
+	stash = lookup_commit_reference(data->r, new_oid);
+	if (!stash || check_stash_topology(data->r, stash)) {
+		return error(_("%s does not look like a stash commit"),
+			     oid_to_hex(new_oid));
+	}
+	data->items = commit_list_append(stash, data->items);
+	return 0;
+}
+
+static int do_export_stash(struct repository *r,
+			   const char *ref,
+			   int argc,
+			   const char **argv)
+{
+	struct object_id base;
+	struct object_context unused;
+	struct commit *prev;
+	struct commit_list *items = NULL, **iter = &items, *cur;
+	int res = 0;
+	int i;
+	struct strbuf revision = STRBUF_INIT;
+	const char *author, *committer;
+
+	/*
+	 * This is an arbitrary, fixed date, specifically the one used by git
+	 * format-patch.  The goal is merely to produce reproducible output.
+	 */
+	prepare_fallback_ident("git stash", "git@stash");
+	author = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			   "2001-09-17T00:00:00Z", 0);
+	committer = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			      "2001-09-17T00:00:00Z", 0);
+
+	/* First, we create a single empty commit. */
+	if (commit_tree_extended("", 0, r->hash_algo->empty_tree, NULL,
+				 &base, author, committer, NULL, NULL))
+		return error(_("unable to write base commit"));
+
+	prev = lookup_commit_reference(r, &base);
+
+	if (argc) {
+		/*
+		 * Find each specified stash, and load data into the array.
+		 */
+		for (i = 0; i < argc; i++) {
+			struct object_id oid;
+			struct commit *stash;
+
+			if (parse_stash_revision(&revision, argv[i], 1) ||
+			    get_oid_with_context(r, revision.buf,
+						 GET_OID_QUIETLY | GET_OID_GENTLY,
+						 &oid, &unused)) {
+				res = error(_("unable to find stash entry %s"), argv[i]);
+				goto out;
+			}
+
+			stash = lookup_commit_reference(r, &oid);
+			if (!stash || check_stash_topology(r, stash)) {
+				res = error(_("%s does not look like a stash commit"),
+						revision.buf);
+				goto out;
+			}
+			iter = commit_list_append(stash, iter);
+		}
+	} else {
+		/*
+		 * Walk the reflog, finding each stash entry, and load data into the
+		 * array.
+		 */
+		struct stash_entry_data cb_data = {
+			.r = r, .items = iter,
+		};
+		if (refs_for_each_reflog_ent_reverse(get_main_ref_store(r),
+						     "refs/stash",
+						     collect_stash_entries,
+						     &cb_data) && cb_data.count)
+			goto out;
+	}
+
+	/*
+	 * Now, create a set of commits identical to the regular stash commits,
+	 * but where their first parents form a chain to our original empty
+	 * base commit.
+	 */
+	items = reverse_commit_list(items);
+	for (cur = items; cur; cur = cur->next) {
+		struct commit_list *parents = NULL;
+		struct commit_list **next = &parents;
+		struct object_id out;
+		struct commit *stash = cur->item;
+
+		next = commit_list_append(prev, next);
+		next = commit_list_append(stash, next);
+		res = write_commit_with_parents(r, &out, &stash->object.oid, parents);
+		if (res)
+			goto out;
+		prev = lookup_commit_reference(r, &out);
+	}
+	if (ref)
+		refs_update_ref(get_main_ref_store(r), NULL, ref,
+				&prev->object.oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	else
+		puts(oid_to_hex(&prev->object.oid));
+out:
+	strbuf_release(&revision);
+	free_commit_list(items);
+
+	return res;
+}
+
+enum export_action {
+	ACTION_NONE,
+	ACTION_PRINT,
+	ACTION_TO_REF,
+};
+
+static int export_stash(int argc,
+			const char **argv,
+			const char *prefix,
+			struct repository *repo)
+{
+	const char *ref = NULL;
+	enum export_action action = ACTION_NONE;
+	struct option options[] = {
+		OPT_CMDMODE(0, "print", &action,
+			    N_("print the object ID instead of writing it to a ref"),
+			    ACTION_PRINT),
+		OPT_STRING(0, "to-ref", &ref, "ref",
+			    N_("save the data to the given ref")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_export_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (ref && action == ACTION_NONE)
+		action = ACTION_TO_REF;
+
+	if (action == ACTION_NONE)
+		return error(_("exactly one of --print and --to-ref is required"));
+
+	return do_export_stash(repo, ref, argc, argv);
+}
+
 int cmd_stash(int argc,
 	      const char **argv,
 	      const char *prefix,
@@ -1915,6 +2174,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("store", &fn, store_stash),
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
+		OPT_SUBCOMMAND("export", &fn, export_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};

[PATCH v7 4/4] builtin/stash: provide a way to import stashes from a ref

[brian m. carlson]

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.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-stash.adoc |   7 ++
 builtin/stash.c              | 167 +++++++++++++++++++++++++++++++++++
 t/t3903-stash.sh             |  94 ++++++++++++++++++++
 3 files changed, 268 insertions(+)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index e8efd43ba4..f8193b712e 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -24,6 +24,7 @@ SYNOPSIS
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
 'git stash' export (--print | --to-ref <ref>) [<stash>...]
+'git stash' import <commit>
 
 DESCRIPTION
 -----------
@@ -161,6 +162,12 @@ export ( --print | --to-ref <ref> ) [<stash>...]::
 	a chain of commits which can be transferred using the normal fetch and
 	push mechanisms, then imported using the `import` subcommand.
 
+import <commit>::
+
+	Import the specified stashes from the specified commit, which must have been
+	created by `export`, and add them to the list of stashes.  To replace the
+	existing stashes, use `clear` first.
+
 OPTIONS
 -------
 -a::
diff --git a/builtin/stash.c b/builtin/stash.c
index bebe004e4d..2c5715eba9 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -61,6 +61,8 @@
 	N_("git stash create [<message>]")
 #define BUILTIN_STASH_EXPORT_USAGE \
 	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
+#define BUILTIN_STASH_IMPORT_USAGE \
+	N_("git stash import <commit>")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -77,6 +79,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
 	BUILTIN_STASH_EXPORT_USAGE,
+	BUILTIN_STASH_IMPORT_USAGE,
 	NULL
 };
 
@@ -135,6 +138,10 @@ static const char * const git_stash_export_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_import_usage[] = {
+	BUILTIN_STASH_IMPORT_USAGE,
+	NULL
+};
 
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
@@ -144,6 +151,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
@@ -154,6 +162,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;
@@ -1992,6 +2001,163 @@ static int write_commit_with_parents(struct repository *r,
 	return ret;
 }
 
+static int do_import_stash(struct repository *r, const char *rev)
+{
+	struct object_id chain;
+	int res = 0;
+	const char *buffer = NULL;
+	unsigned long bufsize;
+	struct commit *this = NULL;
+	struct commit_list *items = NULL, *cur;
+	char *msg = NULL;
+
+	if (repo_get_oid(r, rev, &chain))
+		return error(_("not a valid revision: %s"), rev);
+
+	this = lookup_commit_reference(r, &chain);
+	if (!this)
+		return error(_("not a commit: %s"), rev);
+
+	/*
+	 * Walk the commit history, finding each stash entry, and load data into
+	 * the array.
+	 */
+	for (;;) {
+		const char *author, *committer;
+		size_t author_len, committer_len;
+		const char *p;
+		const char *expected = "git stash <git@stash> 1000684800 +0000";
+		const char *prefix = "git stash: ";
+		struct commit *stash;
+		struct tree *tree = repo_get_commit_tree(r, this);
+
+		if (!tree ||
+		    !oideq(&tree->object.oid, r->hash_algo->empty_tree) ||
+		    (this->parents &&
+		     (!this->parents->next || this->parents->next->next))) {
+			res = error(_("%s is not a valid exported stash commit"),
+					oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+
+		if (!this->parents) {
+			/*
+			 * We don't have any parents.  Make sure this is our
+			 * root commit.
+			 */
+			author = find_commit_header(buffer, "author", &author_len);
+			committer = find_commit_header(buffer, "committer", &committer_len);
+
+			if (!author || !committer) {
+				error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
+				goto out;
+			}
+
+			if (author_len != strlen(expected) ||
+			    committer_len != strlen(expected) ||
+			    memcmp(author, expected, author_len) ||
+			    memcmp(committer, expected, committer_len)) {
+				res = error(_("found root commit %s with invalid data"), oid_to_hex(&this->object.oid));
+				goto out;
+			}
+			break;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		p += 2;
+		if (((size_t)(bufsize - (p - buffer)) < strlen(prefix)) ||
+		    memcmp(prefix, p, strlen(prefix))) {
+			res = error(_("found stash commit %s without expected prefix"), oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		stash = this->parents->next->item;
+
+		if (repo_parse_commit(r, this->parents->item) ||
+		    repo_parse_commit(r, stash)) {
+			res = error(_("cannot parse parents of commit: %s"),
+					oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		if (check_stash_topology(r, stash)) {
+			res = error(_("%s does not look like a stash commit"),
+					oid_to_hex(&stash->object.oid));
+			goto out;
+		}
+
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+		items = commit_list_insert(stash, &items);
+		this = this->parents->item;
+	}
+
+	/*
+	 * Now, walk each entry, adding it to the stash as a normal stash
+	 * commit.
+	 */
+	for (cur = items; cur; cur = cur->next) {
+		const char *p;
+		struct object_id *oid;
+
+		this = cur->item;
+		oid = &this->object.oid;
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+		if (!buffer) {
+			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p += 2;
+		msg = xmemdupz(p, bufsize - (p - buffer));
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+
+		if (do_store_stash(oid, msg, 1)) {
+			res = error(_("cannot save the stash for %s"), oid_to_hex(oid));
+			goto out;
+		}
+		FREE_AND_NULL(msg);
+	}
+out:
+	if (this && buffer)
+		repo_unuse_commit_buffer(r, this, buffer);
+	free_commit_list(items);
+	free(msg);
+
+	return res;
+}
+
+static int import_stash(int argc, const char **argv, const char *prefix,
+			struct repository *repo)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_import_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc != 1)
+		usage_msg_opt("a revision is required", git_stash_import_usage, options);
+
+	return do_import_stash(repo, argv[0]);
+}
+
 struct stash_entry_data {
 	struct repository *r;
 	struct commit_list **items;
@@ -2175,6 +2341,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
 		OPT_SUBCOMMAND("export", &fn, export_stash),
+		OPT_SUBCOMMAND("import", &fn, import_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 74666ff3e4..ca3dd45664 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -11,6 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-unique-files.sh
 
+test_expect_success 'setup' '
+	test_oid_cache <<-EOF
+	export_base sha1:73c9bab443d1f88ac61aa533d2eeaaa15451239c
+	export_base sha256:f210fa6346e3e2ce047bdb570426b17075980c1ac01fec8fc4b75bd3ab4bcfe4
+	EOF
+'
+
 test_expect_success 'usage on cmd and subcommand invalid option' '
 	test_expect_code 129 git stash --invalid-option 2>usage &&
 	grep "or: git stash" usage &&
@@ -1412,6 +1419,93 @@ test_expect_success 'stash --keep-index --include-untracked with empty tree' '
 	)
 '
 
+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 --include-untracked -- subdir/ &&
+	git tag t-stash0 stash@{0} &&
+	git tag t-stash1 stash@{1} &&
+	simple=$(git stash export --print) &&
+	git stash clear &&
+	git stash import "$simple" &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1 &&
+	git stash export --to-ref refs/heads/foo &&
+	test_cmp_rev "$(test_oid empty_tree)" foo: &&
+	test_cmp_rev "$(test_oid empty_tree)" foo^: &&
+	test_cmp_rev t-stash0 foo^2 &&
+	test_cmp_rev t-stash1 foo^^2 &&
+	git log --first-parent --format="%s" refs/heads/foo >log &&
+	grep "^git stash: " log >log2 &&
+	test_line_count = 13 log2 &&
+	git stash clear &&
+	git stash import foo &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1
+'
+
+test_expect_success 'stash import appends commits' '
+	git log --format=oneline -g refs/stash >out &&
+	cat out out >out2 &&
+	git stash import refs/heads/foo &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = $(wc -l <out2) actual
+'
+
+test_expect_success 'stash export can accept specified stashes' '
+	git stash clear &&
+	git stash import foo &&
+	git stash export --to-ref refs/heads/bar stash@{1} stash@{0} &&
+	git stash clear &&
+	git stash import refs/heads/bar &&
+	test_cmp_rev stash@{1} t-stash0 &&
+	test_cmp_rev stash@{0} t-stash1 &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = 2 actual
+'
+
+test_expect_success 'stash can import and export zero stashes' '
+	git stash clear &&
+	git stash export --to-ref refs/heads/baz &&
+	test_cmp_rev "$(test_oid empty_tree)" baz: &&
+	test_cmp_rev "$(test_oid export_base)" baz &&
+	test_must_fail git rev-parse baz^1 &&
+	git stash import baz &&
+	test_must_fail git rev-parse refs/stash
+'
+
+test_expect_success 'stash rejects invalid attempts to import commits' '
+	git stash import foo &&
+	test_must_fail git stash import HEAD 2>output &&
+	oid=$(git rev-parse HEAD) &&
+	grep "$oid is not a valid exported stash commit" output &&
+	test_cmp_rev stash@{0} t-stash0 &&
+
+	git checkout --orphan orphan &&
+	git commit-tree $(test_oid empty_tree) -p "$oid" -p "$oid^" -m "" >fake-commit &&
+	git update-ref refs/heads/orphan "$(cat fake-commit)" &&
+	oid=$(git rev-parse HEAD) &&
+	test_must_fail git stash import orphan 2>output &&
+	grep "found stash commit $oid without expected prefix" output &&
+	test_cmp_rev stash@{0} t-stash0 &&
+
+	git checkout --orphan orphan2 &&
+	git commit-tree $(test_oid empty_tree) -m "" >fake-commit &&
+	git update-ref refs/heads/orphan2 "$(cat fake-commit)" &&
+	oid=$(git rev-parse HEAD) &&
+	test_must_fail git stash import orphan2 2>output &&
+	grep "found root commit $oid with invalid data" output &&
+	test_cmp_rev stash@{0} t-stash0
+'
+
 test_expect_success 'stash apply should succeed with unmodified file' '
 	echo base >file &&
 	git add file &&

Re: [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref

[Phillip Wood]

Hi brian

There are a couple of points outstanding from my last review. The first 
point below about the option handling definitely needs fixing, the other 
is more of a matter of personal taste.

On 01/06/2025 23:32, brian m. carlson wrote:
> 
> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
> index 1a5177f498..e8efd43ba4 100644
> --- a/Documentation/git-stash.adoc
> +++ b/Documentation/git-stash.adoc
> @@ -23,6 +23,7 @@ SYNOPSIS
>   'git stash' clear
>   'git stash' create [<message>]
>   'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
> +'git stash' export (--print | --to-ref <ref>) [<stash>...]
>   
>   DESCRIPTION
>   -----------
> @@ -154,6 +155,12 @@ store::
>   	reflog.  This is intended to be useful for scripts.  It is
>   	probably not the command you want to use; see "push" above.
>   
> +export ( --print | --to-ref <ref> ) [<stash>...]::

--print and --to-ref are documented as mutually exclusive but that is 
not implemented in the code.

     git stash export --print --to-ref refs/exported-stash

creates refs/exported-stash but does not print the object id. We should 
either ensure they are actually mutually exclusive or just allow both. I 
don't have a strong preference either way - I'm not sure it is 
particularly useful for the user to be able to specify both but it 
doesn't do any harm so long as we honor both options.

>   
> +static int write_commit_with_parents(struct repository *r,
> +				     struct object_id *out,
> +				     const struct object_id *oid,
> +				     struct commit_list *parents)
> +{
> [...]
> +out:
> +	strbuf_release(&msg);
> +	repo_unuse_commit_buffer(r, this, buffer);
> +	free_commit_list(parents);

I think taking ownership of function parameters like this makes the code 
harder to reason about because C has no way for the function prototype 
to signal to the reader that the ownership is transferred. It would be 
easier to see that the list of parent commits was not leaked if it was 
freed by the caller.

Best Wishes

Phillip

Re: [PATCH v7 0/4] Importing and exporting stashes to refs

[Phillip Wood]

Hi brian

I've left a couple of comments on patch 3 but everything else looks fine 
to me

Thanks

Phillip

On 01/06/2025 23:32, brian m. carlson wrote:
> Stashes are currently stored using the reflog in a given repository.
> This is an interesting and novel way to handle them, but there is no way
> to easily move a set of stashes across machines.  For example, groups of
> stashes cannot be bundled, pushed, or fetched.
> 
> Let's solve this problem by allowing users to import and export stashes
> to a chain of commits.  The commits used in a stash export contain two
> parents: one which is the pointer to the next exported stash (or to an
> empty commit with no parents if there are no more) and the second is the
> stash commit that would normally be stored in the reflog.
> 
> Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>
> 
> Changes from v6:
> * Add Phillip's sign-off to the last patch.
> * Use `commit_list` for tracking commits.
> * Use reflog entry walker.
> * Fix some commit messages for improved legibility.
> * Rephrase some error messages for precision.
> * Drop the patch that exposes `read_complete_reflog` since it is no longer necessary.
> 
> Changes from v5:
> * Rename `parse_revision`.
> * Remove extra call to `free_stash_info`.
> * Fix parsing of existing commit.
> * Add more validation of imported stash commits.
> * Add more tests for improved validation of imported stash commits.
> * Explicitly cast `items.nr` and make the iteration counter an `ssize_t`
>    to avoid casting problems.
> * Don't require a trailing `\n\n` in commit messages.
> * Use `read_complete_reflog` to walk reflogs.
> * Be more defensive when using `lookup_commit_reference`.
> * Apply parts of Phillip's patches for improved robustness.
> * Update commit message to explain additional use cases.
> * Use `OPT_STRING` for `--to-ref`.
> 
> Changes from v4:
> * Fix another use of oid_array.
> * Fix various memory leaks.
> * Fix a segfault which appeared after a rebase.
> * Use strstr for commits since we don't need to worry about NUL.
> * Added some additional tests.
> * Verify the ident values we're using to avoid using bad values.
> * Various other code cleanups.
> * Rebase on `master`.
> 
> Changes from v3:
> * Fix strbuf handling to avoid leaks and generally be more sensible.
> * Make use of the error return code more often.
> * Use oid_array.
> * Tidy various parts of the code and fix long lines.
> * Simplify tests using git tag.
> * Shorten and tidy tests.
> * Add an additional test covering the base commit OID and importing and
>    exporting empty stashes.
> 
> Changes from v2:
> * Fix uninitialized strbuf.
> * Avoid C99-style initializations.
> 
> Changes from v1:
> * Change storage format as suggested by Junio.
> * Rename to GIT_OID_GENTLY.
> * Remove unnecessary initializations.
> * Use ALLOC_GROW_BY.
> * Ensure completely reproducible exports.
> * Avoid size_t.
> * Various other code cleanups.
> 
> brian m. carlson (4):
>    object-name: make get_oid quietly return an error
>    builtin/stash: factor out revision parsing into a function
>    builtin/stash: provide a way to export stashes to a ref
>    builtin/stash: provide a way to import stashes from a ref
> 
>   Documentation/git-stash.adoc |  29 ++-
>   builtin/stash.c              | 460 ++++++++++++++++++++++++++++++++++-
>   hash.h                       |   1 +
>   object-name.c                |   6 +-
>   t/t3903-stash.sh             |  94 +++++++
>   5 files changed, 577 insertions(+), 13 deletions(-)
> 

Re: [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref

[Kristoffer Haugsbakk]

I only bring up this minor point since I read that there will be a next
version anyway.

On Mon, Jun 2, 2025, at 00:32, brian m. carlson wrote:
> +--print::
> +	This option is only valid for `export`.
> ++
> +Create the chain of commits representing the exported stashes without
> +storing it anywhere in the ref namespace and print the object ID to
> +standard output.  This is designed for scripts.
> +
> +--to-ref::
> +	This option is only valid for `export`.
> ++

The existing options say “for [the] `<cmd>` command”.  So that’s a minor
deviation from the convention.

-- 
Kristoffer Haugsbakk

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[Kristoffer Haugsbakk]

On Fri, May 9, 2025, at 01:44, brian m. carlson wrote:
> Stashes are currently stored using the reflog in a given repository.
> This is an interesting and novel way to handle them, but there is no way
> to easily move a stash across machines.  For example, stashes cannot be
> bundled, pushed, or fetched.

I think this feature works well.  Exporting locally is simple.  Using a
whatever-ref to export to the remote is a fine implementation.
Importing locally is simple as well.  The `--print` option for
script/machine consumption is a nice touch.

Also I think the DAG implementation is cool.

-- 
Kristoffer Haugsbakk

Re: [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On 2025-06-11 at 11:31:19, Kristoffer Haugsbakk wrote:
> I only bring up this minor point since I read that there will be a next
> version anyway.

I was just about to send it out, so I'm glad I saw this before doing so.

> On Mon, Jun 2, 2025, at 00:32, brian m. carlson wrote:
> > +--print::
> > +	This option is only valid for `export`.
> > ++
> > +Create the chain of commits representing the exported stashes without
> > +storing it anywhere in the ref namespace and print the object ID to
> > +standard output.  This is designed for scripts.
> > +
> > +--to-ref::
> > +	This option is only valid for `export`.
> > ++
> 
> The existing options say “for [the] `<cmd>` command”.  So that’s a minor
> deviation from the convention.

I can change that here.  I'll use "for the `<cmd>` command`" because I
think it reads better with the article than without, even though that's
a little less consistent.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH v5 0/4] Importing and exporting stashes to refs

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

On 2025-06-11 at 11:35:12, Kristoffer Haugsbakk wrote:
> I think this feature works well.  Exporting locally is simple.  Using a
> whatever-ref to export to the remote is a fine implementation.
> Importing locally is simple as well.  The `--print` option for
> script/machine consumption is a nice touch.
> 
> Also I think the DAG implementation is cool.

Thank you, I appreciate that feedback.  (I would also have appreciated
feedback about how it could be better, but I do like that you're happy
with it.)
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[PATCH v8 0/4] Importing and exporting stashes to refs

[brian m. carlson]

Stashes are currently stored using the reflog in a given repository.
This is an interesting and novel way to handle them, but there is no way
to easily move a set of stashes across machines.  For example, groups of
stashes cannot be bundled, pushed, or fetched.

Let's solve this problem by allowing users to import and export stashes
to a chain of commits.  The commits used in a stash export contain two
parents: one which is the pointer to the next exported stash (or to an
empty commit with no parents if there are no more) and the second is the
stash commit that would normally be stored in the reflog.

Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>

Changes from v7:
* Rephrase the documentation to be slightly more explicit.
* Don't have `write_commit_with_parents` free its arguments, instead
  letting the caller (who allocated them) handle them.
* Handle invalid combinations of arguments to `export` and add tests for
  this case.

Changes from v6:
* Add Phillip's sign-off to the last patch.
* Use `commit_list` for tracking commits.
* Use reflog entry walker.
* Fix some commit messages for improved legibility.
* Rephrase some error messages for precision.
* Drop the patch that exposes `read_complete_reflog` since it is no longer necessary.

Changes from v5:
* Rename `parse_revision`.
* Remove extra call to `free_stash_info`.
* Fix parsing of existing commit.
* Add more validation of imported stash commits.
* Add more tests for improved validation of imported stash commits.
* Explicitly cast `items.nr` and make the iteration counter an `ssize_t`
  to avoid casting problems.
* Don't require a trailing `\n\n` in commit messages.
* Use `read_complete_reflog` to walk reflogs.
* Be more defensive when using `lookup_commit_reference`.
* Apply parts of Phillip's patches for improved robustness.
* Update commit message to explain additional use cases.
* Use `OPT_STRING` for `--to-ref`.

Changes from v4:
* Fix another use of oid_array.
* Fix various memory leaks.
* Fix a segfault which appeared after a rebase.
* Use strstr for commits since we don't need to worry about NUL.
* Added some additional tests.
* Verify the ident values we're using to avoid using bad values.
* Various other code cleanups.
* Rebase on `master`.

Changes from v3:
* Fix strbuf handling to avoid leaks and generally be more sensible.
* Make use of the error return code more often.
* Use oid_array.
* Tidy various parts of the code and fix long lines.
* Simplify tests using git tag.
* Shorten and tidy tests.
* Add an additional test covering the base commit OID and importing and
  exporting empty stashes.

Changes from v2:
* Fix uninitialized strbuf.
* Avoid C99-style initializations.

Changes from v1:
* Change storage format as suggested by Junio.
* Rename to GIT_OID_GENTLY.
* Remove unnecessary initializations.
* Use ALLOC_GROW_BY.
* Ensure completely reproducible exports.
* Avoid size_t.
* Various other code cleanups.

brian m. carlson (4):
  object-name: make get_oid quietly return an error
  builtin/stash: factor out revision parsing into a function
  builtin/stash: provide a way to export stashes to a ref
  builtin/stash: provide a way to import stashes from a ref

 Documentation/git-stash.adoc |  29 ++-
 builtin/stash.c              | 460 ++++++++++++++++++++++++++++++++++-
 hash.h                       |   1 +
 object-name.c                |   6 +-
 t/t3903-stash.sh             | 101 ++++++++
 5 files changed, 584 insertions(+), 13 deletions(-)

[PATCH v8 1/4] object-name: make get_oid quietly return an error

[brian m. carlson]

A reasonable person looking at the signature and usage of get_oid and
friends might conclude that in the event of an error, it always returns
-1.  However, this is not the case.  Instead, get_oid_basic dies if we
go too far back into the history of a reflog (or, when quiet, simply
exits).

This is not especially useful, since in many cases, we might want to
handle this error differently.  Let's add a flag here to make it just
return -1 like elsewhere in these code paths.

Note that we cannot make this behavior the default, since we have many
other codepaths that rely on the existing behavior, including in tests.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 hash.h        | 1 +
 object-name.c | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hash.h b/hash.h
index d6422ddf45..ec594c63a6 100644
--- a/hash.h
+++ b/hash.h
@@ -216,6 +216,7 @@ struct object_id {
 #define GET_OID_REQUIRE_PATH         010000
 #define GET_OID_HASH_ANY             020000
 #define GET_OID_SKIP_AMBIGUITY_CHECK 040000
+#define GET_OID_GENTLY              0100000
 
 #define GET_OID_DISAMBIGUATORS \
 	(GET_OID_COMMIT | GET_OID_COMMITTISH | \
diff --git a/object-name.c b/object-name.c
index 9288b2dd24..851858975f 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 				 * still fill in the oid with the "old" value,
 				 * which we can use.
 				 */
-			} else {
+			} else if (!(flags & GET_OID_GENTLY)) {
 				if (flags & GET_OID_QUIETLY) {
 					exit(128);
 				}
 				die(_("log for '%.*s' only has %d entries"),
 				    len, str, co_cnt);
 			}
+			if (flags & GET_OID_GENTLY) {
+				free(real_ref);
+				return -1;
+			}
 		}
 	}
 

[PATCH v8 2/4] builtin/stash: factor out revision parsing into a function

[brian m. carlson]

We allow several special forms of stash names in this code.  In the
future, we'll want to allow these same forms without parsing a stash
commit, so let's refactor this code out into a function for reuse.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/stash.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index cfbd92852a..ab491d5ff6 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -169,6 +169,25 @@ static void assert_stash_like(struct stash_info *info, const char *revision)
 		die(_("'%s' is not a stash-like commit"), revision);
 }
 
+static int parse_stash_revision(struct strbuf *revision, const char *commit, int quiet)
+{
+	strbuf_reset(revision);
+	if (!commit) {
+		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
+			if (!quiet)
+				fprintf_ln(stderr, _("No stash entries found."));
+			return -1;
+		}
+
+		strbuf_addf(revision, "%s@{0}", ref_stash);
+	} else if (strspn(commit, "0123456789") == strlen(commit)) {
+		strbuf_addf(revision, "%s@{%s}", ref_stash, commit);
+	} else {
+		strbuf_addstr(revision, commit);
+	}
+	return 0;
+}
+
 static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 {
 	int ret;
@@ -196,17 +215,9 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 	if (argc == 1)
 		commit = argv[0];
 
-	if (!commit) {
-		if (!refs_ref_exists(get_main_ref_store(the_repository), ref_stash)) {
-			fprintf_ln(stderr, _("No stash entries found."));
-			return -1;
-		}
-
-		strbuf_addf(&info->revision, "%s@{0}", ref_stash);
-	} else if (strspn(commit, "0123456789") == strlen(commit)) {
-		strbuf_addf(&info->revision, "%s@{%s}", ref_stash, commit);
-	} else {
-		strbuf_addstr(&info->revision, commit);
+	strbuf_init(&info->revision, 0);
+	if (parse_stash_revision(&info->revision, commit, 0)) {
+		return -1;
 	}
 
 	revision = info->revision.buf;

[PATCH v8 3/4] builtin/stash: provide a way to export stashes to a ref

[brian m. carlson]

A common user problem is how to sync in-progress work to another
machine.  Users currently must use some sort of transfer of the working
tree, which poses security risks and also necessarily causes the index
to become dirty.  The experience is suboptimal and frustrating for
users.

A reasonable idea is to use the stash for this purpose, but the stash is
stored in the reflog, not in a ref, and as such it cannot be pushed or
pulled.  This also means that it cannot be saved into a bundle or
preserved elsewhere, which is a problem when using throwaway development
environments.

In addition, users often want to replicate stashes across machines, such
as when they must use multiple machines or when they use throwaway dev
environments, such as those based on the Devcontainer spec, where they
might otherwise lose various in-progress work.

Let's solve this problem by allowing the user to export the stash to a
ref (or, to just write it into the repository and print the hash, à la
git commit-tree).  Introduce git stash export, which writes a chain of
commits where the first parent is always a chain to the previous stash,
or to a single, empty commit (for the final item) and the second is the
stash commit normally written to the reflog.

Iterate over each stash from top to bottom, looking up the data for each
one, and then create the chain from the single empty commit back up in
reverse order.  Generate a predictable empty commit so our behavior is
reproducible.  Create a useful commit message, preserving the author and
committer information, to help users identify stash commits when viewing
them as normal commits.

If the user has specified specific stashes they'd like to export
instead, use those instead of iterating over all of the stashes.

As part of this, specifically request quiet behavior when looking up the
OID for a revision because we will eventually hit a revision that
doesn't exist and we don't want to die when that occurs.

When exporting stashes, be sure to verify that they look like valid
stashes and don't contain invalid data.  This will help avoid failures
on import or problems due to attempting to export invalid refs that are
not stashes.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-stash.adoc |  22 ++-
 builtin/stash.c              | 260 +++++++++++++++++++++++++++++++++++
 2 files changed, 281 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 1a5177f498..0aef0a5b86 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -23,6 +23,7 @@ SYNOPSIS
 'git stash' clear
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
+'git stash' export (--print | --to-ref <ref>) [<stash>...]
 
 DESCRIPTION
 -----------
@@ -154,6 +155,12 @@ store::
 	reflog.  This is intended to be useful for scripts.  It is
 	probably not the command you want to use; see "push" above.
 
+export ( --print | --to-ref <ref> ) [<stash>...]::
+
+	Export the specified stashes, or all of them if none are specified, to
+	a chain of commits which can be transferred using the normal fetch and
+	push mechanisms, then imported using the `import` subcommand.
+
 OPTIONS
 -------
 -a::
@@ -242,6 +249,19 @@ literally (including newlines and quotes).
 +
 Quiet, suppress feedback messages.
 
+--print::
+	This option is only valid for the `export` command.
++
+Create the chain of commits representing the exported stashes without
+storing it anywhere in the ref namespace and print the object ID to
+standard output.  This is designed for scripts.
+
+--to-ref::
+	This option is only valid for the `export` command.
++
+Create the chain of commits representing the exported stashes and store
+it to the specified ref.
+
 \--::
 	This option is only valid for `push` command.
 +
@@ -259,7 +279,7 @@ For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 <stash>::
 	This option is only valid for `apply`, `branch`, `drop`, `pop`,
-	`show` commands.
+	`show`, and `export` commands.
 +
 A reference of the form `stash@{<revision>}`. When no `<stash>` is
 given, the latest stash is assumed (that is, `stash@{0}`).
diff --git a/builtin/stash.c b/builtin/stash.c
index ab491d5ff6..192b0d9969 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -28,7 +28,10 @@
 #include "log-tree.h"
 #include "diffcore.h"
 #include "reflog.h"
+#include "reflog-walk.h"
 #include "add-interactive.h"
+#include "oid-array.h"
+#include "commit.h"
 
 #define INCLUDE_ALL_FILES 2
 
@@ -56,6 +59,8 @@
 	   "          [-u | --include-untracked] [-a | --all] [<message>]")
 #define BUILTIN_STASH_CREATE_USAGE \
 	N_("git stash create [<message>]")
+#define BUILTIN_STASH_EXPORT_USAGE \
+	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -71,6 +76,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CLEAR_USAGE,
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
+	BUILTIN_STASH_EXPORT_USAGE,
 	NULL
 };
 
@@ -124,6 +130,12 @@ static const char * const git_stash_save_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_export_usage[] = {
+	BUILTIN_STASH_EXPORT_USAGE,
+	NULL
+};
+
+
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
 
@@ -160,6 +172,33 @@ static void free_stash_info(struct stash_info *info)
 	strbuf_release(&info->revision);
 }
 
+static int check_stash_topology(struct repository *r, struct commit *stash)
+{
+	struct commit *p1, *p2, *p3 = NULL;
+
+	/* stash must have two or three parents */
+	if (!stash->parents || !stash->parents->next ||
+			(stash->parents->next->next && stash->parents->next->next->next))
+		return -1;
+	p1 = stash->parents->item;
+	p2 = stash->parents->next->item;
+	if (stash->parents->next->next)
+		p3 = stash->parents->next->next->item;
+	if (repo_parse_commit(r, p1) || repo_parse_commit(r, p2) ||
+			(p3 && repo_parse_commit(r, p3)))
+		return -1;
+	/* p2 must have a single parent, p3 must have no parents */
+	if (!p2->parents || p2->parents->next || (p3 && p3->parents))
+		return -1;
+	if (repo_parse_commit(r, p2->parents->item))
+		return -1;
+	/* p2^1 must equal p1 */
+	if (!oideq(&p1->object.oid, &p2->parents->item->object.oid))
+		return -1;
+
+	return 0;
+}
+
 static void assert_stash_like(struct stash_info *info, const char *revision)
 {
 	if (get_oidf(&info->b_commit, "%s^1", revision) ||
@@ -1895,6 +1934,226 @@ static int save_stash(int argc, const char **argv, const char *prefix,
 	return ret;
 }
 
+static int write_commit_with_parents(struct repository *r,
+				     struct object_id *out,
+				     const struct object_id *oid,
+				     struct commit_list *parents)
+{
+	size_t author_len, committer_len;
+	struct commit *this;
+	const char *orig_author, *orig_committer;
+	char *author = NULL, *committer = NULL;
+	const char *buffer;
+	unsigned long bufsize;
+	const char *p;
+	struct strbuf msg = STRBUF_INIT;
+	int ret = 0;
+	struct ident_split id;
+
+	this = lookup_commit_reference(r, oid);
+	buffer = repo_get_commit_buffer(r, this, &bufsize);
+	orig_author = find_commit_header(buffer, "author", &author_len);
+	orig_committer = find_commit_header(buffer, "committer", &committer_len);
+
+	if (!orig_author || !orig_committer) {
+		ret = error(_("cannot parse commit %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	if (split_ident_line(&id, orig_author, author_len) < 0 ||
+	    split_ident_line(&id, orig_committer, committer_len) < 0) {
+		ret = error(_("invalid author or committer for %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	p = strstr(buffer, "\n\n");
+	strbuf_addstr(&msg, "git stash: ");
+
+	if (p)
+		strbuf_add(&msg, p + 2, bufsize - (p + 2 - buffer));
+	strbuf_complete_line(&msg);
+
+	author = xmemdupz(orig_author, author_len);
+	committer = xmemdupz(orig_committer, committer_len);
+
+	if (commit_tree_extended(msg.buf, msg.len,
+				 r->hash_algo->empty_tree, parents,
+				 out, author, committer,
+				 NULL, NULL)) {
+		ret = error(_("could not write commit"));
+		goto out;
+	}
+out:
+	strbuf_release(&msg);
+	repo_unuse_commit_buffer(r, this, buffer);
+	free(author);
+	free(committer);
+	return ret;
+}
+
+struct stash_entry_data {
+	struct repository *r;
+	struct commit_list **items;
+	size_t count;
+};
+
+static int collect_stash_entries(struct object_id *old_oid UNUSED,
+				 struct object_id *new_oid,
+				 const char *committer UNUSED,
+				 timestamp_t timestamp UNUSED,
+				 int tz UNUSED, const char *msg UNUSED,
+				 void *cb_data)
+{
+	struct stash_entry_data *data = cb_data;
+	struct commit *stash;
+
+	data->count++;
+	stash = lookup_commit_reference(data->r, new_oid);
+	if (!stash || check_stash_topology(data->r, stash)) {
+		return error(_("%s does not look like a stash commit"),
+			     oid_to_hex(new_oid));
+	}
+	data->items = commit_list_append(stash, data->items);
+	return 0;
+}
+
+static int do_export_stash(struct repository *r,
+			   const char *ref,
+			   int argc,
+			   const char **argv)
+{
+	struct object_id base;
+	struct object_context unused;
+	struct commit *prev;
+	struct commit_list *items = NULL, **iter = &items, *cur;
+	int res = 0;
+	int i;
+	struct strbuf revision = STRBUF_INIT;
+	const char *author, *committer;
+
+	/*
+	 * This is an arbitrary, fixed date, specifically the one used by git
+	 * format-patch.  The goal is merely to produce reproducible output.
+	 */
+	prepare_fallback_ident("git stash", "git@stash");
+	author = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			   "2001-09-17T00:00:00Z", 0);
+	committer = fmt_ident("git stash", "git@stash", WANT_BLANK_IDENT,
+			      "2001-09-17T00:00:00Z", 0);
+
+	/* First, we create a single empty commit. */
+	if (commit_tree_extended("", 0, r->hash_algo->empty_tree, NULL,
+				 &base, author, committer, NULL, NULL))
+		return error(_("unable to write base commit"));
+
+	prev = lookup_commit_reference(r, &base);
+
+	if (argc) {
+		/*
+		 * Find each specified stash, and load data into the array.
+		 */
+		for (i = 0; i < argc; i++) {
+			struct object_id oid;
+			struct commit *stash;
+
+			if (parse_stash_revision(&revision, argv[i], 1) ||
+			    get_oid_with_context(r, revision.buf,
+						 GET_OID_QUIETLY | GET_OID_GENTLY,
+						 &oid, &unused)) {
+				res = error(_("unable to find stash entry %s"), argv[i]);
+				goto out;
+			}
+
+			stash = lookup_commit_reference(r, &oid);
+			if (!stash || check_stash_topology(r, stash)) {
+				res = error(_("%s does not look like a stash commit"),
+						revision.buf);
+				goto out;
+			}
+			iter = commit_list_append(stash, iter);
+		}
+	} else {
+		/*
+		 * Walk the reflog, finding each stash entry, and load data into the
+		 * array.
+		 */
+		struct stash_entry_data cb_data = {
+			.r = r, .items = iter,
+		};
+		if (refs_for_each_reflog_ent_reverse(get_main_ref_store(r),
+						     "refs/stash",
+						     collect_stash_entries,
+						     &cb_data) && cb_data.count)
+			goto out;
+	}
+
+	/*
+	 * Now, create a set of commits identical to the regular stash commits,
+	 * but where their first parents form a chain to our original empty
+	 * base commit.
+	 */
+	items = reverse_commit_list(items);
+	for (cur = items; cur; cur = cur->next) {
+		struct commit_list *parents = NULL;
+		struct commit_list **next = &parents;
+		struct object_id out;
+		struct commit *stash = cur->item;
+
+		next = commit_list_append(prev, next);
+		next = commit_list_append(stash, next);
+		res = write_commit_with_parents(r, &out, &stash->object.oid, parents);
+		free_commit_list(parents);
+		if (res)
+			goto out;
+		prev = lookup_commit_reference(r, &out);
+	}
+	if (ref)
+		refs_update_ref(get_main_ref_store(r), NULL, ref,
+				&prev->object.oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	else
+		puts(oid_to_hex(&prev->object.oid));
+out:
+	strbuf_release(&revision);
+	free_commit_list(items);
+
+	return res;
+}
+
+enum export_action {
+	ACTION_NONE,
+	ACTION_PRINT,
+	ACTION_TO_REF,
+};
+
+static int export_stash(int argc,
+			const char **argv,
+			const char *prefix,
+			struct repository *repo)
+{
+	const char *ref = NULL;
+	enum export_action action = ACTION_NONE;
+	struct option options[] = {
+		OPT_CMDMODE(0, "print", &action,
+			    N_("print the object ID instead of writing it to a ref"),
+			    ACTION_PRINT),
+		OPT_STRING(0, "to-ref", &ref, "ref",
+			    N_("save the data to the given ref")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_export_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (ref && action == ACTION_NONE)
+		action = ACTION_TO_REF;
+
+	if (action == ACTION_NONE || (ref && action == ACTION_PRINT))
+		return error(_("exactly one of --print and --to-ref is required"));
+
+	return do_export_stash(repo, ref, argc, argv);
+}
+
 int cmd_stash(int argc,
 	      const char **argv,
 	      const char *prefix,
@@ -1915,6 +2174,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("store", &fn, store_stash),
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
+		OPT_SUBCOMMAND("export", &fn, export_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};

[PATCH v8 4/4] builtin/stash: provide a way to import stashes from a ref

[brian m. carlson]

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.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-stash.adoc |   7 ++
 builtin/stash.c              | 167 +++++++++++++++++++++++++++++++++++
 t/t3903-stash.sh             | 101 +++++++++++++++++++++
 3 files changed, 275 insertions(+)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 0aef0a5b86..e5e6c9d37f 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -24,6 +24,7 @@ SYNOPSIS
 'git stash' create [<message>]
 'git stash' store [(-m | --message) <message>] [-q | --quiet] <commit>
 'git stash' export (--print | --to-ref <ref>) [<stash>...]
+'git stash' import <commit>
 
 DESCRIPTION
 -----------
@@ -161,6 +162,12 @@ export ( --print | --to-ref <ref> ) [<stash>...]::
 	a chain of commits which can be transferred using the normal fetch and
 	push mechanisms, then imported using the `import` subcommand.
 
+import <commit>::
+
+	Import the specified stashes from the specified commit, which must have been
+	created by `export`, and add them to the list of stashes.  To replace the
+	existing stashes, use `clear` first.
+
 OPTIONS
 -------
 -a::
diff --git a/builtin/stash.c b/builtin/stash.c
index 192b0d9969..0e77fa94ee 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -61,6 +61,8 @@
 	N_("git stash create [<message>]")
 #define BUILTIN_STASH_EXPORT_USAGE \
 	N_("git stash export (--print | --to-ref <ref>) [<stash>...]")
+#define BUILTIN_STASH_IMPORT_USAGE \
+	N_("git stash import <commit>")
 #define BUILTIN_STASH_CLEAR_USAGE \
 	"git stash clear"
 
@@ -77,6 +79,7 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
 	BUILTIN_STASH_EXPORT_USAGE,
+	BUILTIN_STASH_IMPORT_USAGE,
 	NULL
 };
 
@@ -135,6 +138,10 @@ static const char * const git_stash_export_usage[] = {
 	NULL
 };
 
+static const char * const git_stash_import_usage[] = {
+	BUILTIN_STASH_IMPORT_USAGE,
+	NULL
+};
 
 static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
@@ -144,6 +151,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
@@ -154,6 +162,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;
@@ -1991,6 +2000,163 @@ static int write_commit_with_parents(struct repository *r,
 	return ret;
 }
 
+static int do_import_stash(struct repository *r, const char *rev)
+{
+	struct object_id chain;
+	int res = 0;
+	const char *buffer = NULL;
+	unsigned long bufsize;
+	struct commit *this = NULL;
+	struct commit_list *items = NULL, *cur;
+	char *msg = NULL;
+
+	if (repo_get_oid(r, rev, &chain))
+		return error(_("not a valid revision: %s"), rev);
+
+	this = lookup_commit_reference(r, &chain);
+	if (!this)
+		return error(_("not a commit: %s"), rev);
+
+	/*
+	 * Walk the commit history, finding each stash entry, and load data into
+	 * the array.
+	 */
+	for (;;) {
+		const char *author, *committer;
+		size_t author_len, committer_len;
+		const char *p;
+		const char *expected = "git stash <git@stash> 1000684800 +0000";
+		const char *prefix = "git stash: ";
+		struct commit *stash;
+		struct tree *tree = repo_get_commit_tree(r, this);
+
+		if (!tree ||
+		    !oideq(&tree->object.oid, r->hash_algo->empty_tree) ||
+		    (this->parents &&
+		     (!this->parents->next || this->parents->next->next))) {
+			res = error(_("%s is not a valid exported stash commit"),
+					oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+
+		if (!this->parents) {
+			/*
+			 * We don't have any parents.  Make sure this is our
+			 * root commit.
+			 */
+			author = find_commit_header(buffer, "author", &author_len);
+			committer = find_commit_header(buffer, "committer", &committer_len);
+
+			if (!author || !committer) {
+				error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
+				goto out;
+			}
+
+			if (author_len != strlen(expected) ||
+			    committer_len != strlen(expected) ||
+			    memcmp(author, expected, author_len) ||
+			    memcmp(committer, expected, committer_len)) {
+				res = error(_("found root commit %s with invalid data"), oid_to_hex(&this->object.oid));
+				goto out;
+			}
+			break;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		p += 2;
+		if (((size_t)(bufsize - (p - buffer)) < strlen(prefix)) ||
+		    memcmp(prefix, p, strlen(prefix))) {
+			res = error(_("found stash commit %s without expected prefix"), oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		stash = this->parents->next->item;
+
+		if (repo_parse_commit(r, this->parents->item) ||
+		    repo_parse_commit(r, stash)) {
+			res = error(_("cannot parse parents of commit: %s"),
+					oid_to_hex(&this->object.oid));
+			goto out;
+		}
+
+		if (check_stash_topology(r, stash)) {
+			res = error(_("%s does not look like a stash commit"),
+					oid_to_hex(&stash->object.oid));
+			goto out;
+		}
+
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+		items = commit_list_insert(stash, &items);
+		this = this->parents->item;
+	}
+
+	/*
+	 * Now, walk each entry, adding it to the stash as a normal stash
+	 * commit.
+	 */
+	for (cur = items; cur; cur = cur->next) {
+		const char *p;
+		struct object_id *oid;
+
+		this = cur->item;
+		oid = &this->object.oid;
+		buffer = repo_get_commit_buffer(r, this, &bufsize);
+		if (!buffer) {
+			res = error(_("cannot read commit buffer for %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p = strstr(buffer, "\n\n");
+		if (!p) {
+			res = error(_("cannot parse commit %s"), oid_to_hex(oid));
+			goto out;
+		}
+
+		p += 2;
+		msg = xmemdupz(p, bufsize - (p - buffer));
+		repo_unuse_commit_buffer(r, this, buffer);
+		buffer = NULL;
+
+		if (do_store_stash(oid, msg, 1)) {
+			res = error(_("cannot save the stash for %s"), oid_to_hex(oid));
+			goto out;
+		}
+		FREE_AND_NULL(msg);
+	}
+out:
+	if (this && buffer)
+		repo_unuse_commit_buffer(r, this, buffer);
+	free_commit_list(items);
+	free(msg);
+
+	return res;
+}
+
+static int import_stash(int argc, const char **argv, const char *prefix,
+			struct repository *repo)
+{
+	struct option options[] = {
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_stash_import_usage,
+			     PARSE_OPT_KEEP_DASHDASH);
+
+	if (argc != 1)
+		usage_msg_opt("a revision is required", git_stash_import_usage, options);
+
+	return do_import_stash(repo, argv[0]);
+}
+
 struct stash_entry_data {
 	struct repository *r;
 	struct commit_list **items;
@@ -2175,6 +2341,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("create", &fn, create_stash),
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
 		OPT_SUBCOMMAND("export", &fn, export_stash),
+		OPT_SUBCOMMAND("import", &fn, import_stash),
 		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 74666ff3e4..12d30a9865 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -11,6 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-unique-files.sh
 
+test_expect_success 'setup' '
+	test_oid_cache <<-EOF
+	export_base sha1:73c9bab443d1f88ac61aa533d2eeaaa15451239c
+	export_base sha256:f210fa6346e3e2ce047bdb570426b17075980c1ac01fec8fc4b75bd3ab4bcfe4
+	EOF
+'
+
 test_expect_success 'usage on cmd and subcommand invalid option' '
 	test_expect_code 129 git stash --invalid-option 2>usage &&
 	grep "or: git stash" usage &&
@@ -1412,6 +1419,100 @@ test_expect_success 'stash --keep-index --include-untracked with empty tree' '
 	)
 '
 
+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 --include-untracked -- subdir/ &&
+	git tag t-stash0 stash@{0} &&
+	git tag t-stash1 stash@{1} &&
+	simple=$(git stash export --print) &&
+	git stash clear &&
+	git stash import "$simple" &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1 &&
+	git stash export --to-ref refs/heads/foo &&
+	test_cmp_rev "$(test_oid empty_tree)" foo: &&
+	test_cmp_rev "$(test_oid empty_tree)" foo^: &&
+	test_cmp_rev t-stash0 foo^2 &&
+	test_cmp_rev t-stash1 foo^^2 &&
+	git log --first-parent --format="%s" refs/heads/foo >log &&
+	grep "^git stash: " log >log2 &&
+	test_line_count = 13 log2 &&
+	git stash clear &&
+	git stash import foo &&
+	test_cmp_rev stash@{0} t-stash0 &&
+	test_cmp_rev stash@{1} t-stash1
+'
+
+test_expect_success 'stash import appends commits' '
+	git log --format=oneline -g refs/stash >out &&
+	cat out out >out2 &&
+	git stash import refs/heads/foo &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = $(wc -l <out2) actual
+'
+
+test_expect_success 'stash export can accept specified stashes' '
+	git stash clear &&
+	git stash import foo &&
+	git stash export --to-ref refs/heads/bar stash@{1} stash@{0} &&
+	git stash clear &&
+	git stash import refs/heads/bar &&
+	test_cmp_rev stash@{1} t-stash0 &&
+	test_cmp_rev stash@{0} t-stash1 &&
+	git log --format=oneline -g refs/stash >actual &&
+	test_line_count = 2 actual
+'
+
+test_expect_success 'stash export rejects invalid arguments' '
+	test_must_fail git stash export --print --to-ref refs/heads/invalid 2>err &&
+	grep "exactly one of --print and --to-ref is required" err &&
+	test_must_fail git stash export 2>err2 &&
+	grep "exactly one of --print and --to-ref is required" err2
+'
+
+test_expect_success 'stash can import and export zero stashes' '
+	git stash clear &&
+	git stash export --to-ref refs/heads/baz &&
+	test_cmp_rev "$(test_oid empty_tree)" baz: &&
+	test_cmp_rev "$(test_oid export_base)" baz &&
+	test_must_fail git rev-parse baz^1 &&
+	git stash import baz &&
+	test_must_fail git rev-parse refs/stash
+'
+
+test_expect_success 'stash rejects invalid attempts to import commits' '
+	git stash import foo &&
+	test_must_fail git stash import HEAD 2>output &&
+	oid=$(git rev-parse HEAD) &&
+	grep "$oid is not a valid exported stash commit" output &&
+	test_cmp_rev stash@{0} t-stash0 &&
+
+	git checkout --orphan orphan &&
+	git commit-tree $(test_oid empty_tree) -p "$oid" -p "$oid^" -m "" >fake-commit &&
+	git update-ref refs/heads/orphan "$(cat fake-commit)" &&
+	oid=$(git rev-parse HEAD) &&
+	test_must_fail git stash import orphan 2>output &&
+	grep "found stash commit $oid without expected prefix" output &&
+	test_cmp_rev stash@{0} t-stash0 &&
+
+	git checkout --orphan orphan2 &&
+	git commit-tree $(test_oid empty_tree) -m "" >fake-commit &&
+	git update-ref refs/heads/orphan2 "$(cat fake-commit)" &&
+	oid=$(git rev-parse HEAD) &&
+	test_must_fail git stash import orphan2 2>output &&
+	grep "found root commit $oid with invalid data" output &&
+	test_cmp_rev stash@{0} t-stash0
+'
+
 test_expect_success 'stash apply should succeed with unmodified file' '
 	echo base >file &&
 	git add file &&

Re: [PATCH v8 0/4] Importing and exporting stashes to refs

[Phillip Wood]

Hi brian

On 12/06/2025 02:12, brian m. carlson wrote:
> Stashes are currently stored using the reflog in a given repository.
> This is an interesting and novel way to handle them, but there is no way
> to easily move a set of stashes across machines.  For example, groups of
> stashes cannot be bundled, pushed, or fetched.
> 
> Let's solve this problem by allowing users to import and export stashes
> to a chain of commits.  The commits used in a stash export contain two
> parents: one which is the pointer to the next exported stash (or to an
> empty commit with no parents if there are no more) and the second is the
> stash commit that would normally be stored in the reflog.
> 
> Original thread at message-ID: <20220310173236.4165310-1-sandals@crustytoothpaste.net>
> 
> Changes from v7:
> * Rephrase the documentation to be slightly more explicit.
> * Don't have `write_commit_with_parents` free its arguments, instead
>    letting the caller (who allocated them) handle them.
> * Handle invalid combinations of arguments to `export` and add tests for
>    this case.

The range-diff between v7 and v8 looks good to me. Sorry for the slow 
reply, I was off the list last week.

Thanks

Phillip

Re: [PATCH v8 0/4] Importing and exporting stashes to refs

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

>> Changes from v7:
>> * Rephrase the documentation to be slightly more explicit.
>> * Don't have `write_commit_with_parents` free its arguments, instead
>>    letting the caller (who allocated them) handle them.
>> * Handle invalid combinations of arguments to `export` and add tests for
>>    this case.
>
> The range-diff between v7 and v8 looks good to me. Sorry for the slow
> reply, I was off the list last week.

Thanks.  The topic is in 'next' now.