(clicky.visophyte.org-hosted CouchDB services offline)

This means doccelerator and my Tinderbox scraper that chucked stuff into a CouchDB for exposure by a modified bugzilla jetpack.  Either couch went crazy or someone gave it a request that is ridiculously expensive to answer with a database the accumulated size of the tinderbox database.  I’m not aware of any trivial ways to contend with the latter.  Judging by the logs, it looks like 2 people other than myself used these services, so, my apologies to those cool, insightful, forward-looking individuals.

The specific services are probably not coming back.  doccelerator is being subsumed into something else that better meets documentation needs and will be more fully baked, more on that soon.  I got the impression at the summit that the tinderbox problem is in hand, or very close to someone’s hand; maybe one of those robot grabby-arm things is involved?  My reviewboard with bugzilla hacks instance is sticking around for the time being, but I think wheels are turning elsewhere in Mozilla on that front too, so hopefully my install is mooted before it falls over.

Lest there be any doubt, all clicky services are provided on a self-interested basis… I am happy when they benefit others, but I do these things for the benefit of my own productivity/sanity and they are hosted using my own resources.  (While I had higher hopes for doccelerator, the MoMo-resourced couchdb service provisioning never happened so doccelerator never got pushed public with nightly updates because of said clicky resource constraints.)

emacs compilation-mode support for reviewboard

Let’s say you’ve suckered someone into performing their review in my review board install (more info).  It looks pretty, no?  As the Amish say: ‘Tis a fine web page, but sure ’tis no emacs buffer, English.

Get your reviewboard-viewer.el here (repo here).  It basically just wgets the review with paths in a format the GNU regex will recognize and tells the temporary file to be a compilation-mode buffer with a little help in setting up the file search path.  There are instructions in the file, but the key non-intuitive things that are going to trip you up:

  • You really want to invoke reviewboard-viewer-view-review from a dired buffer showing the root directory of what the patch is against.  This lets us set up the search path to be exactly correct with no heuristics.
  • The argument the above wants is the id that you will see in the “bugzilla-style export” link on the review page.  For example, I had to type the number 72 above.

Other notes to people who are interested/care:

  • There was a dumb bug in the bugzilla export code involving caching.  Previous bugzilla exports likely only showed the selected hunk for the first comment on the file.  I fixed this.
  • OpenID snafus:
    • Weave OpenID support via the Accout Manager doesn’t work.  (Account Manager is where it got refactored to after it got dropped from Weave core).  I feel like it worked recently with 0.0.2, but just now when I tried (with 0.0.2) it pretended it was going to work and then failed to do so.  When I upgraded to 0.0.10 it decided to not even do that.  This is sad because Weave was the whole reason I went with OpenID; I wanted to make it easier than creating another throwaway account for people.
    • I hear that whatever Google OpenID-ish thing exists, it apparently does not work either.  This was not surprising since it was my understanding Google made their implementation correct but not interoperable without some extra work.
    • Verisign’s freebie OpenID thing does work!
    • If someone knows of a django auth-compatible OpenID/OAuth/whatever implementation that will at least fix the Google situation and requires practically no work on my part, I would be happy to change to using that.  Right now I am using django-openid-auth which is old but works.  My reviewboard repo is here if you are feeling especially generous about fixing things.

Tinderbox results in bugzilla, jetpack times 2, CouchDB, review board

mstange‘s Tinderboxpushlog is awesome.  You know it, I know it, the many people whose sanity has been saved by it know it.  It is a fantastic improvement on checking the tinderbox; it lets you know the current state of the tree, the recent history of the tree, and how these things are correlated with recent commits.  What it is not good at (nor intended for) is to be a historical record.  Tinderboxpushlog ‘scrapes’ the tinderbox on-demand using tinderbox time windows and does not have the ability to key off anything but the time.

So if one refactored the scraper into a CommonJS module suitable for use with the newly rebooted jetpack, hooked it up to a cron job, and crammed its outputinto a CouchDB database, what would you get?

Exactly, something you can hook up to johnath and ehsan‘s magic bugzilla jetpack (last mentioned on the blog here).  You can install it from here.  (johnath/ehsan, please feel free to pull the changes into the upstream repo; I’m still wary of randomly pushing things into other people’s user repos…).  I know the presentation is ugly, not the least because the text labels are inconsistent with tinderboxpushlog; feel free to push into my user repo with improvements.  Oh, and for this to work you need to put an honest-to-goodness URL in your bugzilla comment or attachment description; there is no all-powerful regex if you type things out by hand.

You can find the thing that pushes thing into the couch using refactored tinderboxpushlog logic here.  Some cuddlefish runner tweaks (not all of which are likely advisable) can be found in my user jetpack-sdk repo.  (The new jetpack reboot is wicked awesome, by the way.)

Right now the cron job is running against the MozillaTry, Firefox, and Thunderbird3.1 trees on the tinderbox every 5 minutes.  While it should be pretty easy for me to keep the cron job and couch server online, I make no guarantees about the schema used in the couch, just that I will keep the jetpack in sync with it.  And if the service starts exceeding the resources of my (personal) linode, I may have to tweak polling rates or what not (‘what not’ meaning ‘up to and including turning it off’).

