diff mbox series

[2/2] revision: allow --ancestry-path to take an argument

Message ID 99287b67fd1c1e9fbceb4877738fb0aed722ec4a.1660704498.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Allow --ancestry-path to take an argument | expand

Commit Message

Elijah Newren Aug. 17, 2022, 2:48 a.m. UTC
From: Elijah Newren <newren@gmail.com>

We have long allowed users to run e.g.
    git log --ancestry-path next..seen
which shows all commits which satisfy all three of these criteria:
  * are an ancestor of seen
  * are not an ancestor next
  * have next as an ancestor

This commit allows another variant:
    git log --ancestry-path=$TOPIC next..seen
which shows all commits which satisfy all of these criteria:
  * are an ancestor of seen
  * are not an ancestor of next
  * have $TOPIC in their ancestry-path
that last bullet can be defined as commits meeting any of these
criteria:
    * are an ancestor of $TOPIC
    * have $TOPIC as an ancestor
    * are $TOPIC

This also allows multiple --ancestry-path arguments, which can be
used to find commits with any of the given topics in their ancestry
path.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/rev-list-options.txt | 45 ++++++++++++----
 object.h                           |  2 +-
 revision.c                         | 83 ++++++++++++++++++++----------
 revision.h                         |  3 ++
 t/t6019-rev-list-ancestry-path.sh  | 47 ++++++++++++++++-
 5 files changed, 139 insertions(+), 41 deletions(-)

Comments

Junio C Hamano Aug. 17, 2022, 10:42 p.m. UTC | #1
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> We have long allowed users to run e.g.
>     git log --ancestry-path next..seen
> which shows all commits which satisfy all three of these criteria:
>   * are an ancestor of seen
>   * are not an ancestor next
>   * have next as an ancestor

Is it a very good example, though?  Nothing builds on next, and next
is not an ancestor of seen, so the command without --ancestry-path
does give us individual commits that are not in 'next' yet, plus all
the merge commits in master..seen but with --ancestry-path the
answer is most likely an empty set.

If you replace 'next' with 'master', it does start to make sense,
but that is a bit too straight first-parent merge chain that may not
be all that interesting.

> This commit allows another variant:
>     git log --ancestry-path=$TOPIC next..seen
> which shows all commits which satisfy all of these criteria:
>   * are an ancestor of seen
>   * are not an ancestor of next
>   * have $TOPIC in their ancestry-path
> that last bullet can be defined as commits meeting any of these
> criteria:
>     * are an ancestor of $TOPIC
>     * have $TOPIC as an ancestor
>     * are $TOPIC

So, I have en/ancestry-path-in-a-range topic merged somewhere
between 'master' and 'seen'.  Here is what I see:

    $ seen/git log --oneline --ancestry-path=en/ancestry-path-in-a-range master..seen
    21aef6c754 Merge branch 'ab/submodule-helper-leakfix' into seen
    2a57fcc25e Merge branch 'ab/submodule-helper-prep' into seen
    72ff5f5d3a ###
    edb5cf4c31 Merge branch 'cw/remote-object-info' into seen
    cc8f65a665 Merge branch 'ag/merge-strategies-in-c' into seen
    c1bacacabf Merge branch 'es/mark-gc-cruft-as-experimental' into seen
    fdf2d207d2 Merge branch 'js/bisect-in-c' into seen
    2a1bbfc016 Merge branch 'po/glossary-around-traversal' into seen
    7ecf004b9e Merge branch 'vd/scalar-enables-fsmonitor' into jch
    9dba189986 Merge branch 'en/ancestry-path-in-a-range' into jch
    4461e34d7d revision: allow --ancestry-path to take an argument
    0605b4aad9 rev-list-options.txt: fix simple typo

which is very much expected.  Two commits are what we want to
highlight, and its merge into the first-parent-chain that leads to
'seen', and all its descendants on 'seen' are shown.

Due to the way "ancestry-path" is defined, replacing the value to
"--ancestry-path" from 4461e34d7d to 0605b4aad9 would not change the
output, which is also expected.  If this were a three-commit topic,
giving the middle commit would find both the first one (i.e. the
ancestor) and the third one (i.e. the descendant) in the topic, while
excluding the much-less-interesting base commit and its ancestors
the topic builds on. 

I am not exactly sure when this feature is useful, though.  It is
handy to be able to enumerate descendants of a given commit, so
perhaps the user knows about 0605b4aad9 and is trying to find other
commits on the same topic, or something?  But then the merges nearer
the tip of 'seen' than 9dba189986 are not very useful for that
purpose.  It somehow feels like a solution in search of a problem.

> diff --git a/revision.c b/revision.c
> index 0c6e26cd9c8..660f1dd1b9b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1105,7 +1105,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
>  			   struct commit_list **list, struct prio_queue *queue)
>  {
>  	struct commit_list *parent = commit->parents;
> -	unsigned left_flag;
> +	unsigned left_flag, ancestry_flag;
>  
>  	if (commit->object.flags & ADDED)
>  		return 0;
> @@ -1161,6 +1161,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
>  		return 0;
>  
>  	left_flag = (commit->object.flags & SYMMETRIC_LEFT);
> +	ancestry_flag = (commit->object.flags & ANCESTRY_PATH);

Wouldn't we want

	if (revs->ancestry_path)
		ancestry_flag = (commit->object.flags & ANCESTRY_PATH);

instead, so that the propagation of contaminated flag bits ...

>  	for (parent = commit->parents; parent; parent = parent->next) {
>  		struct commit *p = parent->item;
> @@ -1181,6 +1182,8 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
>  			if (!*slot)
>  				*slot = *revision_sources_at(revs->sources, commit);
>  		}
> +		if (revs->ancestry_path)
> +			p->object.flags |= ancestry_flag;
>  		p->object.flags |= left_flag;

... can become a simple

		p->object.flags |= ancestry_flag;

here?  Or even just use a single variable to compute the set of
flags to pass down i.e.

	pass_flags = commit->object.flags & (SYMMETRIC_LEFT | ANCESTRY_PATH);

before the loop, and then pass these two bits down at once, i.e.

-  		p->object.flags |= left_flag;
+  		p->object.flags |= pass_flags;

taking advantage of the fact that ANCESTRY_PATH and SYMMETRIC_LEFT
bits can be set to any object only when these features are in use?

Or did I misread the patch and sometimes ANCESTRY_PATH bit is set on
objects even when revs->ancestry_path is not in use?

> +static void limit_to_ancestry(struct commit_list *bottoms, struct commit_list *list)
>  {
>  	struct commit_list *p;
>  	struct commit_list *rlist = NULL;
> @@ -1323,7 +1333,7 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
>  	for (p = list; p; p = p->next)
>  		commit_list_insert(p->item, &rlist);
>  
> -	for (p = bottom; p; p = p->next)
> +	for (p = bottoms; p; p = p->next)
>  		p->item->object.flags |= TMP_MARK;
>  
>  	/*
> @@ -1356,38 +1366,39 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
>  	 */
>  
>  	/*
> -	 * The ones that are not marked with TMP_MARK are uninteresting
> +	 * The ones that are not marked with either TMP_MARK or
> +	 * ANCESTRY_PATH are uninteresting
>  	 */
>  	for (p = list; p; p = p->next) {
>  		struct commit *c = p->item;
> -		if (c->object.flags & TMP_MARK)
> +		if (c->object.flags & (TMP_MARK | ANCESTRY_PATH))
>  			continue;
>  		c->object.flags |= UNINTERESTING;
>  	}
>  
> -	/* We are done with the TMP_MARK */
> +	/* We are done with TMP_MARK and ANCESTRY_PATH */
>  	for (p = list; p; p = p->next)
> -		p->item->object.flags &= ~TMP_MARK;
> -	for (p = bottom; p; p = p->next)
> -		p->item->object.flags &= ~TMP_MARK;
> +		p->item->object.flags &= ~(TMP_MARK | ANCESTRY_PATH);
> +	for (p = bottoms; p; p = p->next)
> +		p->item->object.flags &= ~(TMP_MARK | ANCESTRY_PATH);
>  	free_commit_list(rlist);
>  }

We have called process_parents() to paint ancestor commits that can
be reached from the commit(s) of interest.  This helper function is
called after that is done, and propagates the ancestry_path bit in
the reverse direction, i.e. from parent to child.

Once we are done with this processing, we no longer need
ANCESTRY_PATH bit because the surviving ones without UNINTERESTING
bit set are the commits on the ancestry_path.  OK.
Elijah Newren Aug. 18, 2022, 4:01 a.m. UTC | #2
On Wed, Aug 17, 2022 at 3:42 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > We have long allowed users to run e.g.
> >     git log --ancestry-path next..seen
> > which shows all commits which satisfy all three of these criteria:
> >   * are an ancestor of seen
> >   * are not an ancestor next
> >   * have next as an ancestor
>
> Is it a very good example, though?  Nothing builds on next, and next
> is not an ancestor of seen, so the command without --ancestry-path
> does give us individual commits that are not in 'next' yet, plus all
> the merge commits in master..seen but with --ancestry-path the
> answer is most likely an empty set.
>
> If you replace 'next' with 'master', it does start to make sense,
> but that is a bit too straight first-parent merge chain that may not
> be all that interesting.

Yeah, I should have written master..seen.

> > This commit allows another variant:
> >     git log --ancestry-path=$TOPIC next..seen
> > which shows all commits which satisfy all of these criteria:
> >   * are an ancestor of seen
> >   * are not an ancestor of next
> >   * have $TOPIC in their ancestry-path
> > that last bullet can be defined as commits meeting any of these
> > criteria:
> >     * are an ancestor of $TOPIC
> >     * have $TOPIC as an ancestor
> >     * are $TOPIC
>
> So, I have en/ancestry-path-in-a-range topic merged somewhere
> between 'master' and 'seen'.  Here is what I see:
>
>     $ seen/git log --oneline --ancestry-path=en/ancestry-path-in-a-range master..seen
>     21aef6c754 Merge branch 'ab/submodule-helper-leakfix' into seen
>     2a57fcc25e Merge branch 'ab/submodule-helper-prep' into seen
>     72ff5f5d3a ###
>     edb5cf4c31 Merge branch 'cw/remote-object-info' into seen
>     cc8f65a665 Merge branch 'ag/merge-strategies-in-c' into seen
>     c1bacacabf Merge branch 'es/mark-gc-cruft-as-experimental' into seen
>     fdf2d207d2 Merge branch 'js/bisect-in-c' into seen
>     2a1bbfc016 Merge branch 'po/glossary-around-traversal' into seen
>     7ecf004b9e Merge branch 'vd/scalar-enables-fsmonitor' into jch
>     9dba189986 Merge branch 'en/ancestry-path-in-a-range' into jch
>     4461e34d7d revision: allow --ancestry-path to take an argument
>     0605b4aad9 rev-list-options.txt: fix simple typo
>
> which is very much expected.  Two commits are what we want to
> highlight, and its merge into the first-parent-chain that leads to
> 'seen', and all its descendants on 'seen' are shown.
>
> Due to the way "ancestry-path" is defined, replacing the value to
> "--ancestry-path" from 4461e34d7d to 0605b4aad9 would not change the
> output, which is also expected.  If this were a three-commit topic,
> giving the middle commit would find both the first one (i.e. the
> ancestor) and the third one (i.e. the descendant) in the topic, while
> excluding the much-less-interesting base commit and its ancestors
> the topic builds on.

Yes.

> I am not exactly sure when this feature is useful, though.  It is
> handy to be able to enumerate descendants of a given commit, so
> perhaps the user knows about 0605b4aad9 and is trying to find other
> commits on the same topic, or something?  But then the merges nearer
> the tip of 'seen' than 9dba189986 are not very useful for that
> purpose.  It somehow feels like a solution in search of a problem.

Here's my usecase:
    git replay --i --keep-base --contained --ancestry-path=$TOPIC master..seen

Explanation of usecase: I want to be able to replay commits, much like
the current interactive rebase feature.  Unlike rebase, the selection
of commits is done via a standard <revision range>, for flexibility.
Now, replaying of commits in $TOPIC implies I probably want all the
topics that depend on it to be updated, and all the merges that depend
on any of those to be updated.  But, to achieve that, I don't want to
have to manually run N rebases and manually remerge things and whatnot
-- I want it all done in one command.  (And yes, replaying of merges
needs to handle squashed-in semantic fixes and replay those.)  So, I
want this command to replay all commits in the ancestry path of $TOPIC
in the range master..seen, keeping the current base (--keep-base), and
I want it to also update any --contained branches (meaning any
branches pointing to commits in the range being replayed).

Alternatives: For this usecase,

  * I cannot just use $TOPIC_TO_EDIT..seen, because that excludes the
    commits I want to tweak.

  * I could technically use `--ancestry-path master..seen`, but it's
    almost uselessly suboptimal as it would put hundreds of irrelevant
    commits in the TODO list (making it hard for the user to find what
    they want to edit) and then wastes time replaying all those commits
    unnecessarily as well.

  * As far as I can tell, there are no good alternatives here; and my
    question about it on the list didn't spur any satisfying answers[1].

If you'd rather I waited to submit the patch as part of a much larger
series implementing git-replay, I can do that.  I just thought it was
useful to send upstream independently.  But then I ran into the problem
that I thought it was weird to describe a usecase depending on something
that isn't yet very usable, and tried to explain it without that.

[1] https://lore.kernel.org/git/CABPp-BEqWX3Nr2HDxwS9d-_QjcKb_jS=fSjsP_Pbutw7-P5gbg@mail.gmail.com/

> > diff --git a/revision.c b/revision.c
> > index 0c6e26cd9c8..660f1dd1b9b 100644
> > --- a/revision.c
> > +++ b/revision.c
> > @@ -1105,7 +1105,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
> >                          struct commit_list **list, struct prio_queue *queue)
> >  {
> >       struct commit_list *parent = commit->parents;
> > -     unsigned left_flag;
> > +     unsigned left_flag, ancestry_flag;
> >
> >       if (commit->object.flags & ADDED)
> >               return 0;
> > @@ -1161,6 +1161,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
> >               return 0;
> >
> >       left_flag = (commit->object.flags & SYMMETRIC_LEFT);
> > +     ancestry_flag = (commit->object.flags & ANCESTRY_PATH);
>
> Wouldn't we want
>
>         if (revs->ancestry_path)
>                 ancestry_flag = (commit->object.flags & ANCESTRY_PATH);
>
> instead, so that the propagation of contaminated flag bits ...
>
> >       for (parent = commit->parents; parent; parent = parent->next) {
> >               struct commit *p = parent->item;
> > @@ -1181,6 +1182,8 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
> >                       if (!*slot)
> >                               *slot = *revision_sources_at(revs->sources, commit);
> >               }
> > +             if (revs->ancestry_path)
> > +                     p->object.flags |= ancestry_flag;
> >               p->object.flags |= left_flag;
>
> ... can become a simple
>
>                 p->object.flags |= ancestry_flag;
>
> here?  Or even just use a single variable to compute the set of
> flags to pass down i.e.
>
>         pass_flags = commit->object.flags & (SYMMETRIC_LEFT | ANCESTRY_PATH);
>
> before the loop, and then pass these two bits down at once, i.e.
>
> -               p->object.flags |= left_flag;
> +               p->object.flags |= pass_flags;
>
> taking advantage of the fact that ANCESTRY_PATH and SYMMETRIC_LEFT
> bits can be set to any object only when these features are in use?

Ooh, that does seem better.  I'll make that change.

> Or did I misread the patch and sometimes ANCESTRY_PATH bit is set on
> objects even when revs->ancestry_path is not in use?
>
> > +static void limit_to_ancestry(struct commit_list *bottoms, struct commit_list *list)
> >  {
> >       struct commit_list *p;
> >       struct commit_list *rlist = NULL;
> > @@ -1323,7 +1333,7 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
> >       for (p = list; p; p = p->next)
> >               commit_list_insert(p->item, &rlist);
> >
> > -     for (p = bottom; p; p = p->next)
> > +     for (p = bottoms; p; p = p->next)
> >               p->item->object.flags |= TMP_MARK;
> >
> >       /*
> > @@ -1356,38 +1366,39 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
> >        */
> >
> >       /*
> > -      * The ones that are not marked with TMP_MARK are uninteresting
> > +      * The ones that are not marked with either TMP_MARK or
> > +      * ANCESTRY_PATH are uninteresting
> >        */
> >       for (p = list; p; p = p->next) {
> >               struct commit *c = p->item;
> > -             if (c->object.flags & TMP_MARK)
> > +             if (c->object.flags & (TMP_MARK | ANCESTRY_PATH))
> >                       continue;
> >               c->object.flags |= UNINTERESTING;
> >       }
> >
> > -     /* We are done with the TMP_MARK */
> > +     /* We are done with TMP_MARK and ANCESTRY_PATH */
> >       for (p = list; p; p = p->next)
> > -             p->item->object.flags &= ~TMP_MARK;
> > -     for (p = bottom; p; p = p->next)
> > -             p->item->object.flags &= ~TMP_MARK;
> > +             p->item->object.flags &= ~(TMP_MARK | ANCESTRY_PATH);
> > +     for (p = bottoms; p; p = p->next)
> > +             p->item->object.flags &= ~(TMP_MARK | ANCESTRY_PATH);
> >       free_commit_list(rlist);
> >  }
>
> We have called process_parents() to paint ancestor commits that can
> be reached from the commit(s) of interest.  This helper function is
> called after that is done, and propagates the ancestry_path bit in
> the reverse direction, i.e. from parent to child.
>
> Once we are done with this processing, we no longer need
> ANCESTRY_PATH bit because the surviving ones without UNINTERESTING
> bit set are the commits on the ancestry_path.  OK.
diff mbox series

Patch

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 2f85726745a..001e49cee55 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -389,12 +389,15 @@  Default mode::
 	merges from the resulting history, as there are no selected
 	commits contributing to this merge.
 
---ancestry-path::
+--ancestry-path[=<commit>]::
 	When given a range of commits to display (e.g. 'commit1..commit2'
-	or 'commit2 {caret}commit1'), only display commits that exist
-	directly on the ancestry chain between the 'commit1' and
-	'commit2', i.e. commits that are both descendants of 'commit1',
-	and ancestors of 'commit2'.
+	or 'commit2 {caret}commit1'), only display commits in that
+	range where <commit> is part of the ancestry chain.  By "part of
+	the ancestry chain", we mean including <commit> itself and
+	commits that are either ancestors or descendants of <commit>.
+	If no commit is specified, use 'commit1' (the excluded part of
+	the range) as <commit>.  Can be passed multiple times to look for
+	commits in the ancestry range of multiple commits.
 
 A more detailed explanation follows.
 
