Re: [PATCH] revision: --ancestry-path

From: Johan Herland <johan@herland.net>
Date: Apr 21, 2010, 08:46 UTC
Message-ID: <201004211046.22643.johan@herland.net>
In-Reply-To: <7vochdcjz5.fsf@alter.siamese.dyndns.org>

On Wednesday 21 April 2010, Junio C Hamano wrote:

Show 20 quoted lines

> Johannes Sixt <j.sixt@viscovery.net> writes:
> > Am 4/21/2010 9:34, schrieb Johan Herland:
> >> I can confirm that this patch works with my original (more
> >> complicated) scenario. I've also played around with combinations of
> >> --ancestry-path, -- graph and --parents (and even --boundary), and
> >> AFAICS, the new option does not clobber the other options, and (IMHO)
> >> it all behaves correctly in the scenarios I've tested.
> > 
> > How about possible interactions with --reverse;
> > --date-order/--topo-order, --parents (important for gitk);
> > --simplify-by-decoration (useful for your problem that triggered
> > this); --full-history and --simplify-merges with path limiting?
> 
> These are all good points.
> 
> I am reasonably sure that parents (specifically, "rewrite_parents") is
> broken.  The new function should cull parents that do not appear on the
> ancestry path from merges (that is what "NEEDSWORK" is about).  It may or
> may not break gitk, though---these off-path parents are shown as parents
> of an on-path merge but will be marked as UNINTERESTING.

FWIW, I added the following patch to 'gitk', and then ran it against the t6019 test repo as follows:

	gitk --ancestry-path D..M

...and the resulting graph is exactly what I'd expect; showing the uninteresting parents (D, G and K) as "hollow" nodes.

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 1b0e09a..7749d2a 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -190,7 +190,7 @@ proc parseviewargs {n arglist} {
            "--author=*" - "--committer=*" - "--grep=*" - "-[iE]" -
            "--remove-empty" - "--first-parent" - "--cherry-pick" -
            "-S*" - "--pickaxe-all" - "--pickaxe-regex" -
-           "--simplify-by-decoration" {
+           "--simplify-by-decoration" - "--ancestry-path" {
                # These mean that we get a subset of the commits
                set filtered 1
                lappend glflags $arg


> I do not think reverse/date-order/topo-order would be affected by this
> change, as they only change the presentation order of what limit_list()
> returns;
> 
> Also simplify-merges and full-history should be Ok, as they control what
> is done in the current logic in limit_list(), which is an input to the
> new logic, meaning that the new logic will work on already simplified
> history.
> 
> This is not a new problem, but I strongly suspect that cherry-pick is
> broken the same way wrt "rewrite_parents".

I'm not very familiar with "rewrite_parents", nor do I know exactly how it 
should affect/interoperate with --ancestry-path in all cases, but running

	git rev-list D..M -- M.t

produces one commit (M), whereas

	git rev-list --ancestry-path D..M -- M.t

produces nothing, so I suspect there is something not quite right here.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net