393413 - clobber in unittest directory

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Rob Campbell [:rc] (:robcee)]

Currently, the unittest buildbots are watching mozilla/tools/tinderbox-configs/firefox/{Platform}/CLOBBER. We should also be watching a similar path structure under tools/buildbot-configs/testing/unittest/firefox/{Platform}/CLOBBER specifically for these machines.

The reason is that sometimes clobbering all of the machines across the full tinderbox is unnecessary. The unittest machines occasionally require extra clobbers that the others do not.

Comment 1

[Chris Cooper [:coop] (he/him)]

Going to use the existing test_unit branch of tinderbox configs to control unittest-only clobbers. I'll need to add a new build step to checkout/update those files and check for new CLOBBER requests.

Comment 2

[[e-mail] (:bhearsum)]

I believe the unit tests boxes already checkout the tinderbox-configs directory:
This line is in the mozconfig's:
mk_add_options MOZ_CO_MODULE=mozilla/js/tests,mozilla/tools/tinderbox-configs

Comment 3

[Rob Campbell [:rc] (:robcee)]

yes, and that will need to stay there. We wanted to add support to do a unittest only clobber if desired against the test_unit branch.

Comment 4

[[e-mail] (:bhearsum)]

Oh, ok. My bad.

Comment 5

[Chris Cooper [:coop] (he/him)]

Hrmmm, that does reveal that we'd be unable to selectively CLOBBER just trunk or branch if we're only using the one test_unit branch for CLOBBER info. Maybe it does need to go in its own dir tree.

Comment 6

[Rob Campbell [:rc] (:robcee)]

yeah, I was wondering about that too. Having our own directory for these might be the best way to go after all.

Comment 7

[Chris Cooper [:coop] (he/him)]

Since the master has to know about both trunk and branch (and possibly more), I'm advocating the following directory structure *without* doing CVS tagging by branch:

unittest/mozconfigs/$PRODUCT/$BRANCH/$PLATFORM/mozconfig(-debug)

...where $PLATFORM will be 'firefox' for now, and $PLATFORM will be 'macosx,' 'winxp,' etc.

We can throw the CLOBBER files under this same hierarchy, or add another unittest/CLOBBER/... dir system that would have the same layout as above.

Thoughts?

Comment 8

[Rob Campbell [:rc] (:robcee)]

I think this is our best bet and is close to what I was originally thinking before being confused by tagging options.

We might want to consider using more specific platform names. winnt5.1, winnt5.2, centos5, darwin8.10, etc. Or maybe just checking in various versions of mozconfig for specific subplatform would be sufficient. eg, win32/mozconfig-win2k3

Comment 9

[Chris Cooper [:coop] (he/him)]

