git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Junio C Hamano" <junkio@cox.net>,
	"Santi Béjar" <sbejar@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] revision walker: Fix --boundary when limited
Date: Mon, 5 Mar 2007 11:57:42 -0800 (PST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0703051145210.3998@woody.linux-foundation.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0703051130090.3998@woody.linux-foundation.org>



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

  reply	other threads:[~2007-03-05 19:57 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-05 10:02 [BUG] git-rev-list: --topo-order --boundary and --max-count Santi Béjar
2007-03-05 10:39 ` Junio C Hamano
2007-03-05 12:15   ` Junio C Hamano
2007-03-05 17:05   ` Linus Torvalds
2007-03-05 18:55   ` [PATCH] revision walker: Fix --boundary when limited Johannes Schindelin
2007-03-05 19:00     ` Johannes Schindelin
2007-03-05 19:39     ` Linus Torvalds
2007-03-05 19:57       ` Linus Torvalds [this message]
2007-03-05 21:10         ` Junio C Hamano
2007-03-06  1:12           ` Johannes Schindelin
2007-03-06  1:32             ` Junio C Hamano
2007-03-06  1:44               ` Johannes Schindelin
2007-03-06  1:58                 ` Junio C Hamano
2007-03-05 23:17         ` Johannes Schindelin
2007-03-06  0:36           ` Junio C Hamano
2007-03-06  0:41             ` [PATCH 2/4] revision traversal: retire BOUNDARY_SHOW Junio C Hamano
2007-03-06  2:05               ` Johannes Schindelin
2007-03-06  2:17                 ` Junio C Hamano
2007-03-06  2:23                   ` SHOWN means shown Junio C Hamano
2007-03-06  2:29                   ` [PATCH 2/4] revision traversal: retire BOUNDARY_SHOW Johannes Schindelin
2007-03-06 11:34                     ` Junio C Hamano
2007-03-06 15:52                       ` Johannes Schindelin
2007-03-06  0:41             ` [PATCH 3/4] git-bundle: various fixups Junio C Hamano
2007-03-06  2:13               ` Johannes Schindelin
2007-03-06  0:41             ` [PATCH 4/4] git-bundle: --list-prereqs Junio C Hamano
2007-03-05 21:10     ` [PATCH] revision walker: Fix --boundary when limited Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0703051145210.3998@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=sbejar@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).