Last Comment Bug 432236 - Clobber support for mozilla-central depend builds and unit tests
: Clobber support for mozilla-central depend builds and unit tests
Status: RESOLVED FIXED
:
Product: Release Engineering
Classification: Other
Component: Other (show other bugs)
: other
: All All
: P3 normal (vote)
: ---
Assigned To: Chris AtLee [:catlee]
:
Mentors:
: 393417 431529 436449 (view as bug list)
Depends on: 445309 447827
Blocks: 433384
  Show dependency treegraph
 
Reported: 2008-05-05 08:46 PDT by Benjamin Smedberg [:bsmedberg]
Modified: 2013-08-12 21:54 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Rough clobber support (1.18 KB, patch)
2008-05-28 14:04 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details | Diff | Splinter Review
[checked in] ugly, basic front-end for clobbering (2.08 KB, patch)
2008-07-16 06:29 PDT, Ben Hearsum (:bhearsum)
benjamin: review+
Details | Diff | Splinter Review
clobber buildstep - forced clobbers + periodic (2.64 KB, patch)
2008-07-16 06:37 PDT, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
address nick's review comments (2.90 KB, patch)
2008-07-18 09:23 PDT, Ben Hearsum (:bhearsum)
nthomas: review+
Details | Diff | Splinter Review
master.cfg changes for clobbers (3.75 KB, patch)
2008-07-21 07:58 PDT, Ben Hearsum (:bhearsum)
coop: review+
Details | Diff | Splinter Review
[checked in] Clobberer BuildStep as landed, addresses Nick's last review comments (2.96 KB, patch)
2008-07-22 07:52 PDT, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
[checked in] clobbering code in a stand-alone script (2.16 KB, patch)
2008-08-05 12:32 PDT, Ben Hearsum (:bhearsum)
nthomas: review+
Details | Diff | Splinter Review
master.cfg w/ external clobber script (5.49 KB, patch)
2008-08-05 13:06 PDT, Ben Hearsum (:bhearsum)
nthomas: review+
Details | Diff | Splinter Review
master.cfg v3 - addresses Nick's comments (5.48 KB, patch)
2008-08-06 04:40 PDT, Ben Hearsum (:bhearsum)
nthomas: review+
Details | Diff | Splinter Review
updated front-end and clobberer.py script (12.88 KB, patch)
2009-01-27 09:04 PST, Chris AtLee [:catlee]
bhearsum: review+
Details | Diff | Splinter Review
Add clobberURL to MozillaBuildFactory (2.25 KB, patch)
2009-01-27 09:13 PST, Chris AtLee [:catlee]
bhearsum: review+
Details | Diff | Splinter Review
Updates to mozilla2-staging to set clobber URL (3.95 KB, patch)
2009-01-27 09:25 PST, Chris AtLee [:catlee]
bhearsum: review+
Details | Diff | Splinter Review
updated front-end and clobberer.py script (14.61 KB, patch)
2009-01-28 09:40 PST, Chris AtLee [:catlee]
bhearsum: review+
Details | Diff | Splinter Review
Add clobberURL and clobberTime to MozillaBuildFactory (2.29 KB, patch)
2009-02-05 06:02 PST, Chris AtLee [:catlee]
bhearsum: review+
Details | Diff | Splinter Review
updated front-end and clobberer.py script (69.34 KB, patch)
2009-02-05 06:34 PST, Chris AtLee [:catlee]
bhearsum: review+
Details | Diff | Splinter Review
Updates to mozilla2-staging to set clobber URL and clobber time (4.82 KB, patch)
2009-02-05 07:19 PST, Chris AtLee [:catlee]
bhearsum: review+
catlee: checked‑in+
Details | Diff | Splinter Review
Add clobberURL and clobberTime to MozillaBuildFactory (2.54 KB, patch)
2009-02-05 10:40 PST, Chris AtLee [:catlee]
bhearsum: review+
catlee: checked‑in+
Details | Diff | Splinter Review
updated front-end and clobberer.py script (69.49 KB, patch)
2009-02-05 10:43 PST, Chris AtLee [:catlee]
bhearsum: review+
catlee: checked‑in+
Details | Diff | Splinter Review
set default clobber URL and clobber time on production (4.73 KB, patch)
2009-02-05 12:44 PST, Chris AtLee [:catlee]
bhearsum: review+
catlee: checked‑in+
Details | Diff | Splinter Review

Description Benjamin Smedberg [:bsmedberg] 2008-05-05 08:46:16 PDT
I'm going to file this together, though it might end up being separate:

