Thread (1 message) 1 message, 1 author, 2022-03-16

Re: [PATCH 2/6] builtin/stash: fill in all commit data

From: Junio C Hamano <hidden>
Date: 2022-03-16 16:50:48

"brian m. carlson" [off-list ref] writes:
quoted hunk
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 <redacted>
---
 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 ...
 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.
quoted hunk
@@ -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);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help