There is other work happening in this area and I am excited about that.  For example, I think brasstacks has an encompassing vision that should help provide historical data about which tests failed, etc.  With any luck, my efforts here will be mooted by buildbot and magic awesome dust.

The mention of reviewboard in the title is just that if you are using my review board stuff and put a link to your try server commit in the attachment description, we will use that to pull the official hg changeset as the basis for the diff.  The main benefit of this is that if your patch depends on other patches in your queue that are not yet in the trunk, the diff will still work out.  Specifically, if your queue had A, B, and C applied (where C is the tip) and you link to C, then we will provide a diff of C relative to B.  Please be aware that hg.mozilla.org is rather slow about providing the hg changeset diffs on demand so this will be at least an order of magnitude slower than the fetch of an already-available patch from bugzilla.  Repo with changes is here.

Review Board and Bugzilla reviews, take 3

I’ve updated my review board setup once more (part 2, part 1).  The low barrier to entry is now even lower.  “How low?”, you might ask.  “On the ground!”, I might say.  “What other low low price features with big big value are on offer? With more facts and less spiel?”, you might then also ask…

  • It now works with patches that have a header.  Patches that were the result of an mq import and then directly uploaded will tend to have headers.  This was why sometimes patches would fail to import with unlikely errors about empty patches.
  • There is now a magic URL scheme that automatically pulls the patch, creates a review and bounces you to the review.  If the review already exists, it just directly bounces you.  That URL scheme is http://reviews.visophyte.org/r/bzpatch/bug###/attach###/.  There are no authentication requirements on using this URL nor viewing the diff or associated reviews.  However, if you want to actually use the review mechanism to make comments then you will need to login via OpenID.  See part 2 for more on that.
  • I modified johnath and Ehsan Akhgari’s magic bugzilla jetpack so that it also adds a “Review” link to patches.  My modified repo is here.  You can install it from here.  You can see what the word “Review” looks like up in the first screenshot.
  • I upgraded to the reviewboard git trunk.  This adds some improvements to the diff display such as showing you function context information, even if the patch did not include it.  (And even if it did, too.  A lot of patches involving C++ truncate the function signature, whereas as you can see in the screenshot, you get the full text!)  I believe it is also supposed to be clever about recognizing moved blocks.

Limitations and other notes:

  • Patch fetching is synchronous and can take a while because we also parse it all up before we return.  Do not sit there hitting reload.  We’ll give you an error message if it doesn’t work out.  Not a great one, but an error message nonetheless.
  • Patches are still assumed to be against mozilla-central or comm-central (depending on the bugzilla product) trunk as of the moment we fetch the patch.  This means bit-rotted patches that you are only looking at now may fail to apply.  At the same time, patches where you clicked on the link back when it was timely and go back to look at them now that they are going out of style are still going to be applied against the same revision they were in the first place.
  • The repo with my modified changes (on the bzreview-master branch) was nuked and re-created because of the svn -> git transition by the reviewboard people.  So if you previously pulled, you should probably blow your old repo away rather than end up with a weird hybrid mixture.  (btw, hg-git works very nicely, with the caveat that its bookmark-based representation of the git branching idiom confuses pbranch really quite badly.)
  • Ping me on IRC or drop me an e-mail if you are experiencing reliable problems after having determined that the patch in question is not just full of gibberish.

Review Board and Bugzilla reviews, take 2.

review-board-diff

Last time I played with Review Board and bugzilla request queues things were great for me, but no one else.  I had to create an account for you on the instance, add you to my script that synchronizes request queues, run the script, and then keep running the script periodically.  Not to mention there wasn’t really a way to get your review out and into bugzilla.  No one actually tried to use it, so they probably also didn’t notice there were caching issues related to the ever-changing definition of “HEAD” (“tip”).  It sucked, and when I upgraded and everything broke, no one cared, not even me.

But now it’s back and better than ever:

review-board-bugzilla-queue-sync

  • People don’t need to login at all to see review requests and reviews!  Just point them at the URL and away they go.  (Actually, this was the case before, but it was not obvious.)
  • You can/must sign in with Open ID!  If you have Weave and are reading this in the future, Weave can be your Open ID friend!  If you are like me and live in the present (Weave 0.3.2), something is wrong and it doesn’t work, not to mention that Weave takes over the Open ID box so you can’t use credentials that work.
  • There’s a button that updates your request queue for the 16 most recent requests made of you.  Just make sure that you have entered your bugzilla e-mail address on the “my account” page.  This may have happened automatically depending on what your Open ID provider provided/you told them to provide.
  • There’s export functionality so you can take your review and cram it in bugzilla.
  • The definition of “tip” gets nailed down when the review request gets created, so no more ugly caching issues.  Patches can still fail to apply, though, if “tip” has drifted from when the patch was first created.
  • It has friendlier assumptions about what repo you are dealing with.  Thunderbird/MailNews Core/Calendar/SeaMonkey are all assumed to be in comm-central, everyone else is assumed to be in mozilla-central.  Patches against other repos (including mozilla 1.9.1) clearly will not work without additional logic (and some kind of extra info, like people putting “1.9.1” in their attachment descriptions.)