@@ -568,11 +571,10 @@  Note the major differences in `N`, `P`, and `Q` over `--full-history`:
 
 There is another simplification mode available:
 
---ancestry-path::
-	Limit the displayed commits to those directly on the ancestry
-	chain between the ``from'' and ``to'' commits in the given commit
-	range. I.e. only display commits that are ancestor of the ``to''
-	commit and descendants of the ``from'' commit.
+--ancestry-path[=<commit>]::
+	Limit the displayed commits to those containing <commit> in their
+	ancestry path.  I.e. only display <commit> and commits which have
+	<commit> as either a direct ancestor or descendant.
 +
 As an example use case, consider the following commit history:
 +
@@ -604,6 +606,29 @@  option does. Applied to the 'D..M' range, it results in:
 			       \
 				L--M
 -----------------------------------------------------------------------
++
+We can also use `--ancestry-path=D` instead of `--ancestry-path` which
+means the same thing when applied to the 'D..M' range but is just more
+explicit.
++
+If we instead are interested in a given topic within this range, and all
+commits affected by that topic, we may only want to view the subset of
+`D..M` which contain that topic in their ancestry path.  So, using
+`--ancestry-path=H D..M` for example would result in:
++
+-----------------------------------------------------------------------
+		E
+		 \
+		  G---H---I---J
+			       \
+				L--M
+-----------------------------------------------------------------------
++
+Whereas `--ancestry-path=K D..M` would result in
++
+-----------------------------------------------------------------------
+		K---------------L--M
+-----------------------------------------------------------------------
 
 Before discussing another option, `--show-pulls`, we need to
 create a new example history.
