[tahoe-dev] Weekly Dev Hangout 2012-09-18

[Brian Warner]

On 9/19/12 4:09 PM, David-Sarah Hopwood wrote:
> On 19/09/12 22:06, Zooko Wilcox-OHearn wrote:

>> • David-Sarah is happy with Brian's design of the state machine.
> 
> Nitpick: that was a joint design between Brian and me.

Nitnitpickpick: that was almost entirely David-Sarah's design. I flubbed
the first one, D-S studied the issue properly and made a real design.
I'm grateful for the detailed analysis.

>> Git workflow:
>>
>> 1. contributor submits patches in a github pull request, github
>> auto-generates the merge to current master
>>
>> 2. human check (to discourage abuse), human chooses whether to add a
>> comment to the revision (not to the pull request) with the magic
>> string "Buildbot: GO!"
> 
> There was a discussion of whether the string needed to include a
> commithash (so that the branch owner cannot add a commit after the
> human review and before the build). We didn't come to a conclusion in
> the call, but I think it should. Supporting that would also allow more
> easily testing a revision that is not the head of a side-branch.

One of the questions was how easy it'd be for developers to express
their review.. making it fit comfortably into their workflow. Adding a
comment on a github pull-request page is quick and easy, but doesn't
necessarily nail down a specific revision, making it possible for a
contributor to do a "bait-and-switch" sort of attack where they replace
the pull-request branch with something malicious at just the right
moment, and then get control of the buildslaves.

Having the reviewer include a hash might involve a lot of extra clicking
and cut-and-pasting. OTOH, maybe we could build a browser plugin that
automates some of that. OT3H, I'm not so worried about bait-and-switch,
like zooko said I think there are other incentives and auditing tools
that may be enough to discourage such behavior.

>> 3. buildbot gets from github the merged-pull-request code
> 
> ... or does the merge itself. That might be easier if we allow
> specifying the commithash (since the merge with master is then
> not necessarily the same as the one computed by github from the
> branch head).

Yeah.. we'd need to deal with the consequences of a failed merge (which
might leave partially-applied patches lying around, requiring a clobber
build), whereas if we delegate that test to github, we probably don't.

It does raise the question of how this super-duper CI system we're
proposing to build should deal with the evolution of trunk over time
(the moving-target of the proposed merge). I suspect that github
recomputes the current merge target each time someone visits the
pull-request's web page, and maybe more frequently (like when the
contributor updates their branch). It'd be cool if our bot could help
maintain the proposed submission, keeping it up to date and meeting our
style goals:

* does it currently merge cleanly to master?
* should it be rebased to current master? how "bushy" would the commit
  history be if it were landed without rebasing?
* do the merged tests pass?
* would it cause a performance regression?
* has a committer reviewed the proposed delta for buildbot safety?
 * if so, maybe the bot could auto-rebase and test the result without
   additional review
* has a committer approved the delta for landing?

There's a bit of tension between github's native workflow (adding
comments to revisions+code in pull requests) and my personal tastes
about having a tidy tree (rebasing, rewriting commits to tell a better
story), because those rewrites can orphan the comments. I don't yet know
how to manage this.


cheers,
 -Brian