[Buildbot-devel] Build priority functions & Change processing order

[Jorge Gonzalez]

Turns out that sorting the Changes chronologically right before
incorporating them to a SourceStamp remedies everything I described below.
Sent a patch for review https://github.com/buildbot/buildbot/pull/459

Jorge


On Thu, Jun 28, 2012 at 11:54 AM, Jorge Gonzalez <gjorge at google.com> wrote:

> Wouldn't it make sense for builders' customizable *nextBuild* function to
> be a comparator that allows one to define a total order for the set
> candidate of build requests, instead of selection function as it currently
> is?
>
> I ask this because I run into some oddities when providing an
> implementation that returns the "newest" build request. Here's what I saw:
>
>    - *nextBuild *function is called with a sequence of build requests B,
>    where B[x] was submitted before B[x+1]
>    - say my implementation of *nextBuild* always returns the newest
>    request first, so upon receiving say B=[br1, br2, br3], it returns br3
>    - upon receiving br3 from my function, Builder._mergeRequests() will
>    bring br3 at the front of the sequence, so now B'=[br3, br1, *br2*]
>    (we assume br1 & br2 can be merged with br3). At this point requests in B'
>    are not sorted chronologically anymore, yet...
>    - in order to build a single SourceStamp for the Build,
>    Build.__init__() merges all B' requests' SourceStamps into a single and new
>    SourceStamp, but...
>    - SourceStamp.__init__(), which receives a list of Changes C' sorted
>    in the same order their associated requests are in B', assumes those
>    Changes are sorted chronologically, and tries to take the last Change's
>    revision, branch, etc as its main attributes. In this fictitious example,
>    that would would mean br2's last Change is used, which is NOT the most
>    recent change.
>    - Similarly, Build.setupProperties() will process C' sequentially,
>    which means that if there's any property name that's common across all
>    Changes, br2' last Change's properties will take precedence over all others
>    by virtue of being the last one processed.
>
> This yields a very odd looking build details page, where br3 is listed as
> the first Change but where all build properties originated from a Change we
> contributed by br2, the last one in the list.
>
> My proposal would be twofold:
>
>    - have *nextBuild* be a comparator so that the list of build requests
>    (and associated changes) are treated in order prescribed by it
>    - within SourceStamp.__init__() and Build.setupProperties() sort
>    Changes by say Change.when before traversing them, to guarantee their
>    chronological order.
>
>
> Jorge
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://buildbot.net/pipermail/devel/attachments/20120628/994ef360/attachment.html>