Needless to say, having a Chromium slave on build.webkit.org would be pretty useful to WebKit committers. The problem is that we are still quite a ways away from being able a fully functional Chromium WebKit slave on build.webkit.org (see bug 28398 for details). I wonder if we could compromise here by temporarily hooking up an existing pre-configured Chromium slave (http://src.chromium.org/viewvc/chrome/trunk/tools/buildbot/) and modifying WebKit master to feed it Chromiumey commands? Although not a replacement for the proper build slave, this solution allows us to gain immediate benefit of observing the breakage as it happens, right there on the waterfall, rather than having to look at http://tinyurl.com/webkit-canary or worse yet, letting Chromium people put out the fires. What do you think? Patch coming up.
Created attachment 34979 [details] Add canary-style Chromium build slave, v1. .../build.webkit.org-config/config.json | 9 ++++- .../build.webkit.org-config/master.cfg | 37 +++++++++++++++++++- WebKitTools/ChangeLog | 10 +++++ 3 files changed, 54 insertions(+), 2 deletions(-)
Comment on attachment 34979 [details] Add canary-style Chromium build slave, v1. Has this been tested? 283 if platform == "chromium": 284 self.addStep(UpdateChromiumSource) I don't see how platform can ever be equal to "chromium" here, which would suggest that none of the Chromium-specific bits would ever be executed. Where is the output from this build placed? Does it end up below the WebKit source root? I'm still not particularly fond of adding all this Chromium-specific gunk to what is otherwise a cross-platform setup. What is the ETA for getting Chromium building in the same manner as every other WebKit port?
Comment on attachment 34979 [details] Add canary-style Chromium build slave, v1. I apologize. I will set up a master and test this.
(In reply to comment #2) > I'm still not particularly fond of adding all this Chromium-specific gunk to > what is otherwise a cross-platform setup. What is the ETA for getting Chromium > building in the same manner as every other WebKit port? I am trying to lead this effort to completion in the next few months. I don't have a specific date, but it is "as soon as possible" in my book. It's not a good situation for the Chromium folks, either.
Created attachment 35064 [details] Add canary-style Chromium build slave, v2. .../build.webkit.org-config/config.json | 12 ++++- .../build.webkit.org-config/master.cfg | 48 ++++++++++++++++++++ WebKitTools/ChangeLog | 10 ++++ 3 files changed, 68 insertions(+), 2 deletions(-)
Comment on attachment 35064 [details] Add canary-style Chromium build slave, v2. I tested this one :). Can you take another look?
Comment on attachment 35064 [details] Add canary-style Chromium build slave, v2. Will fix issues discussed on #webkit, new patch coming up.
Created attachment 35083 [details] Add canary-style Chromium build slave, v3. .../build.webkit.org-config/config.json | 12 ++++- .../build.webkit.org-config/master.cfg | 51 ++++++++++++++++++++ WebKitTools/ChangeLog | 10 ++++ 3 files changed, 71 insertions(+), 2 deletions(-)
Comment on attachment 35083 [details] Add canary-style Chromium build slave, v3. Addressed #webkit comments: * Changed builder to "ChromiumBuild" * Escaped backslash * Added revision-bound gclient sync
Comment on attachment 35083 [details] Add canary-style Chromium build slave, v3. > diff --git a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg > index 4d92436..6ad9143 100644 > --- a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg > +++ b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg > @@ -47,6 +47,48 @@ class CheckOutSource(source.SVN): > source.SVN.__init__(self, baseURL=self.baseURL, defaultBranch="trunk", mode=self.mode, *args, **kwargs) > > > +# FIXME: Remove this step once Chromium WebKit port build system is decoupled from > +# Chromium (https://bugs.webkit.org/show_bug.cgi?id=28396) > +class UpdateChromiumSource(shell.ShellCommand): > + command = ["gclient", "sync"] > + name = "update-chromium" > + description = ["updating chromium source"] > + descriptionDone = ["updated chromium source"] > + haltOnFailure = True The descriptionDone should really include the SVN revision number so that it is clear from the waterfall page which revision was built. The SVN status step uses a descriptionDone of the form ["updated", "rXXXX"] where XXXX is the value of got_revision (the revision that the "svn update" ended up pulling). > + > + def start(self): > + os = self.getProperty("fullPlatform").split('-')[1] > + if os == "win": > + self.command[0] = "gclient.bat" > + revision = self.getProperty("revision") > + if revision: > + self.command.append("--revision=src/third_party/WebKit@" + revision) We should be using ShellCommand.setCommand like is done elsewhere in the file rather than mutating self.command directly. In theory the step should also be setting up got_revision as other source-fetching steps do. It'll be needed for the descriptionDone property as mentioned above. > +# FIXME: Remove this step once Chromium WebKit port build system is decoupled from > +# Chromium (https://bugs.webkit.org/show_bug.cgi?id=28396) > +class CompileChromiumWebKit(shell.ShellCommand): > + command = ["python", "../../../scripts/slave/compile.py"] > + name = "build-chromium" > + description = ["compiling"] > + descriptionDone = ["compiled"] > + haltOnFailure = True > + > + def start(self): > + os = self.getProperty("fullPlatform").split('-')[1] > + command = self.command > + if not self.getProperty("revision"): > + # Buildbot will set revision to None when a build is forced. Use it > + # as a signal to clobber. > + command.append("--clobber") > + if os == "win": > + command.extend(["--solution=webkit.sln", "--build-dir=src\\webkit", "--", "/project", "webcore"]) > + elif os == "mac": > + command.extend(["--solution=__solution__", "--build-dir=src/build", "--", "-project", "../webkit/webkit.xcodeproj", "-target", "webcore"]) Same comment as above about self.command.
(In reply to comment #10) > The descriptionDone should really include the SVN revision number so that it is > clear from the waterfall page which revision was built. The SVN status step > uses a descriptionDone of the form ["updated", "rXXXX"] where XXXX is the value > of got_revision (the revision that the "svn update" ended up pulling). Let me see how I can figure this out. I don't know if I can, but will try. > > > + > > + def start(self): > > + os = self.getProperty("fullPlatform").split('-')[1] > > + if os == "win": > > + self.command[0] = "gclient.bat" > > + revision = self.getProperty("revision") > > + if revision: > > + self.command.append("--revision=src/third_party/WebKit@" + revision) > > We should be using ShellCommand.setCommand like is done elsewhere in the file > rather than mutating self.command directly. In theory the step should also be > setting up got_revision as other source-fetching steps do. It'll be needed for > the descriptionDone property as mentioned above. > > > +# FIXME: Remove this step once Chromium WebKit port build system is decoupled from > > +# Chromium (https://bugs.webkit.org/show_bug.cgi?id=28396) > > +class CompileChromiumWebKit(shell.ShellCommand): > > + command = ["python", "../../../scripts/slave/compile.py"] > > + name = "build-chromium" > > + description = ["compiling"] > > + descriptionDone = ["compiled"] > > + haltOnFailure = True > > + > > + def start(self): > > + os = self.getProperty("fullPlatform").split('-')[1] > > + command = self.command > > + if not self.getProperty("revision"): > > + # Buildbot will set revision to None when a build is forced. Use it > > + # as a signal to clobber. > > + command.append("--clobber") > > + if os == "win": > > + command.extend(["--solution=webkit.sln", "--build-dir=src\\webkit", "--", "/project", "webcore"]) > > + elif os == "mac": > > + command.extend(["--solution=__solution__", "--build-dir=src/build", "--", "-project", "../webkit/webkit.xcodeproj", "-target", "webcore"]) > > Same comment as above about self.command. Thanks! Will fix.
Comment on attachment 35083 [details] Add canary-style Chromium build slave, v3. New patch coming up ...
Created attachment 35158 [details] Add canary-style Chromium build slave, v4. .../build.webkit.org-config/config.json | 12 +++- .../build.webkit.org-config/master.cfg | 63 ++++++++++++++++++++ WebKitTools/ChangeLog | 10 +++ 3 files changed, 83 insertions(+), 2 deletions(-)
Comment on attachment 35158 [details] Add canary-style Chromium build slave, v4. Comments addressed. Took me a while to get this one right. I also have a variation that doesn't use the super-fugly regex, just startswith and line loop.
From the blackout: --- Comment #15 from Eric Seidel <eric@webkit.org> 2009-08-20 10:38:50 PDT --- (From update of attachment 35158 [details]) I wonder if the master shouldn't just import some chromium module with these hacks in it, instead of inlining them. Anyway, looks sane enough to me, but you need a review from bdash. Does buildbot really uses non-standard python style? 58 haltOnFailure = True How about here? 63 got_revision = "??" # This matches SVN unknown revision response No way to avoid the ../../..? 83 command = ["python", "../../../scripts/slave/compile.py"] So all force builds should be clobber builds? I thought buildbot had a clobber option? Or maybe that's just chromium's special buildbot. This should be inside some sort of "build-chromium" script instead of in ever thing that wants to call compile.py: if os == "win": 97 command.extend(["--solution=webkit.sln", "--build-dir=src\\webkit", "--", "/project", "webcore"]) 98 elif os == "mac": 99 command.extend(["--solution=__solution__", "--build-dir=src/build", "--", "-project", "../webkit/webkit.xcodeproj", "-target", "webcore"]) I gues it's becasue we're trying to build only WebCore...? Sad: " ".join(architectures) Again, looks OK to me. bdash is your man.
> Does buildbot really uses non-standard python style? > 58 haltOnFailure = True yup. > How about here? > 63 got_revision = "??" # This matches SVN unknown revision response Well, the naming here is to match the name of the property. But I can rename that. > No way to avoid the ../../..? > 83 command = ["python", "../../../scripts/slave/compile.py"] Not without more angst :) > So all force builds should be clobber builds? I thought buildbot had a clobber > option? Or maybe that's just chromium's special buildbot. Yup. It's something nsylvain bolted on to build.chromium.org. > This should be inside some sort of "build-chromium" script instead of in ever > thing that wants to call compile.py: > if os == "win": > 97 command.extend(["--solution=webkit.sln", > "--build-dir=src\\webkit", "--", "/project", "webcore"]) > 98 elif os == "mac": > 99 command.extend(["--solution=__solution__", > "--build-dir=src/build", "--", "-project", "../webkit/webkit.xcodeproj", > "-target", "webcore"]) > > I gues it's becasue we're trying to build only WebCore...? Yes, and I really want to minimize the customness here. This will be going away in a matter of months. > Sad: > " ".join(architectures) > > Again, looks OK to me. bdash is your man.
How are we doing here? :)
Comment on attachment 35158 [details] Add canary-style Chromium build slave, v4. > diff --git a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg > index 4d92436..5af0698 100644 > --- a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg > +++ b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/master.cfg > @@ -14,6 +14,7 @@ from buildbot.status.builder import SUCCESS, FAILURE, WARNINGS, SKIPPED > from twisted.internet import defer > > import simplejson > +import re This should be alphabetical. > + def createSummary(self, log): > + scraper = re.compile(r"^________ running '[^\n]+third_party[/\\]WebKit[^\n]+$\n(?:^[UA]\W+[^\n]+$\n)*^(?:Updated to|At) revision (\d+)", re.DOTALL | re.MULTILINE) > + revs = scraper.findall(log.getText()) > + got_revision = "??" # This matches SVN unknown revision response These two variable names don't follow our coding style. "revs" should presumably be "revisions". "got_revision" should be "gotRevision".. > + if len(revs): > + got_revision = ("r%s" % revs[-1]) The parens are unnecessary here. > + def start(self): > + os = self.getProperty("fullPlatform").split('-')[1] > + command = self.command[:] > + if not self.getProperty("revision"): > + # Buildbot will set revision to None when a build is forced. Use it > + # as a signal to clobber. > + command.append("--clobber") This introduces a difference in behavior from the other configurations. While it may make sense to add this behavior, we shouldn't be doing it in such a way that it is specific to a single configuration. r=me
Landed as http://trac.webkit.org/changeset/47787.
Clobber option removal landed as http://trac.webkit.org/changeset/47789