diff --git a/object.h b/object.h
index a2219464c2b..9caef89f1f0 100644
--- a/object.h
+++ b/object.h
@@ -59,7 +59,7 @@  struct object_array {
 
 /*
  * object flag allocation:
- * revision.h:               0---------10         15             23------26
+ * revision.h:               0---------10         15             23------27
  * fetch-pack.c:             01    67
  * negotiator/default.c:       2--5
  * walker.c:                 0-2
diff --git a/revision.c b/revision.c
index 0c6e26cd9c8..660f1dd1b9b 100644
--- a/revision.c
+++ b/revision.c
@@ -1105,7 +1105,7 @@  static int process_parents(struct rev_info *revs, struct commit *commit,
 			   struct commit_list **list, struct prio_queue *queue)
 {
 	struct commit_list *parent = commit->parents;
-	unsigned left_flag;
+	unsigned left_flag, ancestry_flag;
 
 	if (commit->object.flags & ADDED)
 		return 0;
@@ -1161,6 +1161,7 @@  static int process_parents(struct rev_info *revs, struct commit *commit,
 		return 0;
 
 	left_flag = (commit->object.flags & SYMMETRIC_LEFT);
+	ancestry_flag = (commit->object.flags & ANCESTRY_PATH);
 
 	for (parent = commit->parents; parent; parent = parent->next) {
 		struct commit *p = parent->item;
@@ -1181,6 +1182,8 @@  static int process_parents(struct rev_info *revs, struct commit *commit,
 			if (!*slot)
 				*slot = *revision_sources_at(revs->sources, commit);
 		}
+		if (revs->ancestry_path)
+			p->object.flags |= ancestry_flag;
 		p->object.flags |= left_flag;
 		if (!(p->object.flags & SEEN)) {
 			p->object.flags |= (SEEN | NOT_USER_GIVEN);
@@ -1304,13 +1307,20 @@  static int still_interesting(struct commit_list *src, timestamp_t date, int slop
 }
 
 /*
- * "rev-list --ancestry-path A..B" computes commits that are ancestors
+ * "rev-list --ancestry-path=C A..B" computes commits that are ancestors
  * of B but not ancestors of A but further limits the result to those
- * that are descendants of A.  This takes the list of bottom commits and
- * the result of "A..B" without --ancestry-path, and limits the latter
- * further to the ones that can reach one of the commits in "bottom".
+ * that have C in their ancestry path (i.e. are either ancestors of C,
+ * descendants of C, or are C).  If multiple --ancestry-path=$COMMITTISH
+ * arguments are supplied, we limit the result to those that have at
+ * least one of those COMMITTISH in their ancestry path. If
+ * --ancestry-path is specified with no commit, we use all bottom
+ * commits for C.
+ *
+ * Before this function is called, ancestors of C will have already been
+ * marked with ANCESTRY_PATH previously, so we just need to also mark
+ * the descendants here, then collect both sets of commits.
  */
-static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *list)
+static void limit_to_ancestry(struct commit_list *bottoms, struct commit_list *list)
 {
 	struct commit_list *p;
 	struct commit_list *rlist = NULL;
@@ -1323,7 +1333,7 @@  static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
 	for (p = list; p; p = p->next)
 		commit_list_insert(p->item, &rlist);
 
-	for (p = bottom; p; p = p->next)
+	for (p = bottoms; p; p = p->next)
 		p->item->object.flags |= TMP_MARK;
 
 	/*
@@ -1356,38 +1366,39 @@  static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
 	 */
 
 	/*
-	 * The ones that are not marked with TMP_MARK are uninteresting
+	 * The ones that are not marked with either TMP_MARK or
+	 * ANCESTRY_PATH are uninteresting
 	 */
 	for (p = list; p; p = p->next) {
 		struct commit *c = p->item;
-		if (c->object.flags & TMP_MARK)
+		if (c->object.flags & (TMP_MARK | ANCESTRY_PATH))
 			continue;
 		c->object.flags |= UNINTERESTING;
 	}
 
-	/* We are done with the TMP_MARK */
+	/* We are done with TMP_MARK and ANCESTRY_PATH */
 	for (p = list; p; p = p->next)
-		p->item->object.flags &= ~TMP_MARK;
-	for (p = bottom; p; p = p->next)
-		p->item->object.flags &= ~TMP_MARK;
+		p->item->object.flags &= ~(TMP_MARK | ANCESTRY_PATH);
+	for (p = bottoms; p; p = p->next)
+		p->item->object.flags &= ~(TMP_MARK | ANCESTRY_PATH);
 	free_commit_list(rlist);
 }
 
 /*
- * Before walking the history, keep the set of "negative" refs the
- * caller has asked to exclude.
+ * Before walking the history, add the set of "negative" refs the
+ * caller has asked to exclude to the bottom list.
  *
  * This is used to compute "rev-list --ancestry-path A..B", as we need
  * to filter the result of "A..B" further to the ones that can actually
  * reach A.
  */
