From: Kevin Bracey <kevin@bracey.fi>
Date: Tue, 29 May 2018 06:11:44 GMT
Subject: Re: Weird revision walk behaviour
Message-ID: <7de5c4b3-800c-960d-2942-7aa562df8879@bracey.fi>
In-Reply-To: <20180528220651.20287-1-szeder.dev@gmail.com>

On 29/05/2018 01:06, SZEDER Gábor wrote:
>
> So, without investing nearly enough time to understand what is going
> on, I massaged the above diffs into this:
Cool.

> +				treesame_parents = xmalloc(sizeof(*treesame_parents));
There's no need to actually record a list here. This is just for the 
simple history. We are only interested into becoming a non-merge to 1 
treesame parent, so I think we just need to record a pointer to the 
first one we see, just as this would exit immediately for the first 
relevant one. For the full-history case, we already have the full "which 
parents are treesame" recording mechanism just above, but it only kicks 
in for merge commits and only when settings require it. Adding a malloc 
here would be significant machinery overhead.
> FWIW, the test suite passes with the above patch applied.
I doubt there's an existing case like this anywhere in the revision test 
suite :) . And this patch is focused enough that it *should* only be 
changing the behaviour of this very specific case. As such, it does feel 
a little like a kludge, but I think it's fine because it's aligning the 
simple-history analysis with the "analyse relevant parents if any, else 
analyse irrelevant" rule of the full-history.
>
> And here is the small PoC test case to illustrate the issue, which
> fails without but succeeds with the above patch.  Eventually it should
> be part of 't6012-rev-list-simplify.sh', of course, but I haven't
> looked into that yet.
It may be there's enough criss-crossy history to test here to merit 
breaking out into a second test series.

+#   B---M2   master
+#  / \ /
+# A   X
+#  \ / \
+#   C---M1   b2
+#
+# and modify 'file' in commits 'A' and 'B', so one of 'M1's parents
+# ('B') is TREESAME wrt. 'file'.

I guess we'll be wanting test cases for A..B2, B..B2 and C..B2, and some 
where the the base is "some other child of B or C".  "B..B2" is no 
longer a pure set subtraction for simplification as B is UNINTERESTING 
(ie not in the set) but RELEVANT (because you named it as a bottom 
commit), so B..B2 actually still leaves M1 with 2 relevant parents. 
You'd want test cases covering B relevant+C irrelevant and B 
irrelevant+C relevant, which means subtracting them without naming them 
- so name a child of one.

And then we need to think about whether we want it displayed in each of 
the other modes for each of those queries...

Kevin