570676 - Add a web command that annotates by "first seen code", not last changed like hg

Status: NEW

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Axel Hecht [:Pike]]

Attachment: alternative annotate, command-line script

There's some discussion about how to work around a bad "add/remove" instead of "mv", and I propose that we add a new command to hgweb that reverts the order in which we're annotating.

I'll attach some sample code that's perf-wise comparable with hg annotate. Native hg takes some 10 seconds for nsCSSFrameConstructor, my code takes 12.

I'll also attach the output of my code and hg's code for nsCSSFrameConstructor, as well as a diff, as they do differ, and I'd love to get some review of that difference. It might just be what we want, too.

Comment 1

[Axel Hecht [:Pike]]

Attachment: nsCSSFrameConstructor.cpp annonate as given by hg

Comment 2

[Axel Hecht [:Pike]]

Attachment: nsCSSFrameConstructor.cpp annonate as given by my code

Comment 3

[Axel Hecht [:Pike]]

Attachment: nsCSSFrameConstructor.cpp annonate diff, hg vs new code

Comment 4

[Dirkjan Ochtman (:djc)]

Attachment: Extension version

Here's a simple alternative version that can be enabled as an extension.

Comment 5

[(not currently active) Ted Mielczarek]

I know both bz and smaug have had issues with hg annotate.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I'm not sure if this change would helped my work a lot, especially because
CVS history isn't in hg anyway.
It is often more important to see when and why something changed around
some code.
Koningsberg's HG Blame is quite useful for that, since I can just hover
over the lines nearby and see the comments related to the changes.
http://mxr-test.konigsberg.mozilla.org/bonsai/cvsblame.cgi?file=/layout/base/nsCSSFrameConstructor.cpp&xrev=5b3604a3cfbe&tree=mozilla-central
Koningberg approach makes it also very easy to browse back in the 
changeset history.

Comment 7

[(no longer active)]

(In reply to comment #6)
> I'm not sure if this change would helped my work a lot, especially because
> CVS history isn't in hg anyway.
> It is often more important to see when and why something changed around
> some code.
> Koningsberg's HG Blame is quite useful for that, since I can just hover
> over the lines nearby and see the comments related to the changes.
> http://mxr-test.konigsberg.mozilla.org/bonsai/cvsblame.cgi?file=/layout/base/nsCSSFrameConstructor.cpp&xrev=5b3604a3cfbe&tree=mozilla-central
> Koningberg approach makes it also very easy to browse back in the 
> changeset history.

That's so cool!  Is there any reason why we don't host that software and integrate MXR with it?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

There is http://mxr-test.konigsberg.mozilla.org/ MXR. But I don't know why
we haven't included that in mxr.mozilla.org.
The old CVS blame UI (which the HG blame is based on) is just so much easier to use than hgweb annotate.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 449825 [details] [diff] [review]
nsCSSFrameConstructor.cpp annonate diff, hg vs new code

As discussed on irc, blaming IsInlineOutside on rev 4478 is wrong; that rev didn't change that function in any way.  Blaming IsInlineFrame on 4478 is maybe ok; it was landed in 4478, then backed out, then relanded in 4696...  this could lead to confusion when looking at lines added between the backout and 4696 and trying to understand what the file looked like when they were added, of course.

Comment 10

[Boris Zbarsky [:bzbarsky]]

I should note that mxr-test's hg bonsai could benefit from pike's changes too, I bet.  And yes, we should hook that up as default instead of hgweb annotate imho.  I've just switched to using it, with no plans to look back.

Comment 11

[Axel Hecht [:Pike]]

OK, I found the problem.

The latest code does a diff of the current version to the old version, and the location of the IsInlineOutside fragment is just too large for the diff algorithm to find the similar fragment. Used to be line 535, and is at 382 today.

The solution is to actually go from an initial blame, and develop that forward in time diff by diff.

Comment 12

[(no longer active)]

(In reply to comment #10)
> I should note that mxr-test's hg bonsai could benefit from pike's changes too,
> I bet.  And yes, we should hook that up as default instead of hgweb annotate
> imho.  I've just switched to using it, with no plans to look back.

Filed bug 570737 for that.

Comment 13

[(not currently active) Ted Mielczarek]

I hacked up djc's copy of Pike's work into both a shell command and web command (that overrides /annotate) and added cvs blame support for good measure:
http://hg.mozilla.org/users/tmielczarek_mozilla.com/pushlog/

The webcommand relies on uncommitted local template changes at the moment, I'll push my template changes somewhere soon.

Comment 14

[(not currently active) Ted Mielczarek]

Okay, I pushed my template changes as well:
http://hg.mozilla.org/users/tmielczarek_mozilla.com/hg_templates

Comment 15

[(not currently active) Ted Mielczarek]

Oh, also, if you want to test the cvs blame bit, you can grab this file and stick it in .hg/ in a mozilla-central clone:
http://hg.mozilla.org/users/bsmedberg_mozilla.com/static-analysis-buildbot/file/19e7a98a8dc4/cvsblame.db