[WIP PATCH] revision-walk: --parent option to find children of a parent

[WIP PATCH] revision-walk: --parent option to find children of a parent

[Michael J Gruber]

Introduce a new --parent=<commit> option for the revision walker which
limits the commits to those which have <commit> as one of their parents.

This allows to check, e.g., whether <commit> is a fork point.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
This is heavily WIP/RFC for the obvious reasons, but also these:

Currently, multiple --parent options are ORed. I'm still unsure whether that or ANDing
is more useful. ANDing can always be done cheaply by rev-listing with one
--parent and then checking the result commits. ORing, otoh can be achieved only
by doing multiple rev-list runs (unless it is implemented as it is, of course)..

As a consequence (of the decision for ORing), I don't limit automatically by
^<commit>. I should probably do that automatically for the single --parent
option case, it's a huge speed-up.

 revision.c |   18 ++++++++++++++++++
 revision.h |    3 +++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index b1c1890..db56cf6 100644
--- a/revision.c
+++ b/revision.c
@@ -1149,6 +1149,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 {
 	const char *arg = argv[0];
 	const char *optarg;
+	unsigned char sha1[20];
 	int argcount;
 
 	/* pseudo revision arguments */
@@ -1247,6 +1248,15 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--parents")) {
 		revs->rewrite_parents = 1;
 		revs->print_parents = 1;
+	} else if ((argcount = parse_long_opt("parent", argv, &optarg))) {
+		struct commit *a;
+		if (get_sha1(optarg, sha1))
+			return error("Not a valid object name: %s", optarg);
+		a = lookup_commit_reference(sha1);
+		if (!a)
+			return error("Couldn't look up commit object for '%s'", optarg);
+		commit_list_insert(a, &(revs->parent_list));
+		return argcount;
 	} else if (!strcmp(arg, "--dense")) {
 		revs->dense = 1;
 	} else if (!strcmp(arg, "--sparse")) {
@@ -1989,6 +1999,14 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi
 		return commit_ignore;
 	if (revs->merges_only && !(commit->parents && commit->parents->next))
 		return commit_ignore;
+	if (revs->parent_list) {
+		struct commit_list *a, *b;
+		for (b = revs->parent_list; b; b = b->next)
+			for (a = commit->parents; a; a = a->next)
+				if (!hashcmp(b->item->object.sha1, a->item->object.sha1))
+					return commit_show;
+		return commit_ignore;
+	}
 	if (!commit_match(commit, revs))
 		return commit_ignore;
 	if (revs->prune && revs->dense) {
diff --git a/revision.h b/revision.h
index 05659c6..e424a53 100644
--- a/revision.h
+++ b/revision.h
@@ -111,6 +111,9 @@ struct rev_info {
 	int		no_inline;
 	int		show_log_size;
 
+	/* Filter by parents */
+	struct commit_list *parent_list;
+
 	/* Filter by commit log message */
 	struct grep_opt	grep_filter;
 
-- 
1.7.3.98.g5ad7d

Re: [WIP PATCH] revision-walk: --parent option to find children of a parent

[Jonathan Nieder]

Hi,

Michael J Gruber wrote:

> Introduce a new --parent=<commit> option for the revision walker which
> limits the commits to those which have <commit> as one of their parents.

So this lists the direct children for a given commit.  Could you
mention a use case?  Why would one want to use this instead of

 the_commit=$(git rev-parse --verify <commit>) &&
 git rev-list --parents ... ^<commit> |
 grep -F " $the_commit"

?
 
> --- a/revision.c
> +++ b/revision.c
> @@ -1149,6 +1149,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  {
>  	const char *arg = argv[0];
>  	const char *optarg;
> +	unsigned char sha1[20];

Could this be given narrower scope?

> @@ -1989,6 +1999,14 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi
>  		return commit_ignore;
>  	if (revs->merges_only && !(commit->parents && commit->parents->next))
>  		return commit_ignore;
> +	if (revs->parent_list) {
> +		struct commit_list *a, *b;
> +		for (b = revs->parent_list; b; b = b->next)
> +			for (a = commit->parents; a; a = a->next)
> +				if (!hashcmp(b->item->object.sha1, a->item->object.sha1))
> +					return commit_show;
> +		return commit_ignore;
> +	}

The implementation looks good.

Re: [WIP PATCH] revision-walk: --parent option to find children of a parent

[Michael J Gruber]

Jonathan Nieder venit, vidit, dixit 09/30/2010 04:32 PM:
> Hi,
> 
> Michael J Gruber wrote:
> 
>> Introduce a new --parent=<commit> option for the revision walker which
>> limits the commits to those which have <commit> as one of their parents.
> 
> So this lists the direct children for a given commit.  Could you
> mention a use case?  Why would one want to use this instead of
> 
>  the_commit=$(git rev-parse --verify <commit>) &&
>  git rev-list --parents ... ^<commit> |
>  grep -F " $the_commit"

| cut -d' ' -f1 # or such :)

> 
> ?

Full disclosure: I didn't see that it can be scripted that easily,
simply because the description of "--parents" does not indicate at all
that it lists the commit along with its parents. I had the impression
that it outputs the parents only (same goes for --children.) so that
looping over the rev-list output would be required, which would have
been much slower.

As a use case, it not only tells you whether a specific commit is a fork
point, it's also a reverse of "rev-parse commit^@", so to say.

Personally, this was some exercise with the revision walker before
approaching something else there.

[It also helps with "Heck, I committed on top of a detached origin/next
yesterday, how did I tag that commit.", but there are other solutions,
of course...]

Speaking of that: It helps you "going forward(!)" from a commit that you
checked out (maybe for inspection). Just imagine trying to understand a
history by checking out versions iteratively. Say, you want to review
the last 10 revs, do a "checkout HEAD~10", then a "checkout HEAD~9" and
notice that's not quite the way to do it... (Again, there are
alternatives, of course.)

>> --- a/revision.c
>> +++ b/revision.c
>> @@ -1149,6 +1149,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>>  {
>>  	const char *arg = argv[0];
>>  	const char *optarg;
>> +	unsigned char sha1[20];
> 
> Could this be given narrower scope?

Sure! I expected more rough edges (and some resistance) which is why I
sent a WIP first.

Michael