Every time we merge a new NSPR into mozilla-central, various boxes go red, including the unit-test boxes and at least some of the depend build machines. In this particular case it's because of autoconf differences leading to CVS conflicts, but there are other reasons we regularly may need to clobber these builds.

Ideally what I'd like to see is a form on the buildbot waterfall or someplace similar that lets the sheriff a clobber, but I've heard that's hard... doing something from a checkin like with do with CVS clobber files is ok too.

Note that if we do the clobber-file thing, it should trigger a new build immediately, instead of waiting for the next checkin.
Comment 1 Rob Campbell [:rc] (:robcee) 2008-05-05 10:36:25 PDT

*** This bug has been marked as a duplicate of bug 431529 ***
Comment 2 Benjamin Smedberg [:bsmedberg] 2008-05-07 00:18:06 PDT
Reopening: this bug will be about the depend builders, and bug 431529 will be about the unit-test boxes, which are on a different master.
Comment 3 Benjamin Smedberg [:bsmedberg] 2008-05-28 14:04:52 PDT
Created attachment 322851 [details] [diff] [review]
Rough clobber support

This is a rough draft of clobber support via an external webtool that doesn't require LDAP/MPT access.
Comment 4 Ben Hearsum (:bhearsum) 2008-05-30 13:32:50 PDT
I don't really like this method but I've been racking my brain all week trying to come up with a better way, and I can't find one. Writing a proper Buildbot patch that integrates with Waterfall, can clobber all slaves when requested is a hard task. Therefore I'm in favour of this approach, especially given the false-positive performance regression.

