Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in master/buildbot/db/sourcestamps.py #1289

Closed
wants to merge 1 commit into from

Conversation

delanne
Copy link
Contributor

@delanne delanne commented Oct 24, 2014

Dustin says:

<djmitche> you only get multiple changes attached to a sourcestamp if you have a long enough treeStableTimer that the schedule deals with both changes at the same time
<djmitche> asking "which changes led to this sourcestamp", translated to, say, git terms means "which commits led to this revision"
<djmitche> to which the only logical answer is "all of them"
<djmitche> what we really want is "what are the commits *between* this sourcestamp and that sourcestsamp"
<djmitche> so a sourcestamp will have a single change (the commit that generated it)
<djmitche> and we'll do some tree traversal to answer theq uesetion
<djmitche> but until that happens, it's this kind of crappy "it depends what the scheduler sees" answer :(

But with the current implementation, a sourcestamp is created for each change, because "revision" (GIT SHA1 in case of the gitpoller) is used to calculate the ss_hash.
As a consequence, buildbot creates a new sourcestamp for each new changes.

```
<djmitche> you only get multiple changes attached to a sourcestamp if you have a long enough treeStableTimer that the schedule deals with both changes at the same time
<djmitche> asking "which changes led to this sourcestamp", translated to, say, git terms means "which commits led to this revision"
<djmitche> to which the only logical answer is "all of them"
<djmitche> what we really want is "what are the commits *between* this sourcestamp and that sourcestsamp"
<djmitche> so a sourcestamp will have a single change (the commit that generated it)
<djmitche> and we'll do some tree traversal to answer theq uesetion
<djmitche> but until that happens, it's this kind of crappy "it depends what the scheduler sees" answer :(
```

But with the current implementation, a sourcestamp is created for each change, because "revision" (GIT SHA1 in case of the gitpoller) is used to calculate the ss_hash.
As a consequence, buildbot creates a new sourcestamp for each new changes.
@djmitche
Copy link
Member

This fix would only ever create one sourcestamp for each repository, branch, codebase, project, and patchid. Which means you'd always build exactly the same revision. Which is probably not what you or anyone wants!

@delanne
Copy link
Contributor Author

delanne commented Oct 24, 2014

<djmitche> ah, yes, this was partway along to the new way
<djmitche> your fix just means that there will only be one sourcestamp per branch
<djmitche> which isn't really what you want :)
<djmitche> I assume what you want is a proper blamelist?
* bb-github (~bb-github@192.30.252.33) has joined #buildbot
<bb-github> [buildbot] djmitche comment on issue #1289: This fix would only ever create one sourcestamp for each repository, branch, codebase, project, and patchid.  Which means you'd always build exactly the same revision.  Which is probably not what you or anyone wants! http://git.io/vpwXXA
* bb-github (~bb-github@192.30.252.33) has left #buildbot
<delanne> djmitche, so  PR1289 must update sourcestamps.revision in order to checkout the correct revision 
<delanne> eg the revision of the "latest" change
<delanne> right ?
<djmitche> 'update'?
<djmitche> sourcestamps don't change after they're created
<djmitche> and you'd still only have one sourcestamp
<djmitche> so all your builds would say "I built r1234" even if they atually built r1200
<delanne> I don't understand. how a sourcestamp can contain several changes in this case ?
<djmitche> it can't
<djmitche> a sourcestamp specifies a particular version of the source
<djmitche> like a tree object in git
<djmitche> it doesn't contain *any* changes
<delanne> so, currently we have no way to know which changes are included in a build ?
<djmitche> there's a change that *created* a sourcestamp
<djmitche> similar to a commit object in git
<delanne> and you said that multiple changes are attached to a sourcestamp if you have a long enough treeStableTimer ...
<djmitche> I was wrong - I forgot this was changed in nine
<djmitche> so "what changes are included in a build" is actually a function of *two* builds -- this one and the previous
<djmitche> think of "what changes are included in a build" as "git log"
<djmitche> if you just give it one revision, it gives you all changes from the first commit to that one
<djmitche> to get a limited set of commits, you need to give it two revisions
<djmitche> what's missing right now in Buildbot is a 'parent' pointer for changes
<djmitche> with that in place, the MailNotifier, etc. could calculate the equivalent of 'git log last_build..this_build'
<delanne> ok. makes sense.
 Jc2k jemand_pribyl` jklontz jochen__ jollyroger JStoker jwdevel 
<delanne> thanks djmitche 
<djmitche> it sucks that it's in an in-between state
``

@delanne delanne closed this Oct 24, 2014
@delanne delanne deleted the dbsourcestampsFix branch October 24, 2014 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants