Re: Weird revision walk behaviour

From: Kevin Bracey <kevin@bracey.fi>
Date: May 29, 2018, 06:11 UTC
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.

Show 5 quoted lines

>
> 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