[Feature Request] git blame showing only revisions from git rev-list --first-parent

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

[Feature Request] git blame showing only revisions from git rev-list --first-parent

From: Stephen Connolly <hidden>
Date: 2016-06-15 23:06:30

Background
===

My colleagues and I disagree on how exactly to handle feature
development in branches.
There are approximately two camps:

Camp 1: All feature branches should be squashed after code review to a
single commit before merging.

Camp 2: Leave the history of development alone, being able to trace
why the feature evolved that way is useful information, never squash
commits after code review.

Being somebody who leans towards the second camp I have been trying to
understand the first camps objections...

Those objections seem to boil down to tracing when the feature was introduced.

I have been able to get past some of the objections by pointing out
the `--first-parent` option available to `git log` and `gitk`. With
that command line option they are presented with a single history of
the branch and all merge commits are shown *as if squashed* (or at
least that is what it looks like from my testing)

The only remaining objection is around `git blame`...

You see a problem on one line in the file, you run `git blame
file.txt` and it says <rev1> was when it changed...

Now <rev1> is not on the main line of the branch but actually is a
commit that was merged from one branch to another before finally being
merged into the current branch in <rev2>

That is a complex nest of commits to detangle, especially when we have
had some refactoring branches with multiple experiments that drag on
over the course of a couple of weeks with many commits... all they
want to know is <rev2>.

Yes I could (and I do try to) teach them how to find the commit where
that SHA was merged into the current branch, but really they just want
to be able to see the current branches SHAs only.

Doing some reading in the git blame sources I discovered that git
blame takes the rev-list command line options... so I thought "hey
--first-parent should work"

Except it doesn't

I suspect that this is because of how blame uses rev-list

Request
===

A command line option to `git blame HEAD -- path` that instructs that
the revisions of blame be the revisions where the change was applied
to the current branch not the revision where the change first
originated (i.e. limit commits to `git rev-list --first-parent HEAD`)

I can get what I want with the following:

git rev-list --first-parent HEAD | awk '{print p " " $0}{p=$0}' >
tmpfile && git blame -b -S tmpfile HEAD -- path && rm tmpfile

But that is a rather ugly command. Could we have something built in to
git blame to make this much easier for users?

Thanks

Re: [Feature Request] git blame showing only revisions from git rev-list --first-parent

From: Jeff King <hidden>
Date: 2016-06-15 23:06:30

On Fri, Sep 11, 2015 at 11:47:30AM +0100, Stephen Connolly wrote:
A command line option to `git blame HEAD -- path` that instructs that
the revisions of blame be the revisions where the change was applied
to the current branch not the revision where the change first
originated (i.e. limit commits to `git rev-list --first-parent HEAD`)

I can get what I want with the following:

git rev-list --first-parent HEAD | awk '{print p " " $0}{p=$0}' >
tmpfile && git blame -b -S tmpfile HEAD -- path && rm tmpfile

But that is a rather ugly command. Could we have something built in to
git blame to make this much easier for users?
I agree this would be a useful feature. Though blame takes rev-list
options, it doesn't use the stock rev-list traversal internally, so it
has to handle first-parent itself.

I'm not too familiar with the code, but this _seems_ to work for me:
diff --git a/builtin/blame.c b/builtin/blame.c
index 21321be..2e03d47 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit
 static int num_scapegoats(struct rev_info *revs, struct commit *commit)
 {
 	struct commit_list *l = first_scapegoat(revs, commit);
+	if (!l)
+		return 0;
+	if (revs->first_parent_only)
+		return 1;
 	return commit_list_count(l);
 }
 
I suspect it doesn't work at all with `--reverse`. I also have the
nagging feeling that this could be handled inside revision.c with
parent-rewriting, but I don't really know.

But "git blame --first-parent <file>" seems to behave sanely in git.git
with this.

-Peff

Re: [Feature Request] git blame showing only revisions from git rev-list --first-parent

From: Stephen Connolly <hidden>
Date: 2016-06-15 23:06:30

On 11 September 2015 at 15:01, Jeff King [off-list ref] wrote:
quoted hunk
On Fri, Sep 11, 2015 at 11:47:30AM +0100, Stephen Connolly wrote:
quoted
A command line option to `git blame HEAD -- path` that instructs that
the revisions of blame be the revisions where the change was applied
to the current branch not the revision where the change first
originated (i.e. limit commits to `git rev-list --first-parent HEAD`)

I can get what I want with the following:

git rev-list --first-parent HEAD | awk '{print p " " $0}{p=$0}' >
tmpfile && git blame -b -S tmpfile HEAD -- path && rm tmpfile

But that is a rather ugly command. Could we have something built in to
git blame to make this much easier for users?
I agree this would be a useful feature. Though blame takes rev-list
options, it doesn't use the stock rev-list traversal internally, so it
has to handle first-parent itself.

I'm not too familiar with the code, but this _seems_ to work for me:
diff --git a/builtin/blame.c b/builtin/blame.c
index 21321be..2e03d47 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1375,6 +1375,10 @@ static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit
 static int num_scapegoats(struct rev_info *revs, struct commit *commit)
 {
        struct commit_list *l = first_scapegoat(revs, commit);
+       if (!l)
+               return 0;
+       if (revs->first_parent_only)
+               return 1;
        return commit_list_count(l);
 }

I suspect it doesn't work at all with `--reverse`. I also have the
nagging feeling that this could be handled inside revision.c with
parent-rewriting, but I don't really know.

But "git blame --first-parent <file>" seems to behave sanely in git.git
with this.

-Peff
It would help if somebody who knew the code could comment, but I am
impressed with the progress ;-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help