* [PATCH v6 0/5] Importing and exporting stashes to refs
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
@ 2025-05-22 18:55 ` brian m. carlson
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
2025-05-22 18:55 ` [PATCH v6 1/5] object-name: make get_oid quietly return an error brian m. carlson
` (5 subsequent siblings)
6 siblings, 1 reply; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 18:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble
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(-)
^ permalink raw reply [flat|nested] 72+ messages in thread* [PATCH v7 0/4] Importing and exporting stashes to refs
2025-05-22 18:55 ` [PATCH v6 0/5] Importing and exporting stashes to refs brian m. carlson
@ 2025-06-01 22:32 ` brian m. carlson
2025-06-01 22:32 ` [PATCH v7 1/4] object-name: make get_oid quietly return an error brian m. carlson
` (5 more replies)
0 siblings, 6 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-01 22:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble
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(-)
^ permalink raw reply [flat|nested] 72+ messages in thread* [PATCH v7 1/4] object-name: make get_oid quietly return an error
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
@ 2025-06-01 22:32 ` brian m. carlson
2025-06-01 22:32 ` [PATCH v7 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
` (4 subsequent siblings)
5 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-01 22:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble
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;
+ }
}
}
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v7 2/4] builtin/stash: factor out revision parsing into a function
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
2025-06-01 22:32 ` [PATCH v7 1/4] object-name: make get_oid quietly return an error brian m. carlson
@ 2025-06-01 22:32 ` brian m. carlson
2025-06-01 22:32 ` [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
` (3 subsequent siblings)
5 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-01 22:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble
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;
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
2025-06-01 22:32 ` [PATCH v7 1/4] object-name: make get_oid quietly return an error brian m. carlson
2025-06-01 22:32 ` [PATCH v7 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
@ 2025-06-01 22:32 ` brian m. carlson
2025-06-05 9:25 ` Phillip Wood
2025-06-11 11:31 ` Kristoffer Haugsbakk
2025-06-01 22:32 ` [PATCH v7 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
` (2 subsequent siblings)
5 siblings, 2 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-01 22:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble
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()
};
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref
2025-06-01 22:32 ` [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
@ 2025-06-05 9:25 ` Phillip Wood
2025-06-11 11:31 ` Kristoffer Haugsbakk
1 sibling, 0 replies; 72+ messages in thread
From: Phillip Wood @ 2025-06-05 9:25 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, D. Ben Knoble
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
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref
2025-06-01 22:32 ` [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
2025-06-05 9:25 ` Phillip Wood
@ 2025-06-11 11:31 ` Kristoffer Haugsbakk
2025-06-11 23:35 ` brian m. carlson
1 sibling, 1 reply; 72+ messages in thread
From: Kristoffer Haugsbakk @ 2025-06-11 11:31 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble
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
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref
2025-06-11 11:31 ` Kristoffer Haugsbakk
@ 2025-06-11 23:35 ` brian m. carlson
0 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-11 23:35 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, Junio C Hamano, Phillip Wood, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v7 4/4] builtin/stash: provide a way to import stashes from a ref
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
` (2 preceding siblings ...)
2025-06-01 22:32 ` [PATCH v7 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
@ 2025-06-01 22:32 ` brian m. carlson
2025-06-05 9:38 ` [PATCH v7 0/4] Importing and exporting stashes to refs Phillip Wood
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
5 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-01 22:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble
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 &&
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v7 0/4] Importing and exporting stashes to refs
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
` (3 preceding siblings ...)
2025-06-01 22:32 ` [PATCH v7 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
@ 2025-06-05 9:38 ` Phillip Wood
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
5 siblings, 0 replies; 72+ messages in thread
From: Phillip Wood @ 2025-06-05 9:38 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, D. Ben Knoble
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(-)
>
^ permalink raw reply [flat|nested] 72+ messages in thread* [PATCH v8 0/4] Importing and exporting stashes to refs
2025-06-01 22:32 ` [PATCH v7 0/4] " brian m. carlson
` (4 preceding siblings ...)
2025-06-05 9:38 ` [PATCH v7 0/4] Importing and exporting stashes to refs Phillip Wood
@ 2025-06-12 1:12 ` brian m. carlson
2025-06-12 1:12 ` [PATCH v8 1/4] object-name: make get_oid quietly return an error brian m. carlson
` (4 more replies)
5 siblings, 5 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-12 1:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble, Kristoffer Haugsbakk
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(-)
^ permalink raw reply [flat|nested] 72+ messages in thread* [PATCH v8 1/4] object-name: make get_oid quietly return an error
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
@ 2025-06-12 1:12 ` brian m. carlson
2025-06-12 1:12 ` [PATCH v8 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
` (3 subsequent siblings)
4 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-12 1:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble, Kristoffer Haugsbakk
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;
+ }
}
}
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v8 2/4] builtin/stash: factor out revision parsing into a function
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
2025-06-12 1:12 ` [PATCH v8 1/4] object-name: make get_oid quietly return an error brian m. carlson
@ 2025-06-12 1:12 ` brian m. carlson
2025-06-12 1:12 ` [PATCH v8 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
` (2 subsequent siblings)
4 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-12 1:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble, Kristoffer Haugsbakk
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;
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v8 3/4] builtin/stash: provide a way to export stashes to a ref
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
2025-06-12 1:12 ` [PATCH v8 1/4] object-name: make get_oid quietly return an error brian m. carlson
2025-06-12 1:12 ` [PATCH v8 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
@ 2025-06-12 1:12 ` brian m. carlson
2025-06-12 1:12 ` [PATCH v8 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
2025-06-25 8:40 ` [PATCH v8 0/4] Importing and exporting stashes to refs Phillip Wood
4 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-12 1:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble, Kristoffer Haugsbakk
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()
};
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v8 4/4] builtin/stash: provide a way to import stashes from a ref
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
` (2 preceding siblings ...)
2025-06-12 1:12 ` [PATCH v8 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
@ 2025-06-12 1:12 ` brian m. carlson
2025-06-25 8:40 ` [PATCH v8 0/4] Importing and exporting stashes to refs Phillip Wood
4 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-06-12 1:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Phillip Wood, D. Ben Knoble, Kristoffer Haugsbakk
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 &&
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v8 0/4] Importing and exporting stashes to refs
2025-06-12 1:12 ` [PATCH v8 " brian m. carlson
` (3 preceding siblings ...)
2025-06-12 1:12 ` [PATCH v8 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
@ 2025-06-25 8:40 ` Phillip Wood
2025-06-25 16:30 ` Junio C Hamano
4 siblings, 1 reply; 72+ messages in thread
From: Phillip Wood @ 2025-06-25 8:40 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, D. Ben Knoble, Kristoffer Haugsbakk
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
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v8 0/4] Importing and exporting stashes to refs
2025-06-25 8:40 ` [PATCH v8 0/4] Importing and exporting stashes to refs Phillip Wood
@ 2025-06-25 16:30 ` Junio C Hamano
0 siblings, 0 replies; 72+ messages in thread
From: Junio C Hamano @ 2025-06-25 16:30 UTC (permalink / raw)
To: Phillip Wood; +Cc: brian m. carlson, git, D. Ben Knoble, Kristoffer Haugsbakk
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.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v6 1/5] object-name: make get_oid quietly return an error
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
2025-05-22 18:55 ` [PATCH v6 0/5] Importing and exporting stashes to refs brian m. carlson
@ 2025-05-22 18:55 ` brian m. carlson
2025-05-22 19:27 ` Junio C Hamano
2025-05-22 18:55 ` [PATCH v6 2/5] reflog-walk: expose read_complete_reflog brian m. carlson
` (4 subsequent siblings)
6 siblings, 1 reply; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 18:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble
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;
+ }
}
}
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v6 1/5] object-name: make get_oid quietly return an error
2025-05-22 18:55 ` [PATCH v6 1/5] object-name: make get_oid quietly return an error brian m. carlson
@ 2025-05-22 19:27 ` Junio C Hamano
0 siblings, 0 replies; 72+ messages in thread
From: Junio C Hamano @ 2025-05-22 19:27 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, D. Ben Knoble
"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?
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
2025-05-22 18:55 ` [PATCH v6 0/5] Importing and exporting stashes to refs brian m. carlson
2025-05-22 18:55 ` [PATCH v6 1/5] object-name: make get_oid quietly return an error brian m. carlson
@ 2025-05-22 18:55 ` brian m. carlson
2025-05-22 21:53 ` Ramsay Jones
2025-05-29 16:01 ` Phillip Wood
2025-05-22 18:55 ` [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function brian m. carlson
` (3 subsequent siblings)
6 siblings, 2 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 18:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble
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
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-22 18:55 ` [PATCH v6 2/5] reflog-walk: expose read_complete_reflog brian m. carlson
@ 2025-05-22 21:53 ` Ramsay Jones
2025-05-23 23:22 ` brian m. carlson
2025-05-29 16:01 ` Phillip Wood
1 sibling, 1 reply; 72+ messages in thread
From: Ramsay Jones @ 2025-05-22 21:53 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, D. Ben Knoble
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
>
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-22 21:53 ` Ramsay Jones
@ 2025-05-23 23:22 ` brian m. carlson
2025-05-24 1:09 ` Ramsay Jones
0 siblings, 1 reply; 72+ messages in thread
From: brian m. carlson @ 2025-05-23 23:22 UTC (permalink / raw)
To: Ramsay Jones; +Cc: git, Junio C Hamano, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-23 23:22 ` brian m. carlson
@ 2025-05-24 1:09 ` Ramsay Jones
2025-05-26 19:55 ` brian m. carlson
0 siblings, 1 reply; 72+ messages in thread
From: Ramsay Jones @ 2025-05-24 1:09 UTC (permalink / raw)
To: brian m. carlson, git, Junio C Hamano, D. Ben Knoble
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
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-24 1:09 ` Ramsay Jones
@ 2025-05-26 19:55 ` brian m. carlson
0 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-26 19:55 UTC (permalink / raw)
To: Ramsay Jones; +Cc: git, Junio C Hamano, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-22 18:55 ` [PATCH v6 2/5] reflog-walk: expose read_complete_reflog brian m. carlson
2025-05-22 21:53 ` Ramsay Jones
@ 2025-05-29 16:01 ` Phillip Wood
2025-05-29 21:59 ` Junio C Hamano
1 sibling, 1 reply; 72+ messages in thread
From: Phillip Wood @ 2025-05-29 16:01 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, D. Ben Knoble
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
>
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 2/5] reflog-walk: expose read_complete_reflog
2025-05-29 16:01 ` Phillip Wood
@ 2025-05-29 21:59 ` Junio C Hamano
0 siblings, 0 replies; 72+ messages in thread
From: Junio C Hamano @ 2025-05-29 21:59 UTC (permalink / raw)
To: Phillip Wood; +Cc: brian m. carlson, git, D. Ben Knoble
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.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
` (2 preceding siblings ...)
2025-05-22 18:55 ` [PATCH v6 2/5] reflog-walk: expose read_complete_reflog brian m. carlson
@ 2025-05-22 18:55 ` brian m. carlson
2025-05-22 20:34 ` Junio C Hamano
2025-05-22 18:55 ` [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref brian m. carlson
` (2 subsequent siblings)
6 siblings, 1 reply; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 18:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble
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;
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function
2025-05-22 18:55 ` [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function brian m. carlson
@ 2025-05-22 20:34 ` Junio C Hamano
2025-05-23 23:25 ` brian m. carlson
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2025-05-22 20:34 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, D. Ben Knoble
"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?
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function
2025-05-22 20:34 ` Junio C Hamano
@ 2025-05-23 23:25 ` brian m. carlson
2025-05-24 0:23 ` Junio C Hamano
0 siblings, 1 reply; 72+ messages in thread
From: brian m. carlson @ 2025-05-23 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function
2025-05-23 23:25 ` brian m. carlson
@ 2025-05-24 0:23 ` Junio C Hamano
2025-05-26 19:36 ` brian m. carlson
0 siblings, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2025-05-24 0:23 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, D. Ben Knoble
"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.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function
2025-05-24 0:23 ` Junio C Hamano
@ 2025-05-26 19:36 ` brian m. carlson
0 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-26 19:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
` (3 preceding siblings ...)
2025-05-22 18:55 ` [PATCH v6 3/5] builtin/stash: factor out revision parsing into a function brian m. carlson
@ 2025-05-22 18:55 ` brian m. carlson
2025-05-22 20:51 ` Junio C Hamano
2025-05-29 16:01 ` Phillip Wood
2025-05-22 18:55 ` [PATCH v6 5/5] builtin/stash: provide a way to import stashes from " brian m. carlson
2025-05-22 19:00 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
6 siblings, 2 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 18:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble
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()
};
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref
2025-05-22 18:55 ` [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref brian m. carlson
@ 2025-05-22 20:51 ` Junio C Hamano
2025-05-26 19:42 ` brian m. carlson
2025-05-29 16:01 ` Phillip Wood
1 sibling, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2025-05-22 20:51 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, D. Ben Knoble
"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.
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref
2025-05-22 20:51 ` Junio C Hamano
@ 2025-05-26 19:42 ` brian m. carlson
0 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-26 19:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref
2025-05-22 18:55 ` [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref brian m. carlson
2025-05-22 20:51 ` Junio C Hamano
@ 2025-05-29 16:01 ` Phillip Wood
1 sibling, 0 replies; 72+ messages in thread
From: Phillip Wood @ 2025-05-29 16:01 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Junio C Hamano, D. Ben Knoble
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
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
` (4 preceding siblings ...)
2025-05-22 18:55 ` [PATCH v6 4/5] builtin/stash: provide a way to export stashes to a ref brian m. carlson
@ 2025-05-22 18:55 ` brian m. carlson
2025-05-22 21:09 ` Junio C Hamano
2025-05-22 21:15 ` Junio C Hamano
2025-05-22 19:00 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
6 siblings, 2 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 18:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble
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 &&
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref
2025-05-22 18:55 ` [PATCH v6 5/5] builtin/stash: provide a way to import stashes from " brian m. carlson
@ 2025-05-22 21:09 ` Junio C Hamano
2025-05-26 20:03 ` brian m. carlson
2025-05-22 21:15 ` Junio C Hamano
1 sibling, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2025-05-22 21:09 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, D. Ben Knoble
"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.
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref
2025-05-22 21:09 ` Junio C Hamano
@ 2025-05-26 20:03 ` brian m. carlson
0 siblings, 0 replies; 72+ messages in thread
From: brian m. carlson @ 2025-05-26 20:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, D. Ben Knoble
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref
2025-05-22 18:55 ` [PATCH v6 5/5] builtin/stash: provide a way to import stashes from " brian m. carlson
2025-05-22 21:09 ` Junio C Hamano
@ 2025-05-22 21:15 ` Junio C Hamano
2025-05-23 13:17 ` Phillip Wood
1 sibling, 1 reply; 72+ messages in thread
From: Junio C Hamano @ 2025-05-22 21:15 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Phillip Wood, D. Ben Knoble
"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?
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v6 5/5] builtin/stash: provide a way to import stashes from a ref
2025-05-22 21:15 ` Junio C Hamano
@ 2025-05-23 13:17 ` Phillip Wood
0 siblings, 0 replies; 72+ messages in thread
From: Phillip Wood @ 2025-05-23 13:17 UTC (permalink / raw)
To: Junio C Hamano, brian m. carlson; +Cc: git, D. Ben Knoble
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?
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH] Makefile: avoid constant rebuilds with compilation database
2025-05-22 18:55 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
` (5 preceding siblings ...)
2025-05-22 18:55 ` [PATCH v6 5/5] builtin/stash: provide a way to import stashes from " brian m. carlson
@ 2025-05-22 19:00 ` brian m. carlson
2025-05-22 19:08 ` Junio C Hamano
6 siblings, 1 reply; 72+ messages in thread
From: brian m. carlson @ 2025-05-22 19:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, pwodd, D. Ben Knoble, Philippe Blain
[-- 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 --]
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH] Makefile: avoid constant rebuilds with compilation database
2025-05-22 19:00 ` [PATCH] Makefile: avoid constant rebuilds with compilation database brian m. carlson
@ 2025-05-22 19:08 ` Junio C Hamano
0 siblings, 0 replies; 72+ messages in thread
From: Junio C Hamano @ 2025-05-22 19:08 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, pwodd, D. Ben Knoble, Philippe Blain
"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.
^ permalink raw reply [flat|nested] 72+ messages in thread