Re: [PATCH 2/6] builtin/stash: fill in all commit data
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 16, 2022, 16:50 UTC
- Message-ID
- <xmqqv8wdn8nx.fsf@gitster.g>
- In-Reply-To
- <20220310173236.4165310-3-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 21 quoted lines
> get_stash_info doesn't ensure that all entries are filled in in all > cases. However, we'll want to use this information to write new > commits, and when we do so, we'll need all the information to be > present. Fill in all the commit information whenever we call this > function. > > Note that the behavior of info->has_u doesn't change here. If we > previously read a tree a refs/stash^3:, then refs/stash^3 must be a > treeish. We already here assume that the other parents specifically > commits, so it should be safe to do so here as well. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > builtin/stash.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/builtin/stash.c b/builtin/stash.c > index 2aa06cc91d..128f0a01ef 100644 > --- a/builtin/stash.c > +++ b/builtin/stash.c > @@ -124,6 +124,7 @@ static void free_stash_info(struct stash_info *info)
Before this function ...
Show 7 quoted lines
> static void assert_stash_like(struct stash_info *info, const char *revision)
> {
> if (get_oidf(&info->b_commit, "%s^1", revision) ||
> + get_oidf(&info->i_commit, "%s^2", revision) ||
> get_oidf(&info->w_tree, "%s:", revision) ||
> get_oidf(&info->b_tree, "%s^1:", revision) ||
> get_oidf(&info->i_tree, "%s^2:", revision))... there is this comment.
/* * w_commit is set to the commit containing the working tree * 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 * w_tree is set to the working tree * b_tree is set to the base tree * i_tree is set to the index tree * u_tree is set to the untracked files tree */
We probably would want to comment that u_commit (hence u_tree) is optional. That is why assert_stash_like() does not say anything about u_commit or u_tree, right?
It is curious why the function does not learn to check w_commit, even though the proposed commit log message claims that this is about filling all commit data.
Show 10 quoted lines
> @@ -166,7 +167,8 @@ static int get_stash_info_1(struct stash_info *info, const char *commit, int qui > > assert_stash_like(info, revision); > > - info->has_u = !get_oidf(&info->u_tree, "%s^3:", revision); > + info->has_u = !get_oidf(&info->u_commit, "%s^3", revision) && > + !get_oidf(&info->u_tree, "%s^3:", revision); > > end_of_rev = strchrnul(revision, '@'); > strbuf_add(&symbolic, revision, end_of_rev - revision);