Re: [PATCH] stash: add 'rename' subcommand

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jul 16, 2026 at 08:31:45AM +0000, Emin Özata via GitGitGadget wrote:
> From: =?UTF-8?q?Emin=20=C3=96zata?= <eminozata@proton.me>
> 
> There is no way to change the message of a stash entry after the
> fact.  The only option is dropping the entry and re-storing it by
> hand, which moves it to the top of the stash list and gets fiddly
> for deeper entries.
> 
> Add 'git stash rename <message> [<stash>]', defaulting to the
> latest entry like the other subcommands do.  It reads the object id
> and reflog message of the target entry and of the entries above it,
> drops them all like 'git stash drop' would, and stores them back in
> the same order, with the new message going to the target.  Position,
> contents and the reflog chain stay as they were.
> 
> The command checks every entry it is about to rewrite and refuses
> to start if one of them does not look like a stash commit, which
> can only happen when refs/stash was written to by hand.  Finding
> that out halfway through the sequence would lose entries.  Should a
> write-back fail anyway, the entry's object id is reported so it can
> be recovered with 'git stash store', and the command only reports
> success when the reflog ended up in the requested state.
> 
> This was proposed before: in 2010, as a "git reflog update" command
> that edited reflog entries in place [1].  When it came up again in
> 2013 [2], Junio rejected it on the grounds that reflogs are
> append-only recovery logs, and that whoever really cares about a
> stash message can pop and re-stash [3].  Michael Haggerty pointed
> out in that thread that refs/stash does not fit the description:
> its reflog is the primary data store for stash entries, and 'git
> stash drop' rewrites it all the time [4].  So this patch stays away
> from the reflog machinery entirely and does the suggested
> pop-and-re-stash workaround mechanically, without the detour
> through the working tree.

Hm. It's good to refer to to previous discussions. But I think it would
make sense to also document why explicitly _you_ want to have this
functionality. Like, what use case does it enable that you currently
cannot have right now? How is this different to what was proposed back
then that should make us reconsider whether or not to include it now?

> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
> index 50bb89f483..03f2e03096 100644
> --- a/Documentation/git-stash.adoc
> +++ b/Documentation/git-stash.adoc
> @@ -163,6 +164,12 @@ with no conflicts.
>  	created by `export`, and add them to the list of stashes.  To replace the
>  	existing stashes, use `clear` first.
>  
> +`rename [-q | --quiet] <message> [<stash>]`::
> +	Change the message of a single stash entry.  The entry keeps its
> +	position and its contents.  _<stash>_ must name an entry by
> +	index (e.g. `stash@{1}`); renaming refreshes the reflog
> +	timestamps of the entry and of the entries above it.

I think "rename" is a bit of a misleading name, doubly so with the
recently introduced `git refs rename` feature that renames a reference.
I'd suggest "reword" instead.

> diff --git a/builtin/stash.c b/builtin/stash.c
> index c4809f299a..94e66d6074 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1190,6 +1204,166 @@ out:
[snip]
> +static int do_rename_stash(struct stash_info *info, size_t idx,
> +			   const char *msg, int quiet)
> +{
> +	struct rename_data data = { .want = idx + 1 };
> +	size_t i, missing = 0;
> +	int ret = -1;
> +
> +	refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository),
> +					 ref_stash, collect_rename_entries,
> +					 &data);
> +	if (data.nr <= idx) {
> +		error(_("%s does not exist"), info->revision.buf);
> +		goto cleanup;
> +	}
> +
> +	if (!oideq(&info->w_commit, &data.entries[idx].oid)) {
> +		error(_("%s changed concurrently; try again"),
> +		      info->revision.buf);
> +		goto cleanup;
> +	}
> +
> +	/* refuse up front; do_store_stash() would die halfway through */
> +	for (i = 0; i < data.nr; i++) {
> +		struct commit *stash = lookup_commit_reference(the_repository,
> +							       &data.entries[i].oid);
> +
> +		if (!stash || check_stash_topology(the_repository, stash)) {
> +			error(_("%s does not look like a stash commit"),
> +			      oid_to_hex(&data.entries[i].oid));
> +			goto cleanup;
> +		}
> +	}

This loop here has potentially-quadratic runtime. Not so much with the
"files" backend, where we'll simply append the data to the log. But with
the reftable backend we'll basically end up writing each reflog entry
into a new table, and we'll end up compacting the tables many times
over.

> +
> +	while (missing <= idx) {
> +		if (drop_reflog_entry("stash@{0}"))
> +			goto restore;
> +		missing++;
> +	}

Same here, this will not perform well if you have a huge reflog.

We really should do all of this atomically, where we ideally delete the
old reflog and create the new reflog in a single transaction.

Thanks!

Patrick