(In reply to comment #8) 
> We might want to consider using more specific platform names. winnt5.1,
> winnt5.2, centos5, darwin8.10, etc. Or maybe just checking in various versions
> of mozconfig for specific subplatform would be sufficient. eg,
> win32/mozconfig-win2k3

We don't have to mirror the tinderbox hierarchy at all. I've started using the more-specific platform designations, e.g. win2k3, already.

Comment 10

[Rob Campbell [:rc] (:robcee)]

sounds good. wfm!

Comment 11

[Chris Cooper [:coop] (he/him)]

Attachment: Add unittest-specific CLOBBER support.

robcee: Do with this what you will. 

I've tested the linuxFactory update and clobber steps in isolation here and they work.  I've added the steps to the other ${platform}Factories, but haven't had a chance to try them out yet. The same applies for the killAndClobberWin.py script.

I've added one CLOBBER dir + file already to CVS under unittest/CLOBBER/firefox/TRUNK/linux for testing. We'll need to add the other branch/platform dirs as required.

There's also some code commented out in there to do the config updating. We probably don't want to go down that route if we can get botrunner working though.

Comment 12

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 280662 [details] [diff] [review]
Add unittest-specific CLOBBER support.

killAndClobberWin.py still using hard-coded "trunk_2k3" directory. We should probably fix this in a later version, but ok for now (will require manual updating on winxp)

Should move from cvs-mirror to cvs.mozilla.org (in CVSShellCommand)

I don't think the UnitTestUpdateClobberFiles section works on Windows. You don't get a bash shell by default there and I've had bad luck invoking bash and passing long constructed commands to it. I think we'll need a separate subclass for windows.

Comment 13

[Chris Cooper [:coop] (he/him)]

(In reply to comment #12)
> (From update of attachment 280662 [details] [diff] [review])
> killAndClobberWin.py still using hard-coded "trunk_2k3" directory. We should
> probably fix this in a later version, but ok for now (will require manual
> updating on winxp)
> 
> Should move from cvs-mirror to cvs.mozilla.org (in CVSShellCommand)

Yes, there has likely been some bitrot since the patch was submitted. I will update.

> I don't think the UnitTestUpdateClobberFiles section works on Windows. You
> don't get a bash shell by default there and I've had bad luck invoking bash and
> passing long constructed commands to it. I think we'll need a separate subclass
> for windows.

Do we need a bug to get staging slaves setup to test changes like this then?

Comment 14

[Reed Loden [:reed]]

Any update on this? There have been times lately where I've only wanted to clobber the unit test machines instead of the nightlies.

Comment 16

[Chris Cooper [:coop] (he/him)]

I have this working now against the leak test boxes on all platforms. I'll work up a patch against the unittest master shortly.

Comment 17

[Chris Cooper [:coop] (he/him)]

Attachment: Add proper clobber support to buildbot

Changes in this patch:
* pass an (optional) platform, slaveName, and branch to killAndClobberWin.py. Current default behavior is preserved if these options are not provided.
* reorder build steps in master.cfg so that clobbers happen first. This is important because we now...
* remove the entire mozilla/ dir rather just the objdir/ when a clobber is triggered. This should eliminate most cases where human interaction is required to clobber these builds.
* added the UpdateClobberFiles() step to mozbuild.py. By default, this step checks out/updates both tinderbox and buildbot CLOBBER files under a dir named clobber_files/ (can be overridden). The log files from this step are checked by MozillaClobber to determine whether a clobber is required.

I have a separate patch coming for the ancillary changes to the mozconfigs (we can stop checking out the  tinderbox-configs modules now).

Comment 18

[Chris Cooper [:coop] (he/him)]

Attachment: Stop checking out the tinderbox-configs module

Comment 19

[Chris Cooper [:coop] (he/him)]

Attachment: Add proper clobber support to buildbot, v2

Fixes the self.clobberFilePath and mkdir issues on Windows from the first patch. Note: this means that the clobber_files/ dir must be created by hand initially on Windows.

This patch is running right now on the leak+unittest boxes.

Comment 20

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 298329 [details] [diff] [review]
Stop checking out the tinderbox-configs module

we should probably stop checking out mozilla/js/tests from these machines too at some point.

Comment 21

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 298355 [details] [diff] [review]
Add proper clobber support to buildbot, v2

I wonder if we're killing enough here. Often the stuck machines (mozilla-build, anyway) have make.exe and sh.exe in multiple quantities running, preventing a clean rmdir. I wonder if we should try killing these as well?

hard tabs in file from lines 52-56, 71-74.

We could try using shutils.rmtree() which I was happy to learn about last week (line 44), but not necessary.

You could be using os.path.join for constructing logDir variable in lines 294, 297, 327, 329.

eg., os.path.join('..', 'logs') + os.sep // for trailing separator

This would get rid of the if self.platform.startswith('win'): block.

we can try this out first before adding extra bits to kill make and sh on windows.

Comment 22

[[e-mail] (:bhearsum)]

(In reply to comment #21)
> We could try using shutils.rmtree() which I was happy to learn about last week
> (line 44), but not necessary.

Hey,
It's my understanding that rmtree() doesn't work so great under Windows sometimes. Buildbot uses a replacement function that can be found here:
http://lxr.mozilla.org/mozilla/source/tools/buildbot/buildbot/slave/commands.py#59

From what I can tell, all it does is try to give itself write permissions to the files it's trying to delete if they don't already have them.

Comment 23

[Rob Campbell [:rc] (:robcee)]

ugh. Thanks for the tip, Ben!

Comment 24

[Chris Cooper [:coop] (he/him)]

Attachment: Remove mozilla/js/tests module as well [checked in]

Comment 25

[Chris Cooper [:coop] (he/him)]

Attachment: Add proper clobber support to buildbot, v3

* use os.path where appropriate
* include rmdirRecursive from buildbot in killAndClobberWin.py
* pskill both sh.exe and make.sh to prevent overlapping builds

Comment 26

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 298489 [details] [diff] [review]
Add proper clobber support to buildbot, v3

are the sections referring to macosxLeakFactory supposed to be in there or do they just need to have the receiver's variable renamed? (lines 175-183, 202-210) E.g., osxFactory, leopardFactory?

lines 252 and 301:
+        slaveName="slave_coop",

should be just "slave" on these machines.

gettin' there!

Comment 27

[Chris Cooper [:coop] (he/him)]

Attachment: Add proper clobber support to buildbot, v4 [checked in]

Sigh, so much cut-n-paste fail. This is the problem with having such a big delta between the leak and unit test masters. I don't suppose I get any points for getting it right on the first two patch series?

Errors fixed.

Comment 28

[Rob Campbell [:rc] (:robcee)]

oh yeah, you totally got points. Then they got taken away on the paste-fail. :)

I agree, these files are a real pain to deal with. Cut'n'paste programming is never ideal.

Comment 29

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 298648 [details] [diff] [review]
Add proper clobber support to buildbot, v4 [checked in]

I am a clod.

I recommended you use os.sep os.path.join in mozbuild.py but doesn work because that section runs on the server (lines 378, 410). I feel suitably stupid.

You could set logdir (if not set) in the MozillaClobber __init__  and then set it to a different value in MozillaClobberWin's __init__ method.

e.g., if not logDir: logDir = "logs/"

This should be good to go with those changes.

Comment 30

[Chris Cooper [:coop] (he/him)]

All checked in. Planning to piggyback on the downtime for bug 392788 to update the master cfg on qm-rhel02 -> currently scheduled for 10am PST on 2008/01/24.

Comment 31

[Chris Cooper [:coop] (he/him)]

Attachment: Create accessory dirs before accessing them for (potentially) the first time [checked in]

We had a couple of red cycles today after manual clobbers where the entire trunk_* slave dir was blown away and we lost the dirs we use to hold logs and the clobber_files. I've added a simple build step to check for/create those dirs before we access them for the first time in each cycle.

Comment 32

[Chris Cooper [:coop] (he/him)]

Instruction for clobbering can be found here:

http://wiki.mozilla.org/Build:ClobberingATinderbox