-static struct commit_list *collect_bottom_commits(struct commit_list *list)
+static void collect_bottom_commits(struct commit_list *list,
+				   struct commit_list **bottom)
 {
-	struct commit_list *elem, *bottom = NULL;
+	struct commit_list *elem;
 	for (elem = list; elem; elem = elem->next)
 		if (elem->item->object.flags & BOTTOM)
-			commit_list_insert(elem->item, &bottom);
-	return bottom;
+			commit_list_insert(elem->item, bottom);
 }
 
 /* Assumes either left_only or right_only is set */
@@ -1414,12 +1425,12 @@  static int limit_list(struct rev_info *revs)
 	struct commit_list *original_list = revs->commits;
 	struct commit_list *newlist = NULL;
 	struct commit_list **p = &newlist;
-	struct commit_list *bottom = NULL;
 	struct commit *interesting_cache = NULL;
 
-	if (revs->ancestry_path) {
-		bottom = collect_bottom_commits(original_list);
-		if (!bottom)
+	if (revs->ancestry_path_need_bottoms) {
+		collect_bottom_commits(original_list,
+				       &revs->ancestry_path_bottoms);
+		if (!revs->ancestry_path_bottoms)
 			die("--ancestry-path given but there are no bottom commits");
 	}
 
@@ -1464,9 +1475,8 @@  static int limit_list(struct rev_info *revs)
 	if (revs->left_only || revs->right_only)
 		limit_left_right(newlist, revs);
 
-	if (bottom)
-		limit_to_ancestry(bottom, newlist);
-	free_commit_list(bottom);
+	if (revs->ancestry_path)
+		limit_to_ancestry(revs->ancestry_path_bottoms, newlist);
 
 	/*
 	 * Check if any commits have become TREESAME by some of their parents
@@ -2213,7 +2223,7 @@  static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 			       const struct setup_revision_opt* opt)
 {
 	const char *arg = argv[0];
-	const char *optarg;
+	const char *optarg = NULL;
 	int argcount;
 	const unsigned hexsz = the_hash_algo->hexsz;
 
@@ -2280,10 +2290,26 @@  static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->first_parent_only = 1;
 	} else if (!strcmp(arg, "--exclude-first-parent-only")) {
 		revs->exclude_first_parent_only = 1;
-	} else if (!strcmp(arg, "--ancestry-path")) {
+	} else if (!strcmp(arg, "--ancestry-path") ||
+		   skip_prefix(arg, "--ancestry-path=", &optarg)) {
 		revs->ancestry_path = 1;
 		revs->simplify_history = 0;
 		revs->limited = 1;
+		if (optarg) {
+			struct commit *c;
+			struct object_id oid;
+			const char *msg = _("could not get commit for ancestry-path argument %s");
+
+			if (repo_get_oid_committish(revs->repo, optarg, &oid))
+				return error(msg, optarg);
+			get_reference(revs, optarg, &oid, ANCESTRY_PATH);
+			c = lookup_commit_reference(revs->repo, &oid);
+			if (!c)
+				return error(msg, optarg);
+			commit_list_insert(c, &revs->ancestry_path_bottoms);
+		} else {
+			revs->ancestry_path_need_bottoms = 1;
+		}
 	} else if (!strcmp(arg, "-g") || !strcmp(arg, "--walk-reflogs")) {
 		init_reflog_walk(&revs->reflog_info);
 	} else if (!strcmp(arg, "--default")) {
@@ -2991,6 +3017,7 @@  static void release_revisions_topo_walk_info(struct topo_walk_info *info);
 void release_revisions(struct rev_info *revs)
 {
 	free_commit_list(revs->commits);
+	free_commit_list(revs->ancestry_path_bottoms);
 	object_array_clear(&revs->pending);
 	object_array_clear(&revs->boundary_commits);
 	release_revisions_cmdline(&revs->cmdline);
diff --git a/revision.h b/revision.h
index e576845cdd1..d86241ad8fc 100644
--- a/revision.h
+++ b/revision.h
@@ -48,6 +48,7 @@ 
  */
 #define NOT_USER_GIVEN	(1u<<25)
 #define TRACK_LINEAR	(1u<<26)
+#define ANCESTRY_PATH	(1u<<27)
 #define ALL_REV_FLAGS	(((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR | PULL_MERGE)
 
 #define DECORATE_SHORT_REFS	1
@@ -164,6 +165,7 @@  struct rev_info {
 			cherry_mark:1,
 			bisect:1,
 			ancestry_path:1,
+			ancestry_path_need_bottoms:1,
 			first_parent_only:1,
 			exclude_first_parent_only:1,
 			line_level_traverse:1,
@@ -306,6 +308,7 @@  struct rev_info {
 	struct saved_parents *saved_parents_slab;
 
 	struct commit_list *previous_parents;
+	struct commit_list *ancestry_path_bottoms;
 	const char *break_bar;
 
 	struct revision_sources *sources;
diff --git a/t/t6019-rev-list-ancestry-path.sh b/t/t6019-rev-list-ancestry-path.sh
index af57a04b7ff..d3657737fa6 100755
--- a/t/t6019-rev-list-ancestry-path.sh
+++ b/t/t6019-rev-list-ancestry-path.sh
@@ -8,8 +8,13 @@  test_description='--ancestry-path'
 #   /                     \
 #  A-------K---------------L--M
 #
-#  D..M                 == E F G H I J K L M
-#  --ancestry-path D..M == E F H I J L M
+#  D..M                                     == E F G H I J K L M
+#  --ancestry-path                     D..M == E F   H I J   L M
+#  --ancestry-path=F                   D..M == E F       J   L M
+#  --ancestry-path=G                   D..M ==     G H I J   L M
+#  --ancestry-path=H                   D..M == E   G H I J   L M
+#  --ancestry-path=K                   D..M ==             K L M
+#  --ancestry-path=K --ancestry-path=F D..M == E F       J K L M
 #
 #  D..M -- M.t                 == M
 #  --ancestry-path D..M -- M.t == M
@@ -66,6 +71,44 @@  test_expect_success 'rev-list --ancestry-path D..M' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rev-list --ancestry-path=F D..M' '
+	test_write_lines E F J L M >expect &&
+	git rev-list --ancestry-path=F --format=%s D..M |
+	sed -e "/^commit /d" |
+	sort >actual &&
+	test_cmp expect actual
+'
+test_expect_success 'rev-list --ancestry-path=G D..M' '
+	test_write_lines G H I J L M >expect &&
+	git rev-list --ancestry-path=G --format=%s D..M |
+	sed -e "/^commit /d" |
+	sort >actual &&
+	test_cmp expect actual
+'
+test_expect_success 'rev-list --ancestry-path=H D..M' '
+	test_write_lines E G H I J L M >expect &&
+	git rev-list --ancestry-path=H --format=%s D..M |
+	sed -e "/^commit /d" |
+	sort >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rev-list --ancestry-path=K D..M' '
+	test_write_lines K L M >expect &&
+	git rev-list --ancestry-path=K --format=%s D..M |
+	sed -e "/^commit /d" |
+	sort >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rev-list --ancestry-path=F --ancestry-path=K D..M' '
+	test_write_lines E F J K L M >expect &&
+	git rev-list --ancestry-path=F --ancestry-path=K --format=%s D..M |
+	sed -e "/^commit /d" |
+	sort >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'rev-list D..M -- M.t' '
 	echo M >expect &&
 	git rev-list --format=%s D..M -- M.t |