Ticket #130 (closed enhancement: fixed)
Better support for git
| Reported by: | hskinnemoen | Owned by: | hskinnemoen |
|---|---|---|---|
| Priority: | major | Milestone: | 0.7.7 |
| Version: | 0.7.6 | Keywords: | git |
| Cc: |
Description
This patch aims to improve the support for git in BuildBot. More specifically:
- Use git, not cogito
- Support checking out or updating to a specific branch
- Support checking out or updating to a specific revision (SHA1)
- Implement parseGotRevision() on the slave side
- Implement computeSourceRevision() on the master side
- Implement GitExtractor? class in tryclient
- Add testcases
- Add post-receive hook (contrib/git_buildbot.py)
- Update documentation
Attachments
Change History
comment:2 Changed 6 years ago by warner
- Owner set to hskinnemoen
- Milestone changed from undecided to 0.7.7
Wow! What an amazing patch! So complete!
One thing I need though.. it looks like you're using tabs for indentation, but the rest of the buildbot code uses purely spaces, and of course python warns you a lot if you mix tabs and spaces. If you can do two small things I'll drop this patch in right away:
- run 'expand' or something on your tree so it has spaces instead of tabs. (I'd do this myself but I'm slightly afraid of breaking something, and I figure you'll be able to spot any problems more reliably than me)
- fix the minor typo in buildbot/slave/commands.py that adds the string 'URL' in the middle of an unrelated comment
other than that, it looks great.. thanks!
-Brian
comment:3 Changed 6 years ago by hskinnemoen
Ok, done. I ran all the modified files through expand, and the patch grew three unrelated changes. Two of them looked trivial, so I left them in there, but the last one changed something that looked like CVS output so I removed it from the patch.
I've inspected the result but not tested it.
comment:4 Changed 6 years ago by warner
- Status changed from new to closed
- Resolution set to fixed
Applied in [bc53d760b75d89299c879e08f65c4fd99ad5506e]. Tests pass. Thanks again for the amazing patch!
comment:5 Changed 6 years ago by warner
- Status changed from closed to reopened
- Resolution fixed deleted
oops, buildbot is red. It looks like the buildslaves have an older version of git (1.1.3) that doesn't recognize the 'git init' command. The error text in the logs shows a list of "git commands available in /usr/bin" and includes "init-db" but not "init".
Am I safe in assuming that 1.1.3 is so inconceivably old that nobody uses it anymore? I think the buildslaves are running ubuntu Dapper. Do we want to change the buildbot code to handle the older git, or document that you must be running git version X or newer to use this support? If we do the latter, I can install a newer version on the buildslave, but there are a lot of dapper machines out there that would be nice to support..
comment:6 Changed 5 years ago by warner
I added code to skip the Git unit tests if the version of Git installed is older than 1.2.x . I have no idea what the actual borderline is.. it'd be nice to get some buildslaves with a variety of distributions so we can discover i.e. dapper is too old but edgy is ok.
If someone could look more closely at the git support and figure out what's the oldest version of /usr/bin/git it can handle is, I'd be grateful. It would also be wonderful to support older versions of git somehow. Doing this sort of thing usually involves looking through the Git release notes (and building a variety of old versions) and running 'trial buildbot.test.test_vc.Git' against each one.
![[Buildbot Logo]](/chrome/site/header-text-transparent.png)

There are a couple of remaining issues I've found after I did the original patch.
One is that buildbot seems to throw away the working repository and do a new clone when switching branch. That can be a bit time-consuming when working on large repositories like the Linux kernel.
The other is more serious: When using the git_buildbot.py as a post-receive hook it gets in trouble when pushing lots of changes all at once. For example, when rebasing a branch on top of the latest upstream branch during the Linux kernel merge window (which added several thousand changes to the branch), it stalled for about a minute and then aborted because it hit a recursion limit.
I think it might be better to send the branch update as a single change. The downside is that the original authors of the changes are lost (there may be lots of them), but other properties like the list of files that have been changed are easy to extract. We may of course hit other limits this way -- more than nine thousand files were changed in the two weeks between Linux 2.6.23 and 2.6.24-rc1.