Here is a (fake) example of the “pretty” review output that is possible if you tell people about your reviewboard review (see it live here).  Although it says Bienvenu, it’s just me pretending to be him because his review queue is more interesting than mine.  The comments are accordingly mine.

review-board-review-pretty

Now, what does the export look like (see it live here)?

on file: mailnews/base/src/nsMessenger.cpp line 635
>     // if we have a mailbox:// url that points to an .eml file, we have to read
>     // the file size as well

what a pretty comment

on file: mailnews/base/src/nsMessenger.cpp line 642
>     NS_ENSURE_SUCCESS(rv, rv);

please rename rv ARRRRRR-VEEEEEE

Yeah, it looks like that.

A quick feature summary that explains why this is better than just looking at diffs in bugzilla:

  • Syntax highlighting!
  • It actually has the full-context of the rest of the file!  No more being limited to the 3 or 8 lines of context in the diff you are provided.  I know I have done a lazy review and let a bug through that would not have happened if I had more context at my fingertips (or was not sometimes lazy).
  • People just trimming down your patch to what they are commenting on leaves you with no context of what changed at all!

Useful links:

  • A more interesting live diff to check out.
  • The root of the review board that will prompt you to login via an Open ID account.  When syncing your review queue, please keep in mind that it can take a bit to do all the legwork and you won’t see any feedback until it is actually done doing everything else.  You should get some form of feedback no matter what happens, so don’t keep hitting refresh.
  • The hg repo for my modified version of review-board.  It’s based on an extremely shallow hgsvn checkout.  My questionable development strategy was to make changes with emacs locally, commit, then push to my VM, so the changesets are sometimes a bit excessive.

Caveat usor:

  • This is running on a linode VM right now.  This is better than my local box or what not, but it’s not Mo[MC]o IT or anything.
  • My changes, at least the export functionality, may be buggy.  You may need to rely on me to fix the export functionality to get your stuff out that way.  (If the hg diff doesn’t apply cleanly, you can’t enter data to lose it, so I’m less concerned about that.)
  • I have no plans to blow away the database, but at the same time, please be prepared for the possibility that space ninjas destroy your data.  Use the export functionality and save it to a text file or something periodically if you’re doing a major review.  (In case it’s not obvious, the export functionality is the text labeled “bugzilla-style export” to the right of the reviewer’s name at the top-left of each review.)  You can do your review in multiple passes, exporting each review pass individually.
  • I am confident something will go wrong.  Feel free to post comments here or ping me on IRC (:asuth).
  • If people actually try using this, I’ll stop developing on the live server, but do be aware that apache restarts (lasting a few seconds) may periodically happen, but this should not really impact anything.

Props:

using review board for bugzilla request queues / reviews

review-board-dashboard

Review board is a delightful django-based review tool.  Whereas bugzilla’s diff display is limited to taking the provided diff and coloring hunks without additional context, review board actually retrieves the underlying files from mercurial, applies the patch, colorizes the source files (using pygments), and thus can give you additional context.  Also, it allows you to click on line(s) and attach a note at that location.

Anywho, I made a new tool that ‘synchronizes’ bugzilla request queues with review board.  It finds the things in your request queue, and then creates a review in review board, transferring the patch.  There is some limited hard-coded biasing towards comm-central, but it’s arguably easily adaptable.  It uses a slightly hacked up version of the post-review script found in review board’s contrib directory to serve as the API to review board.

review-board-diffy-comment

The intent is to make it easier for people, or at least me, to do reviews.  I am definitely NOT proposing that the review “of record” live outside bugzilla.  But I think it might make it easier to prepare reviews in the first place.  For example, see the picture below?  It’s the result of my clicking on lines of code and making notes.  And those hyperlink-looking things that look like they might take you to the actual context, rather than manually having to try and determine context from people’s comments in the bugzilla notes?  They are actual hyperlinks that do what I just said!

review-board-review

If you are a mailnews hacker and would like to see your review queue in my reviewboard instance, ping me on IRC.  I may have already processed your queue and made you an account, or maybe not.  The review board API does not yet automate user creation and I got tired of doing things manually and gave up.

Warnings:

  • Things go awry on old-school patches.  For example, a patch that thinks it is against (the CVS layout of) “mozilla/mailnews” appears to not work.  The right answer is a patch against “a/mailnews” (I’m not sure if the git-style is essential).  Things worked fine for my queue, but some people with old stuff in their queues experienced glitches.  The script now inserts “problem!” into the summary when this happens, but in my reviewboard instance there may be some reviews that pre-date this logic.  (There should be a note in the description though.  Also, there will obviously be no diff attached.)
  • I initially forgot about the whole review=one person, super-review=another person, and maybe review=yet another person.  Although this is now fixed and reviews target all requested revieweres, there may be some reviews missing from your queue because they were created for one of the dudes who was not you and my manual nuking didn’t get to those.
  • Uh, it creates the reviews, but it doesn’t destroy them.
  • Also uh, there will be scaling issues if the reviews don’t eventually get closed out.