I think we need to put the actual form behind ldap, probably on build.mozilla.org (all hg users have an LDAP account anyways, so this shouldn't unnecessarily restrict anyone).

I think it makes sense to have it do periodic clobbers too. Either clobbering if there hasn't been one for X hours (24 hours, maybe?), or clobbering when time hits X hour of the day (2am? *shrug).

Benjamin, if you can write the rest of the patch I'd be happy to test it on all platforms for you.
Comment 5 Rob Campbell [:rc] (:robcee) 2008-06-02 06:20:24 PDT
+1 to bhearsum's comments.

I was thinking about periodic clobbers this weekend as well. We could just add a separate Clobber buildstep inside a Periodic scheduler that does the business of wiping out a tree followed by a build.
Comment 6 Ben Hearsum (:bhearsum) 2008-07-14 07:27:23 PDT
I'll finish this up this week.
Comment 7 Ben Hearsum (:bhearsum) 2008-07-16 06:29:10 PDT
Created attachment 329822 [details] [diff] [review]
[checked in] ugly, basic front-end for clobbering

This script does the same thing as yours, Benjamin, but it also records the name of the person that clobbers (working under the assumption we'll require LDAP to do it). I added short_name/long_name because PHP mangles <input name="..."> when there's special characters (such as spaces or dots). Because of this, I removed the ability to automatically add a new tree by simply visiting .../index.php?tree=...
Comment 8 Ben Hearsum (:bhearsum) 2008-07-16 06:37:35 PDT
Created attachment 329824 [details] [diff] [review]
clobber buildstep - forced clobbers + periodic

Here's a BuildStep that reads polls the PHP script for forced clobbers, and also does periodic ones. I think the code is pretty straight forward...

It's set to clobber if there hasn't been one for 24 hours. This means that it *could* happen in the middle of the day. If that's not a great idea we can change it up.

One thing I don't really like is that slaves may only get one build of a tree before clobbering...eg, if moz2-win32-slave02 builds mozilla-central today, but then doesn't get another build of that until friday, it seems silly that it will clobber. To minimize this, I think it might be useful to bump up the periodic clobber time to a few days, a week, maybe longer if that's OK. Looking for opinions here (ted/bsmedberg?).

Forced clobbers have a similar problem in that it's possible that they don't build a tree that a clobber was forced on for a day or two. While it's good that they *do* clobber ASAP after the request, it could be confusing that it takes a day or two to happen.

I've tested this on a local Buildbot and it's working perfectly. I'll be running tests on our staging setup shortly.
Comment 9 Ben Hearsum (:bhearsum) 2008-07-16 11:18:14 PDT
Comment on attachment 329824 [details] [diff] [review]
clobber buildstep - forced clobbers + periodic

Hrm. Coop is out this week, moving the review...
Comment 10 Nick Thomas [:nthomas] 2008-07-17 03:45:10 PDT
Comment on attachment 329824 [details] [diff] [review]
clobber buildstep - forced clobbers + periodic

A few questions, all clues welcome.

>Index: misc.py
>===================================================================
...
>+        if 'command' not in kwargs:
>+            kwargs['command'] = ["python", "-c", """
>+import sys, shutil, urllib2, urllib, os
>+from datetime import datetime, timedelta
>+ONE_DAY = timedelta(days=1)

Seems to be quite a lot of inline code here, what was the decision process there ? Could it be stashed in the build system somewhere, so that it's already on the slave ? Or does that create a chicken and egg problem on the first build ? Would MozillaClobber be the first addStep in the Factory, and therefore be before the hg pull ?

...
>+cur_force_date = urllib2.urlopen(url).read()

What happens if the information can't be retrieved ? eg the site is down or returning bogus info. Would it be worth checking for sane input here before trampling on the state files ? 

And do you think we'd want to return an error and prevent builds if url is down ? Probably not, but I guess there needs to be something relatively noisy if clobbers are failing. Perhaps a TinderboxPrint, if you can get to the log from here.

>+  print "Checking clobber URL: %s" % url
>+  print "Current clobber date: %s" % cur_force_date

Would be nice to move these up to where it really happens (either side of the urlopen call).

>+  if (last_clobber + ONE_DAY < cur_date):
>+    print "More than 24 hours have passed since the last clobber, clobbering " \
>+          "build directory"
>+    do_clobber()

I'm a bit concerned about this, given we could end up with N slow builds a day for N connected slaves (assuming there are enough jobs/slave for randomness to do it's thing when they're allocated). Backing it off to once a week would help, particularly if we can stagger the clobbers over that period. (I'd really love if we could send the tinderbox server some TinderPrints as part of the build start email, including the source revision and clobber, then it's clear to all why a build is slow and what it's building.)
Comment 11 Ben Hearsum (:bhearsum) 2008-07-17 05:12:42 PDT
(In reply to comment #10)
> (From update of attachment 329824 [details] [diff] [review])
> A few questions, all clues welcome.
> 
> >Index: misc.py
> >===================================================================
> ...
> >+        if 'command' not in kwargs:
> >+            kwargs['command'] = ["python", "-c", """
> >+import sys, shutil, urllib2, urllib, os
> >+from datetime import datetime, timedelta
> >+ONE_DAY = timedelta(days=1)
> 
> Seems to be quite a lot of inline code here, what was the decision process
> there ? Could it be stashed in the build system somewhere, so that it's already
> on the slave ? Or does that create a chicken and egg problem on the first build
> ? Would MozillaClobber be the first addStep in the Factory, and therefore be
> before the hg pull ?
> 

I don't know that this belongs in the build system but I could be swayed on that. The other option is to store this in a Python script alongside this BuildStep. The downside here is that we'd need an additional ShellCommand to check-out the script. Admittedly, this isn't much of a barrier.

> ...
> >+cur_force_date = urllib2.urlopen(url).read()
> 
> What happens if the information can't be retrieved ? eg the site is down or
> returning bogus info. Would it be worth checking for sane input here before
> trampling on the state files ? 
> 
> And do you think we'd want to return an error and prevent builds if url is down
> ? Probably not, but I guess there needs to be something relatively noisy if
> clobbers are failing. Perhaps a TinderboxPrint, if you can get to the log from
> here.
> 

Yeah, I think we just want to not clobber if we can't pull the URL. I'll add some error handling code here.

> >+  print "Checking clobber URL: %s" % url
> >+  print "Current clobber date: %s" % cur_force_date
> 
> Would be nice to move these up to where it really happens (either side of the
> urlopen call).
> 

Sure.

> >+  if (last_clobber + ONE_DAY < cur_date):
> >+    print "More than 24 hours have passed since the last clobber, clobbering " \
> >+          "build directory"
> >+    do_clobber()
> 
> I'm a bit concerned about this, given we could end up with N slow builds a day
> for N connected slaves (assuming there are enough jobs/slave for randomness to
> do it's thing when they're allocated). Backing it off to once a week would
> help, particularly if we can stagger the clobbers over that period.

Sure, can do.

> (I'd really
> love if we could send the tinderbox server some TinderPrints as part of the
> build start email, including the source revision and clobber, then it's clear
> to all why a build is slow and what it's building.)
> 

No kidding!
Comment 12 Ben Hearsum (:bhearsum) 2008-07-18 09:23:42 PDT
Created attachment 330253 [details] [diff] [review]
address nick's review comments
Comment 13 Ben Hearsum (:bhearsum) 2008-07-21 07:58:47 PDT
Created attachment 330575 [details] [diff] [review]
master.cfg changes for clobbers
Comment 14 Wolf [:wolf] 2008-07-21 08:14:12 PDT
> (I'd really
> love if we could send the tinderbox server some TinderPrints as part of the
> build start email, including the source revision and clobber, then it's clear
> to all why a build is slow and what it's building.)
> 

Is there a bug filed on what sounds like a good idea against tinderbox server? :-)
Comment 15 Nick Thomas [:nthomas] 2008-07-22 02:11:24 PDT
Comment on attachment 330253 [details] [diff] [review]
address nick's review comments

r+, but with maybe two fixes on checkin please, and take a look at some suggested improvements.

>Index: misc.py
>===================================================================
>+except URLError:
>+  print "Couldn't poll %s, skipping forced clobber" % url

I ran a few tests with the lines added with this patch, and I think this will barf if there is a DNS problem (NameError). Might be worth just a blanket "except:" ? Hard to know what the main error conditions will be. You could change this without review.

>+  if (last_clobber + PERIODIC_CLOBBER_TIME < cur_date):
>+    print "More than 24 hours have passed since the last clobber, clobbering " \
>+          "build directory"

The "24 hours" part of this message is out of date now, so please fix it up in a way that displays PERIODIC_CLOBBER_TIME in a human-friendly way (the default print format for a timedelta isn't too bad). No further review req'd.

>+    try:
>+      do_clobber()
>+    except:
>+      print "Couldn't clobber properly, not updating last-clobber file"
>+      sys.exit(0)

These are both suggestions: 

* You could argue that failure to clobber will require intervention from us and should send the build red. Or perhaps that would be a PITA. I'm assuming that exit(1) propagates back up. What do you think ?
* Should there be a try wrapper around the do_clobber for the forced case as well as the periodic one ? Maybe move the try inside do_clobber ?
Comment 16 Ben Hearsum (:bhearsum) 2008-07-22 07:50:24 PDT
(In reply to comment #15)
> (From update of attachment 330253 [details] [diff] [review])
> r+, but with maybe two fixes on checkin please, and take a look at some
> suggested improvements.
> 
> >Index: misc.py
> >===================================================================
> >+except URLError:
> >+  print "Couldn't poll %s, skipping forced clobber" % url
> 
> I ran a few tests with the lines added with this patch, and I think this will
> barf if there is a DNS problem (NameError). Might be worth just a blanket
> "except:" ? Hard to know what the main error conditions will be. You could
> change this without review.
> 

Yeah, that makes sense.

> >+  if (last_clobber + PERIODIC_CLOBBER_TIME < cur_date):
> >+    print "More than 24 hours have passed since the last clobber, clobbering " \
> >+          "build directory"
> 
> The "24 hours" part of this message is out of date now, so please fix it up in
> a way that displays PERIODIC_CLOBBER_TIME in a human-friendly way (the default
> print format for a timedelta isn't too bad). No further review req'd.
> 

Sounds good.

> >+    try:
> >+      do_clobber()
> >+    except:
> >+      print "Couldn't clobber properly, not updating last-clobber file"
> >+      sys.exit(0)
> 
> These are both suggestions: 
> 
> * You could argue that failure to clobber will require intervention from us and
> should send the build red. Or perhaps that would be a PITA. I'm assuming that
> exit(1) propagates back up. What do you think ?

I think the most common case here will be win32 only partly clobbering. I think it makes a lot of sense to fail out in that case, and probably most other failures in do_clobber()

> * Should there be a try wrapper around the do_clobber for the forced case as
> well as the periodic one ? Maybe move the try inside do_clobber ?
> 

Yeah, that makes sense too.
Comment 17 Ben Hearsum (:bhearsum) 2008-07-22 07:52:17 PDT
Created attachment 330774 [details] [diff] [review]
[checked in] Clobberer BuildStep as landed, addresses Nick's last review comments

Checking in misc.py;
/cvsroot/mozilla/tools/buildbotcustom/steps/misc.py,v  <--  misc.py
new revision: 1.5; previous revision: 1.4
done
Comment 18 Chris Cooper [:coop] [away until Aug 29] 2008-07-24 10:17:30 PDT
Comment on attachment 330575 [details] [diff] [review]
master.cfg changes for clobbers

"clobberer" <- awesome
Comment 19 Ben Hearsum (:bhearsum) 2008-08-05 10:31:19 PDT
Comment on attachment 330774 [details] [diff] [review]
[checked in] Clobberer BuildStep as landed, addresses Nick's last review comments

>Index: misc.py
>===================================================================
>+try:
>+  print "Checking clobber URL: %s" % url
>+  cur_force_date = urllib2.urlopen(url).read()
>+  print "Current forced clobber date: %s" % cur_force_date
>+  if not os.path.exists('force-clobber'):
>+    write_file(cur_force_date, 'force-clobber')
>+  else:
>+    old_force_date = open('force-clobber').read()
>+    print "Last forced clobber: %s" % old_force_date
>+    if old_force_date != cur_force_date:
>+      print "Clobber forced, clobbering build directory"
>+      do_clobber()
>+      write_file(cur_force_date, 'force-clobber')
>+      write_file(cur_force_date, 'last-clobber')
>+      sys.exit(0)
>+except:
>+  print "Couldn't poll %s, skipping forced clobber" % url

Turns out that sys.exit() throws an exception to get Python to quit, and this except statement will catch it. I've got to re-work this a little bit.
Comment 20 Ben Hearsum (:bhearsum) 2008-08-05 11:10:00 PDT
Comment on attachment 329822 [details] [diff] [review]
[checked in] ugly, basic front-end for clobbering

changeset:   0:7ae7fb134bf7
Comment 21 Ben Hearsum (:bhearsum) 2008-08-05 11:11:25 PDT
Trying to push this through now. Looks like Windows is having issues with the BuildStep. It's not printing _anything_ out in the logs, or clobbering properly.

Suspect it has something to do with MSYS i/o handling.

http://staging-master.build.mozilla.org:8010/builders/WINNT%205.2%20mozilla-central%20build/builds/95/steps/shell_2/logs/stdio
Comment 22 Ben Hearsum (:bhearsum) 2008-08-05 12:32:48 PDT
Created attachment 332401 [details] [diff] [review]
[checked in] clobbering code in a stand-alone script

I fussed about but couldn't get Windows to work with with our clobber step. This is mostly just a straight rip-out off that Python code. The only change I've made is to make sys.exit() work properly by catching SystemExit.

When I check this in, I will be deleting the MozillaClobber step from buildbotcustom.
Comment 23 Ben Hearsum (:bhearsum) 2008-08-05 12:35:32 PDT
Comment on attachment 332401 [details] [diff] [review]
[checked in] clobbering code in a stand-alone script

I'm open to suggestions about the directory structure in hg.m.o/build/tools, btw
Comment 24 Ben Hearsum (:bhearsum) 2008-08-05 13:06:30 PDT
Created attachment 332405 [details] [diff] [review]
master.cfg w/ external clobber script
Comment 25 Nick Thomas [:nthomas] 2008-08-06 04:29:33 PDT
Comment on attachment 332405 [details] [diff] [review]
master.cfg w/ external clobber script

r+ with nit's addressed.

>diff -r 93816cedf102 mozilla2-unittest/master.cfg
...
>+moz2_win32_unittest_factory2.addStep(ShellCommand,
>+    command=['rm', '-rfv', 'tools'],
>+    workdir='.',
>+    env=MozillaEnvironments['win32-vc8-mozbuild-unittest'])
>+moz2_win32_unittest_factory2.addStep(ShellCommand,
>+    command=['hg', 'clone', 'http://hg.mozilla.org/build/tools'],
>+    workdir='.',
>+    env=MozillaEnvironments['win32-vc8-mozbuild-unittest'])
>+moz2_win32_unittest_factory2.addStep(ShellCommand,
>+    command=['python', 'tools/clobberer/clobberer.py', clobberURL],
>+    workdir='.',
>+    env=MozillaEnvironments['win32-vc8-mozbuild-unittest'])

nit - LocalEnvironments['win32-vc8-mozbuild-unittest'] is used in subsequent steps, seems to be used for this factory only. We should probably be consistent within the factory, and merge the Environments for win32 elsewhere.

>diff -r 93816cedf102 mozilla2/config.py
...
>+BUILD_TOOLS_REPO = 'http://hg.mozilla.org/build/tools'

Nit: _URL suffix to match existing style, and fix use in master.cfg
Comment 26 Nick Thomas [:nthomas] 2008-08-06 04:32:32 PDT
(In reply to comment #23)
> (From update of attachment 332401 [details] [diff] [review])
> I'm open to suggestions about the directory structure in hg.m.o/build/tools,
> btw

I don't have any dogma to dispense, other than "not too flat, not too nested". At least we can move/rename easily in Hg.

Comment 27 Ben Hearsum (:bhearsum) 2008-08-06 04:33:19 PDT
(In reply to comment #25)
> (From update of attachment 332405 [details] [diff] [review])
> r+ with nit's addressed.
> 
> >diff -r 93816cedf102 mozilla2-unittest/master.cfg
> ...
> >+moz2_win32_unittest_factory2.addStep(ShellCommand,
> >+    command=['rm', '-rfv', 'tools'],
> >+    workdir='.',
> >+    env=MozillaEnvironments['win32-vc8-mozbuild-unittest'])
> >+moz2_win32_unittest_factory2.addStep(ShellCommand,
> >+    command=['hg', 'clone', 'http://hg.mozilla.org/build/tools'],
> >+    workdir='.',
> >+    env=MozillaEnvironments['win32-vc8-mozbuild-unittest'])
> >+moz2_win32_unittest_factory2.addStep(ShellCommand,
> >+    command=['python', 'tools/clobberer/clobberer.py', clobberURL],
> >+    workdir='.',
> >+    env=MozillaEnvironments['win32-vc8-mozbuild-unittest'])
> 
> nit - LocalEnvironments['win32-vc8-mozbuild-unittest'] is used in subsequent
> steps, seems to be used for this factory only. We should probably be consistent
> within the factory, and merge the Environments for win32 elsewhere.
> 

I'm not sure what you mean by this...
Comment 28 Nick Thomas [:nthomas] 2008-08-06 04:38:11 PDT
Just that 
* moz2_win32_unittest_factory2 uses LocalEnvironments['win32-vc8-mozbuild-unittest']
* all the other factories use one of the MozillaEnvironments
* your copy and paste adds steps to moz2_win32_unittest_factory2 using  MozillaEnvironments
* I don't know if the two windows environments differ, so we should be consistent within a given factory
Comment 29 Ben Hearsum (:bhearsum) 2008-08-06 04:39:50 PDT
Yeah, just saw that, whoops.
Comment 30 Ben Hearsum (:bhearsum) 2008-08-06 04:40:26 PDT
Created attachment 332515 [details] [diff] [review]
master.cfg v3 - addresses Nick's comments
Comment 31 Ben Hearsum (:bhearsum) 2008-08-06 04:42:06 PDT
Comment on attachment 332401 [details] [diff] [review]
[checked in] clobbering code in a stand-alone script

changeset:   1:881cad398b4f
Comment 32 Chris AtLee [:catlee] 2009-01-08 08:42:39 PST
*** Bug 436449 has been marked as a duplicate of this bug. ***
Comment 33 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2009-01-21 10:54:49 PST
*** Bug 431529 has been marked as a duplicate of this bug. ***
Comment 34 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2009-01-21 14:20:25 PST
*** Bug 393417 has been marked as a duplicate of this bug. ***
Comment 35 Chris AtLee [:catlee] 2009-01-27 09:04:28 PST
Created attachment 359056 [details] [diff] [review]
updated front-end and clobberer.py script
Comment 36 Chris AtLee [:catlee] 2009-01-27 09:13:01 PST
Created attachment 359059 [details] [diff] [review]
Add clobberURL to MozillaBuildFactory
Comment 37 Chris AtLee [:catlee] 2009-01-27 09:25:52 PST
Created attachment 359065 [details] [diff] [review]
Updates to mozilla2-staging to set clobber URL
Comment 38 Ben Hearsum (:bhearsum) 2009-01-27 10:07:56 PST
Comment on attachment 359065 [details] [diff] [review]
Updates to mozilla2-staging to set clobber URL

>diff -r 714ad802ae42 mozilla2-staging/config.py
>--- a/mozilla2-staging/config.py	Mon Jan 26 21:32:14 2009 +1300
>+++ b/mozilla2-staging/config.py	Tue Jan 27 12:24:35 2009 -0500
>@@ -13,18 +13,19 @@
> STAGE_BASE_PATH = '/home/ftp/pub/firefox'
> STAGE_GROUP = None
> STAGE_SSH_KEY = 'ffxbld_dsa'
> AUS2_USER = 'cltbld'
> AUS2_HOST = 'staging-stage.build.mozilla.org'
> DOWNLOAD_BASE_URL = 'ftp://ftp.mozilla.org/pub/mozilla.org/firefox'
> GRAPH_SERVER = 'graphs-stage.mozilla.org'
> GRAPH_SELECTOR = 'server'
> BUILD_TOOLS_REPO_PATH = 'users/stage-ffxbld/tools'
> DEFAULT_BUILD_SPACE = 5
>+CLOBBER_URL = 'http://build.mozilla.org/clobberer/staging.php'
> 

Is this going to be the final home for this? I'm a little worried about updating the PHP file and forgetting to change the database name in it. I think it's a bit nicer to have a separate directory so we can use the exact same php file without modification or worry. It's up to you though!

r=bhearsum either way
Comment 39 Ben Hearsum (:bhearsum) 2009-01-27 10:10:59 PST
Comment on attachment 359059 [details] [diff] [review]
Add clobberURL to MozillaBuildFactory

>Index: factory.py
>===================================================================
>+        if self.clobberURL is not None:
>+            command = ['python', 'tools/clobberer/clobberer.py',
>+             self.clobberURL, self.branchName,
>+             WithProperties("%(buildername)s"),
>+             WithProperties("%(slavename)s")
>+            ]
>+            self.addStep(ShellCommand,
>+             command=command,
>+             description=['checking','clobber','times'],
>+             workdir='.',
>+             flunkOnFailure=False,

Why flunkOnFailure=False? Whether we clobber or not this script should return 0, unless something weird happens. Please remove this.

r=bhearsum with that change
Comment 40 Ben Hearsum (:bhearsum) 2009-01-27 10:24:41 PST
Comment on attachment 359056 [details] [diff] [review]
updated front-end and clobberer.py script

>diff -r cf6614a8d619 clobberer/clobberer.py

>+  parser.add_option("-t", "--periodic", dest="period", type="float",
>+      default=24*7, help="hours between periodic clobbers")

Nice. Thanks for making this an option.

>+
>+  options, args = parser.parse_args()
>+  PERIODIC_CLOBBER_TIME = timedelta(hours = options.period)

camelcase this to make it consistent, please.

>+    cur_date = datetime.utcnow()
>+    try:
>+      last_clobber = str_to_datetime(open("last-clobber").read().strip())
>+      print "Last clobber: %s" % datetime_to_str(last_clobber)
>+      print "Current time: %s" % datetime_to_str(cur_date)
>+      if (last_clobber + PERIODIC_CLOBBER_TIME < cur_date):
>+        print "*** More than %s have passed since the last clobber, clobbering *** " % \
>+          PERIODIC_CLOBBER_TIME
>+        do_clobber(options.dryrun)
>+        # if do_clobber fails the script will exit and this will not be executed
>+        write_file(cur_date, "last-clobber")
>+    except ValueError:
>+      write_file(cur_date, "last-clobber")

What are you updating last-clobber on ValueError? Purposely, it doesn't get updated when the clobber fails.

>diff -r cf6614a8d619 clobberer/index.php

>@@ -1,86 +1,218 @@
>-<!---
>+<?php
>+/*
> This is a web interface that allows developers to clobber mozilla-central,
> actionmonkey, and tracemonkey builds.

Let's update this comment to say "...allow developers to clobber Mercurial based builds", or something to that effect.

> 
> This script simply updates a database that Buildbot reads from at the start of
> every build. The BuildStep that does this is here:
> http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/steps/misc.py#167

And remove the '#167' portion of this. (This is my fault, but since you're doing the patches anyways...;-)



>-$q = $dbh->query('SELECT count(*) FROM sqlite_master WHERE NAME="CLOBBERS"');
>+$q = $dbh->query('SELECT count(*) FROM sqlite_master WHERE NAME="clobber_times"');
> $exists = $q->fetch(PDO::FETCH_NUM);
>-if (!$exists[0]) {
>-  $dbh->exec('CREATE TABLE CLOBBERS (short_name VARCHAR(30),'
>-            .'long_name VARCHAR(50),'
>-            .'lastclobber VARCHAR(30),'
>-            .'clobberer VARCHAR(50))');
>+if (!$exists or !$exists[0]) {

Why !$exists?

> 
>+//
>+// Handle form submision

nit: fix the typo here

>+//
> if ($_POST['form_submitted']) {
>   $clobbers = array();
>+  $slaves = array();
>   foreach ($_POST as $k => $v) {
>     $t = explode('-', $k, 2);
>-    if ($t[0] == 'clobber') {
>-      array_push($clobbers, $t[1]);
>+    // We only care about slaves
>+    if ($t[0] == 'slave') {
>+      $id = e($t[1]);
>+      $user = e($_SERVER['REMOTE_USER']);
>+      $dbh->exec("UPDATE clobber_times SET lastclobber = DATETIME(\"NOW\"), clobberer = $user WHERE id = $id");

What's $id here? It doesn't look like this code handles slave+builder, so I'm thinking it would clobber all trees for a slave every time.

Everything else looks fine, holding off on changing the flag because of the question above.
Comment 41 Ben Hearsum (:bhearsum) 2009-01-28 08:03:58 PST
Comment on attachment 359056 [details] [diff] [review]
updated front-end and clobberer.py script

Chris talked me through the form submission section, this looks good.
Comment 42 Chris AtLee [:catlee] 2009-01-28 09:40:53 PST
Created attachment 359304 [details] [diff] [review]
updated front-end and clobberer.py script

This new patch should address the issues above.
Comment 43 Ben Hearsum (:bhearsum) 2009-01-28 14:43:58 PST
Comment on attachment 359304 [details] [diff] [review]
updated front-end and clobberer.py script

Looks good to me.
Comment 44 Chris AtLee [:catlee] 2009-02-05 06:02:53 PST
Created attachment 360709 [details] [diff] [review]
Add clobberURL and clobberTime to MozillaBuildFactory
Comment 45 Chris AtLee [:catlee] 2009-02-05 06:34:47 PST
Created attachment 360712 [details] [diff] [review]
updated front-end and clobberer.py script
Comment 46 Chris AtLee [:catlee] 2009-02-05 07:19:45 PST
Created attachment 360721 [details] [diff] [review]
Updates to mozilla2-staging to set clobber URL and clobber time
Comment 47 Ben Hearsum (:bhearsum) 2009-02-05 07:43:14 PST
Comment on attachment 360709 [details] [diff] [review]
Add clobberURL and clobberTime to MozillaBuildFactory

>diff -r 8b3c53f98959 process/factory.py
>--- a/process/factory.py	Wed Feb 04 09:59:53 2009 -0500
>+++ b/process/factory.py	Thu Feb 05 09:01:53 2009 -0500
>+        if self.clobberURL is not None:
>+            if self.clobberTime is None:
>+                self.clobberTime = 24*7

I guess you can't assign self.clobberTime above, but can you set defaultClobberTime as a class-level variable, or some such?

Looks good otherwise. r=me with that change
Comment 48 Ben Hearsum (:bhearsum) 2009-02-05 07:53:56 PST
Comment on attachment 360712 [details] [diff] [review]
updated front-end and clobberer.py script

>+every build.
>+http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/process/factory.py
>+*/

Should update this link now.

This is fine otherwise.
Comment 49 Chris AtLee [:catlee] 2009-02-05 10:40:20 PST
Created attachment 360754 [details] [diff] [review]
Add clobberURL and clobberTime to MozillaBuildFactory

Changes here are we clobber the tools directory before starting, and then we don't have to do hg pull -u since we've just cloned it.

Also, pass '-s tools' to clobberer.py, which will skip clobbering the tools directory, which will let purge_builds.py run properly afterwards.
Comment 50 Chris AtLee [:catlee] 2009-02-05 10:43:35 PST
Created attachment 360756 [details] [diff] [review]
updated front-end and clobberer.py script

Add support for skipping files/directories on the command line.
Comment 51 Chris AtLee [:catlee] 2009-02-05 12:33:40 PST
Comment on attachment 360754 [details] [diff] [review]
Add clobberURL and clobberTime to MozillaBuildFactory

changeset:   187:9517989a64e5
Comment 52 Chris AtLee [:catlee] 2009-02-05 12:37:33 PST
Comment on attachment 360721 [details] [diff] [review]
Updates to mozilla2-staging to set clobber URL and clobber time

changeset:   726:5a0a4e79714e
Comment 53 Chris AtLee [:catlee] 2009-02-05 12:40:29 PST
Comment on attachment 360756 [details] [diff] [review]
updated front-end and clobberer.py script

changeset:   39:03ee28c55c27
Comment 54 Chris AtLee [:catlee] 2009-02-05 12:44:47 PST
Created attachment 360784 [details] [diff] [review]
set default clobber URL and clobber time on production

for when we want to deploy this onto production
Comment 55 Chris AtLee [:catlee] 2009-02-09 05:50:21 PST
Comment on attachment 360784 [details] [diff] [review]
set default clobber URL and clobber time on production

changeset:   932:afd401b42348
Comment 56 Chris AtLee [:catlee] 2009-02-10 18:22:29 PST
It looks like this is working well since being deployed yesterday.

Note You need to log in before you can comment on or make changes to this bug.