That improved git-gui blame viewer..

Shawn, could you please push it out somewhere else than just "pu", and convince Junio to merge it? Right now, "git gui blame" is almost useless in the standard git tree, and maybe the new one has some bugs, but it's at least _useful_ (and about a million times prettier than it used to be), so I don't think you can possibly regress in this area.

That said, I do have one comment about the state of git-gui "pu".. I think it's fairly pretty, and definitely useful, but one feature I end up really wishing for is a "search" button (or Ctrl-F). I might not know what line-number I'm looking for, I'm more likely to know which function I want to look at, and the most natural way to find it is with a simple ctrl-F.

Maybe it exists, but if so, I didn't find it.

But the old git-gui blame viewer didn't have that either, so this shouldn't hold up getting a useable git-gui into the standard git distro..

		Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

I do not need convincing but "Hey, pull it" needs to be given, especially as I do not use git-gui myself. The last time I looked at it, I think I said I liked it much better than the previous one.

Linus Torvalds <torvalds@linux-foundation.org> wrote:

Thanks! I'm glad at least one other person on this planet finds my work on the blame viewer at least partially useful.

No, there isn't a search. Tk internally offers one, I just haven't bound UI to it. Ctrl-F is actually bound to page down, much as it is in vi. Actually vi keys work everywhere in git-gui for scrolling.

I pushed out 0.7.3 tonight without search. I'll work on search and jump-to-line this week, and try to get an 0.7.4 early next week with those and maybe some other improvements in the blame viewer that I want to get done.

I may actually wind up just doing 0.8.0 sooner than I had expected. There's a lot of new features queued up in the master branch that I slated for 0.8.0 that I'm using on a daily basis and is production stable.

I wanted to implement some slick inotify() based features for the main window during 0.8.0, but I haven't had time to work on them. It would really improve performance of git-gui on Windows NT, and I do have a lot of git-gui users there.

On Mon, 11 Jun 2007, Shawn O. Pearce wrote:

I think a lot of people are going to enjoy the git-gui blame viewer..

