Re: [PATCH 5/6] builtin/stash: provide a way to import stashes from a ref
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 16, 2022, 17:26 UTC
- Message-ID
- <xmqqa6dpn70b.fsf@gitster.g>
- In-Reply-To
- <20220310173236.4165310-6-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 16 quoted lines
> @@ -104,6 +109,7 @@ static struct strbuf stash_index_path = STRBUF_INIT;
> * b_commit is set to the base commit
> * i_commit is set to the commit containing the index tree
> * u_commit is set to the commit containing the untracked files tree
> + * c_commit is set to the first parent (chain commit) when importing and is otherwise unset
> * w_tree is set to the working tree
> * b_tree is set to the base tree
> * i_tree is set to the index tree
> @@ -114,6 +120,7 @@ struct stash_info {
> struct object_id b_commit;
> struct object_id i_commit;
> struct object_id u_commit;
> + struct object_id c_commit;
> struct object_id w_tree;
> struct object_id b_tree;
> struct object_id i_tree;With the redesign that an exported chain is a series of two-parent merges, where the first parent is used to string them together in a single strand of pearls and the second parent is the stash entry, the above change becomes totally unnecessary, right? The import side will be doing a first-parent walk of the export, pushing the second parent into reflog of refs/stash---we may want sanity check these second parents with assert_stash_like(), but there is no need to re-synthesize the stash entries anymore, which would simplify the implementation quite a bit, right? Namely:
Show 25 quoted lines
> +static int do_import_stash(const char *rev)
> +{
> + struct object_id oid;
> + size_t nitems = 0, nalloc = 0;
> + struct stash_info *items = NULL;
> + int res = 0;
> +
> + if (get_oid(rev, &oid))
> + return error(_("not a valid revision: %s"), rev);
> +
> + /*
> + * Walk the commit history, finding each stash entry, and load data into
> + * the array.
> + */
> + for (size_t i = 0;; i++, nitems++) {
> + int ret;
> +
> + if (nalloc <= i) {
> + size_t new = nalloc * 3 / 2 + 5;
> + items = xrealloc(items, new * sizeof(*items));
> + nalloc = new;
> + }
> + memset(&items[i], 0, sizeof(*items));
> + /* We want this to be quiet because it might not exist. */
> + ret = get_stash_info_for_import(&items[i], &oid);The new helper function is not necessary; we can use vanilla get_stash_info() on the second parent to get the same information, and we do not really need to keep it in core. We can sanity check the shape of the imported stash entry right away and discard everything except for the commit object name.
Show 18 quoted lines
> + /*
> + * Now, walk each entry, adding it to the stash as a normal stash
> + * commit.
> + */
> + for (ssize_t i = nitems - 1; i >= 0; i--) {
> + struct commit_list *parents = NULL;
> + struct commit_list **next = &parents;
> + struct object_id out;
> + char *msg;
> +
> + next = commit_list_append(lookup_commit_reference(the_repository, &items[i].b_commit), next);
> + next = commit_list_append(lookup_commit_reference(the_repository, &items[i].i_commit), next);
> + if (items[i].has_u)
> + next = commit_list_append(lookup_commit_reference(the_repository,
> + &items[i].u_commit),
> + next);
> +
> + msg = write_commit_with_parents(&out, &items[i], parents);And this part becomes completely unnecessary if we reuse what the origin repository already had, which directly can be ...
Show 5 quoted lines
> + if (!msg) {
> + res = -1;
> + goto out;
> + }
> + if (do_store_stash(&out, msg, 1)) {... fed to this call.