Ticket #959 (closed enhancement: fixed)
Add 'native' CVS support
| Reported by: | AHowell | Owned by: | AHowell |
|---|---|---|---|
| Priority: | major | Milestone: | 0.8.2 |
| Version: | master | Keywords: | cvs |
| Cc: |
Description
Add support for CVS with script in contrib. I had problems finding and/or making the methods in the docs work.
I will submit scripts and docs to do this. A script is called by CVS at checkin. This generates a mail the is picked up by a new source in changes/mail.py. The source has a hook to add URLs for the changed files. It also adds a property to the Change that lists both the files and their old and new versions. I'm not sure if that should stay in there or not.
Change History
comment:1 Changed 3 years ago by dustin
- Keywords cvs added
- Milestone changed from undecided to 0.8.2
comment:2 Changed 3 years ago by AHowell
Sure, sorry that was not very descriptive. I had problems tracking down a working change notification method.
FreshCVS - Could not get this going at all, project dead? Bonsai - Tried this awhile ago, can't remember why I didn't use it. syncmail - Works, but could not determine repo. repository not set. No updates since 05
I sent some mail on it previously. http://sourceforge.net/mailarchive/message.php?msg_name=4C345020.3010903%40austin.rr.com
By 'native', I meant CVS support that is shipped with buildbot. With in the distribution, its more likely to be supported in the future. It also makes it easier to deploy buildbot, as its one less external package to track down. The changes to the docs have more detail how it works etc. I'm happy to elaborate if it doesn't make sense. Thanks.
comment:3 Changed 3 years ago by dustin
Thanks - that's exactly what I was looking for. Let's remove the cruft while we're at it.
The new stuff should have tests, too -- and tests that don't depend on an actual CVS installation. I've done a bit of work to that end with the slave-side source tests, if you want to have a look.
comment:4 Changed 3 years ago by AHowell
De-cruffted and pushed cvsmailsouce branch
The patch involves files under master/builbot
changes/mail.py
-- new class BuildbotCVSMaildirSource
contrib/buildbot_cvs_mail.py
-- submits mail with changes
test/unit/test_contrib_buildbot_cvs_mail.py
-- test contrib/buildbot_cvs_mail.py
test/unit/test_changes_mail_MailDirSource.py
-- test new BuildbotCVSMaildirSource class
Under master/docs
cfg-changesources.texinfo
-- Explain how to install and use above
comment:5 Changed 3 years ago by AHowell
Dustin, you said: "Let's remove the cruft while we're at it."
I took that to mean removing my unused code (fileTupleList) in mail.py. Were you referring to the other cvs MaildirSources?? I don't know which, if any, are in current use. I'd be hesitant to remove/change them.
comment:6 Changed 3 years ago by dustin
I was referring to the other, no-longer-maintained stuff, on the basis that it's just extra code we're trying to maintain. But maybe it's best to just leave it.
Is there anything else you're planning to do here? Thanks!
comment:7 Changed 3 years ago by AHowell
Maybe the approach take is to mark the existing CVS MailDirSources? as legacy in the docs and release notes, saying they may be removed in a future release. If thats acceptable, I'll modify the docs.
I think my code is functionally complete for most needs. The two attributes of a Change that it does not set are 'revlink' and 'properties'. 'revlink' does not doesn't make sense for CVS since it does not have an id for a commit. I get around that by having a user supplied function to map each file name, old CVS revision, new CVS revision to a URL. Hmm, I should have called it 'urlmapper' instead of 'urlmaker'.
Perhaps I should add the 'properties' attribute, so they can be specified as part of a BuildbotCVSMaildirSource. 'properties' in this case is a dictionary, not a Properties class. That is the only tweak I can think of.
comment:8 Changed 3 years ago by dustin
I think that a deprecation is a good idea. Please also add it to the NEWS section, and ask users of the old class to contact the mailing list if they would like to see the old classes stick around.
As for Change attributes - 'properties' would be a good addition, and as far as I can see 'repository' is not set, either.
Before you work on that, though, I'm working on some changes that I have pushed to
In particular:
- rename BuildbotCVSMaildirSource to CVSMaildirSource (since the "Buildbot" part is redundant within Buildbot itself)
- use Python's optparse (available since python-2.3) instead of manually parsing arguments
At the moment, the tests are passing even though the optparse stuff fails, so I need to figure out what's wrong with the tests, and then fix the optparsing.
comment:9 Changed 3 years ago by AHowell
Thanks for fixing the option parsing. I'd do the grunt work if you pointed me in the right direction. :)
Should I wait for the test fixes?
I need some git hand-holding. How do I update my cvsmailsource branch? I tried doing a fetch:
git fetch --dry-run http://github.com/djmitche/buildbot/commits/bug959
fatal: http://github.com/djmitche/buildbot/commits/bug959/info/refs not found: did you run git update-server-info on the server?
Thanks
comment:10 Changed 3 years ago by dustin
OK, I'm done with the changes on my branch, so you can take it back. As for fetching from a remote repository, there are a few small things you're getting wrong. First, the URL you're using is a regular HTML page - you want to use git://github.com/djmitche/buildbot.git instead. Second, fetch is designed to bring a 'remote' up to date, and that might be overkill. Here's how you would set up that remote, though:
git remote add djmitche git://github.com/djmitche/buildbot.git git fetch djmitche
However, since all you want to do is pull my changes into your branch, you can just do this:
git pull git://github.com/djmitche/buildbot.git bug959
No fuss, no mas. The command will do a fast-forward in this particular case - meaning it just appends my commits to your commits - but if necessary it will perform a merge. This is, in fact, how I pull others' buildbot patches into the master repository.
comment:11 Changed 3 years ago by AHowell
Thanks a bunch. I've pushed my changes. I added a number of tests to the both the mail.py side and buildbot_cvs_mail.py unit tests.
I've updated the docs, adding note about the depreciation. I noticed that the mozilla people are using bonsai, but it looks like they are using tinderbox for the builds. Not sure if we should depreciate bonsai support. A long time ago I had a tinderbox setup here. When the box it was on died, I couldn't find updated source for it, so I moved to buildbot! Every day I can avoid perl is a good day.
I wasn't sure about your comment in buildbot_cvs_mail.py: # TODO: make sure tests really run
Regarding the setting of the 'repository', that is set by the --cvsroot option.
comment:12 Changed 3 years ago by Dustin J. Mitchell
- Status changed from new to closed
- Resolution set to fixed
Merge branch 'cvsmailsource' of git://github.com/AndyHowell/buildbot
Fixes #959
- 'cvsmailsource' of git://github.com/AndyHowell/buildbot: option.smtplib was not getting when running live Added depreciation notes to news and manual Added more test to test_changes_mail_CVSMaildirSource.py and test_contrib_buildbot_cvs_mail.py Fixed mail.py problems brought out by tests use optparse for buildbot_cvs_mail; fix its tests rename to CVSMaildirSource fix relative path handling in buildbot_cvs_mail tests docs tweaks for cvs support buildbot/changes/mail.py buildbot/test/unit/test_changes_mail_MaildirSource.py removed fileTupleList as it was not being used master/buildbot/changes/mail.py master/contrib/buildbot_cvs_mail.py Replaced tabs with spaces master/buildbot/changes/mail.py added BuildbotCVSMaildir source master/buildbot/test/unit/test_changes_mail_MaildirSource.py test for BuildbotCVSMaildir in mail.py master/buildbot/test/unit/test_contrib_buildbot_cvs_mail.py test for buildbot_cvs_mail.py master/contrib/buildbot_cvs_mail.py send mail to buildbot on CVS checkins master/docs/cfg-changesources.texinfo updated docs Fixes #951. Web rss feed is choking on UTF-8 log data Changeset: b9feff0338ecd7cceead5ede70852abc1f0a6d2a
comment:13 Changed 3 years ago by dustin
Committed, as you can see. I'll remove that comment - it was a brain-dump when I got up from the keyboard.
comment:14 Changed 3 years ago by AHowell
Thanks! I forgot to mention I added the ability to set properties in CVSMaildirSource. I saw that you made the retrieving of the user name failsafe. The buildbot_cvs_mail.py script won't run on windows, as it forks for sending the mail. The forks so that CVS can continue on. I don't have much expertise in windows, nor I don't imagine there are many users running the CVS repository under windows either; I'm leaving it as is for now.
The other 'deficiency' with it stems from the fact that when you check in files from multiple directories, each directory generates a separate mail. This shows up in the waterfall as multiple changes, each with the same comment, but different list of files, and possible a slightly different revision time. Annoying but usable.
After my short experience with GIT, I'm ready to switch to that. I'm stuck with CVS though, so hopefully this will help others in a similar situation. Thanks for all all your help adding this.
comment:15 Changed 3 years ago by dustin
Yes, I needed to get the tests to pass on Windows. I think smtplib actually uses smtp, rather than forking, so it should work. But you're right - running a CVS master on windows is rare, if it ever happens :)
As for CVS's inability to define a single "change" properly, that's life..
Git is pretty cool, for sure!
![[Buildbot Logo]](/chrome/site/header-text-transparent.png)
I'm not sure what "native" means here -- can you tell me more about what is inadequate in the current setup? Is it just documentation?
I've absolutely no opposition to improving things - I just want to be clear on the goal :)