Re: [PATCH] revision walker: Fix --boundary when limited

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mar 5, 2007, 19:57 UTC
Message-ID: <Pine.LNX.4.64.0703051145210.3998@woody.linux-foundation.org>
In-Reply-To: <Pine.LNX.4.64.0703051130090.3998@woody.linux-foundation.org>

On Mon, 5 Mar 2007, Linus Torvalds wrote:

Show 9 quoted lines

> 
> NOTE! Our patches aren't really mutually incompatible, and they attack the 
> problem from two different directions. You do the separate phase (which is 
> also correct), and my patch instead tries to clean up the commit walking 
> so that the commit number limiter works more like the date limiter (which 
> fundamentally has all the same issues! Including the problem with some 
> commits possibly being marked as boundary commits when they aren't really, 
> because the path-limiting or revision-limiting ended up cutting things off 
> *differently* than the date-limiting).

Side note: the reason you don't *notice* it with the date-limiter is simply that the date limiter *also* runs at limit-time, rather than just at the incremental "run at the end" phase. So the date-limiter is much simpler when done together with other limiters (like path and revision limiters).

HOWEVER. We can't do the same thing for the numerical one, because we need to run the other limiters *first*, and the numerical limiter always comes at the end. And the path-based "dense" limiter actually runs mostly incrementally, so you cannot do the numerical limiter until after it has run..

The way to really clean stuff up would be to:

 - first phase: limit by date and revision ranges first (both of those are 
   static and quick, and don't depend on anything else)

   We do this already (limit_list)

 - second phase: limit by pathname (we don't do this as a phase at all, we 
   do it incrementally: see "rewrite_parents()")

 -third phase: limit by number

HOWEVER. There's a damn good reason why we do things the way we do, namely simply the fact that we want to do pathname limiting as much at run-time as possible.. But we *could* do the "rewrite_parents()" thing both in the non-incremental and in the final phase. However, doing the parent rewriting is quite nasty and error-prone, so I've been avoiding it.

Anyway, I *suspect* that Dscho's patch might do the wrong thing for something like

	gitk -20 v1.4.4.. t/

exactly because of the subtle interaction between pathname limiting, static commit limiting *and* commit number limiting. Dscho?

		Linus