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

On Mon, 5 Mar 2007, Linus Torvalds wrote:

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