I don't generally do "git blame" very much at all, and I've found that any time I do it (even historically - when I've used bk annotate and even CVS), I have invariably _always_ wanted to go back in history to see the blame of the *previous* version (because the commit that gets blamed initially is simply not interesting, and is just whitespace cleanups or whatever!).

Now, this was something I knew very well on an intellectual level, and I tried to explain in one of my early discourses on why "annotate" is such a horribly interface, and why git could do so much better, but it's something that I think "git-gui blame" is showing that could really be done.

Now, admittedly, git-gui seems buggy in this regard (I *think* you meant that the two commits that show up listed to the left of each line are the "before and after" commit, but they end both being the "after" commit for me, but I can almost *taste* git-gui doing the right thing here. And once it does that, I think a lot of people will start to realize how useful this kind of interactive "drill down into the history of a line" thing can be.

So I think git-gui isn't quite there yet, but I think it's getting pretty close. It would need

 - the afore-mentioned bug fixed (I _think_ it is a bug, but maybe you had 
   some other reason for having two separate columns of commits per line), 
   so that you can literally click on the *previous* version of a line and 
   say "I don't care about this commit, I want to see the previous one".

 - I'd love to see a "go to most recent change" thing (the way _I_ work, I 
   tend to peel history, so I often look at recent changes, but if that 
   one isn't interesting, I'd go to the "previous version" and then again 
   ask for "go to the most recent change".

 - I like the floating tooltip showing what the commit was, but I think it 
   could be extended a bit more to perhaps literally show what the 
   previous contents of that chunk were (again, that's useful for the "am 
   I interested in _this_ commit, or should I just dig down to the 
   previous one")

But yes, the current git-gui blame is already quite interesting. It's not "all that it could be", but it has gone from "Whee, that looks so ugly that I need to find me a couple of spoons to dig out my eyeballs with" to "Hey, I can see this being _really_ good".

		Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

Incidentally I added a small patch to underlying git-blame to let you ignore whitespace changes so that you can blame through them. I do not think I merged it to 'master' yet, but it is trivial and look obviously safe and correct.

I think the two columns are for "who _placed_ these lines in the final image" vs "where these lines originally came into the history". The former is with -C, and the latter is without.

An option to re-blame starting from the parent commit of what is currently blamed (i.e. "peel" one level) would certainly be interesting but I do not think git-gui has it (yet). Of course it has a not-so-interesting corner case of deciding which parent to follow for a merge, but a merge commit is blamed only for lines that are either evil or conflict fixups, so in practice there is not much to be gained from peeling a merge.

On 6/11/07, Junio C Hamano <gitster@pobox.com> wrote:

Not to advertise, but qgit has already that from ages.

Annotate algorithm of qgit is little different in that it starts from the oldest revision that modified a file and goes to the latest. In this way we can have the whole file history annotated in one pass and very fast.

Currently the slowest steps are, from fastest to slowest:

1 - annotating the files 2 - getting the file history 3 - getting the corrisponding patches (in case of long histories)

Thanks for your patience. Marco

"Marco Costalba" <mcostalba@gmail.com> writes:

Advertising is good.

I am not sure about two things in this description.

 (1) Are you emulating CVS-like "a file has an identity, and we
     follow its changes" model?  How does it handle file split,
     merge, and code movement in general?

 (2) It is unclear why going from old to new has the advantage
     of being "one pass", implication of which is that the
     opposite direction needs to be done as more than one pass.
     Care to enlighten?

On 6/12/07, Junio C Hamano <gitster@pobox.com> wrote:

It uses 'git rev-list HEAD -- <path>' to get the list of revisions that modified a path, I really would like to keep it like that because it is the way 'git' works, and I would feel uncomfortable in filtering out git results, it seems quite fragile to me.

This means that file splits, merges, renames etc.. are handled as much as they are handled in git. IOW *if* 'git rev-list HEAD -- <path>' returns a list of revisions taking in account all of the above, so it will, automatically, do qgit.

BTW _currentlly_ git-rev-list does not do that.

Going from oldest to newest has this advantage:

1 - start from a known good first (empty) annotation, i.e. the first revision in history has an empty annotation (this is a choice to get consistent results when dealing with git repository started after the begining of the project, Linux tree started from 2.6.12 is an example).

2 - Given a good annotation (ann1) at a given time in history you can calculate the next annotation (ann2), the annotation corresponding to the next (newer) revision in history that modified the file using just the diff between the two. If you don't discards ann1 you end up having both ann1 and ann2.

3 - So at the end of applying all the diff chain you get all the annotations for all the file revisions, each annotation requires only the previous one and the corresponding diff.

4 - You don't need to touch anymore an already calculated file, nor applying the corresponding diff more then one time.

So the complexity of annotating *all* the files revisions grows only linearly with the revision list size.

 Marco

On 6/12/07, Marco Costalba <mcostalba@gmail.com> wrote:

Going from newest to oldest, after having elaborated all the diffs between revisions you have the newest file correctly annotated.

Going from oldest to newest, after having elaborated *in the same way as above* all the diffs between revisions, you have _all_ the files correctly annotated.

Marco Costalba <mcostalba@gmail.com> wrote:

So apparently yes, qgit is emulating CVS. And yet much better things exist (git-blame).

Its not "the way git works". Its just one way of looking at the data in the object database. And its not as accurate or as interesting as what git-blame does.

And it may never do it.  Most of the split/merge code is actually
in git-blame and is probably too CPU intensive to slam into the
middle of git-rev-list as a path limiter operator.  But renames
*might* someday be included.
 

It is unclear why you aren't just using `git blame --incremental`. git-blame running in two passes (with and without -M -C -C) can yield some very interesting results on files, like Git's own revision.c. And with the new -w flag that Junio just added, there's even more interesting possibilities...

;-)

"Shawn O. Pearce" <spearce@spearce.org> writes:

I would not use the word "better", as it depends on what you are looking for.

Again, I would not say "accurate".

The way Marco describes is a perfectly valid way to satisfy expectations of people migrating from CVS. It's more faithful reproduction of CVS annotate behaviour. In a sense, git-blame does too much, but that is exactly why these "accurate and interesting" behaviours are optional.

Oh, I can guarantee you that git-rev-list will never ever do that. It is to traverse revisions while simplifying with path limiters, and path limiters by definition will not look inside contents. Think of it as asking "Had my project consisted of only arch/i386 and include/asm-i386 directories, what would the history be".

However, adding a new option to "git log" so that you can say 'git log --single-follow=$this_file_at_the_tip $branch' is a separate matter. I think it is a sensible thing to do. But even if we do that, I think --single-follow should limit itself to the rename following of "diff -M/-C" style. It is insane to include another path only because the result did copy&paste only a handful lines out of during its history.

On 6/12/07, Junio C Hamano <gitster@pobox.com> wrote:

Some weeks ago I've started to experiment with git-log as a replacement for git-rev-list (as suggested by Linus in a thread).

I found some problems and so I stopped, but I think it's time to give it another look.

Maybe this week-end I'll do some testing.

Marco

On 6/13/07, Marco Costalba <mcostalba@gmail.com> wrote:

Now I remember why I've quit with git-log: due to a big performance regression under Windows.

I've made some tests on a git tree copied under Windows:

running:  git rev-list --header --parents --boundary HEAD > tmp.txt

1,38s 1,37s 1,39s

then running: git log --pretty=raw -z --parents --boundary HEAD > tmp.txt

8,87s 8,87s 8,86s 8,86s

So at the end git-log seems 6,4 times slower the git-rev-list !

This would mean the end of the story for me.

Marco

Hi,

On Wed, 13 Jun 2007, Marco Costalba wrote:

Isn't that a bit unfair? You should have passed "--pretty=raw" to rev-list, too.

Since you already have that tree, could you try rev-list _with_ "--pretty=raw", just for comparison?

Thanks, Dscho

On 6/13/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Of course, here we are

running: git rev-list --pretty=raw --parents --boundary HEAD > tmp.txt

times are: 1,16s - 1,11s - 1,13s

BTW this performance gap seems to exsist only under Windows, under Linux if I remember correctly (cannot test now) the difference is smaller.

Marco

Hi,

On Wed, 13 Jun 2007, Marco Costalba wrote:

I just tested on cygwin. The funny thing is, I never get anything like 8 seconds (this is on git.git itself).

For me it is

~1.0s rev-list _without_ --pretty=raw ~1.6s rev-list _with_ --pretty=raw ~1.4s log _with_ --pretty=raw ~3.5s log _with_ --pretty=raw _and_ -z (!)

So, your delay could stem from -z doing weird things.

For fun, I also did it with the MinGW port:

~0.7s rev-list _without_ --pretty=raw ~1.0s rev-list _with_ --pretty=raw ~0.8s log _with_ --pretty=raw ~1.9s log _with_ --pretty=raw _and_ -z (!)

Draw your conclusions, gentlemen.

Ciao, Dscho

On 6/13/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Yes. I can confirm that without -z the speed is more or less the same of git-rev-list.

I've tested with cygwin installation of git *.exe files.

Something slows down terribly git-log under Windows when -z option is given, expecially on cygwin.

Some idea, sir?

Thanks Marco

P.S: I really would need a zero terminated record for parsing reasons, and git-rev-list prints a zero terminated output too with --header option and it works correctly.

Hi,

On Wed, 13 Jun 2007, Marco Costalba wrote:

No. Not yet.

Ciao, Dscho

On Wednesday 13 June 2007, Johannes Schindelin wrote:

That happens on linux, too.

I am not really familiar with that code, but comparision of the call graphs of "git log" vs. "git log -z", as produced by callgrind (shameless plug) and visualized by kcachegrind (again, another shameless plug) shows that the difference happens in log_tree_diff, called from log_tree_commit.

While without "-z", log_tree_diff immediatly returns because "opt->diff" is 0, in the case of "-z", the tree differences are fully done even not used at all ?!

The result is that "log_tree_commit" executes (inclusive callees) 340 million instructions without "-z", while it runs 2300 million instructions in the "-z" case.

Josef

Hi,

On Wed, 13 Jun 2007, Josef Weidendorfer wrote:

That's a cool trick!

And indeed it is:

This code in revision.c is responsible:

1181 opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i); 1182 if (opts > 0) { 1183 revs->diff = 1; 1184 i += opts - 1; 1185 continue; 1186 }

Now, to tell which diff options actually need revs->diff to be set to 1, that is the question.

Ciao, Dscho

On Wednesday 13 June 2007, Josef Weidendorfer wrote:

I wished git-gui and gitk would be better integrated for history/blame browsing; I also missed a text search functionality in the blame view of git-gui.

Neverless...

opt->diff is set to 1 in setup_revisions() whenever diff_opt_parse() parses an option. And "-z" is parsed in diff_opt_parse().

In cd2bdc, Linus write in the commit log

 - make setup_revision set a flag (revs->diff) if the diff-related
   arguments were used. This allows "git log" to decide whether it wants
   to show diffs or not.

So why is "-z" regarded as tree-diff related, leading to calculating diffs?

Josef

Hi,

On Wed, 13 Jun 2007, Josef Weidendorfer wrote:

Because it changes the behaviour of a diff. As far as I can tell, the line termination does not have any effect on any output _except_ the diff output.

So, I should have realized earlier that "git-log -z" _without_ anything in the way of "-p", "--stat" or friends does not make sense at all.

Ciao, Dscho

On Wednesday 13 June 2007, Johannes Schindelin wrote:

But it does make sense: it separates commits using '\0'.

Josef

Hi,

On Wed, 13 Jun 2007, Josef Weidendorfer wrote:

Indeed, I missed that. Of course, you could do something as simple as this, although it looks really hacky to me:

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

Because of the thinko in that commit. He obviously meant things like --stat, -p and its friends.

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:

I'm not sure when/if git-gui and gitk will ever integrate. It looks like it would be a good bit of work. ;-)

That's on my list of things to do very-soon-now. I'm hoping to get it into the next release of git-gui, which should be 0.8.0, and maybe will happen by early next week.

Junio C Hamano wrote:

[...]

Perhaps the qgit annotating would find it's way in core git as git-annotate (which currently is alias to git-blame + some compatibility options), or as an option (--bottom-up) to git-blame?

On the other hand side, blaming multiple files in parallel has sense I think only for graphical viewer, not for command line command.

On 6/14/07, Jakub Narebski <jnareb@gmail.com> wrote:

Yes, I agree. The only useful thing for command line is if you use it togheter with some cache mechanism to allow following runs faster.

git-blame --cache-full-history (?)

Marco

On 6/12/07, Shawn O. Pearce <spearce@spearce.org> wrote:

I will try to explore with git-blame better, thanks.

Marco