clobber in unittest directory

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: rcampbell, Assigned: coop)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

11 years ago
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.
(Reporter)

Updated

11 years ago
Assignee: nobody → ccooper
Priority: -- → P3
Status: NEW → ASSIGNED
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.
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
(Reporter)

Comment 3

11 years ago
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.
Oh, ok. My bad.
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.
(Reporter)

Comment 6

11 years ago
yeah, I was wondering about that too. Having our own directory for these might be the best way to go after all.
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?
(Reporter)

Comment 8

11 years ago
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
(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.

(Reporter)

Comment 10

11 years ago
sounds good. wfm!
Created attachment 280662 [details] [diff] [review]
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.
Attachment #280662 - Flags: review?(rcampbell)
(Reporter)

Comment 12

11 years ago
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.
Attachment #280662 - Flags: review?(rcampbell) → review-
(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?
Depends on: 402123
Any update on this? There have been times lately where I've only wanted to clobber the unit test machines instead of the nightlies.
Duplicate of this bug: 403399
I have this working now against the leak test boxes on all platforms. I'll work up a patch against the unittest master shortly.
Priority: P3 → P2
Created attachment 298328 [details] [diff] [review]
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).
Attachment #280662 - Attachment is obsolete: true
Attachment #298328 - Flags: review?(rcampbell)
Created attachment 298329 [details] [diff] [review]
Stop checking out the tinderbox-configs module
Attachment #298329 - Flags: review?(rcampbell)
Created attachment 298355 [details] [diff] [review]
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.
Attachment #298328 - Attachment is obsolete: true
Attachment #298355 - Flags: review?(rcampbell)
Attachment #298328 - Flags: review?(rcampbell)
(Reporter)

Comment 20

11 years ago
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.
Attachment #298329 - Flags: review?(rcampbell) → review+
(Reporter)

Comment 21

11 years ago
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.
Attachment #298355 - Flags: review?(rcampbell) → review+
(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.
(Reporter)

Comment 23

11 years ago
ugh. Thanks for the tip, Ben!
Created attachment 298487 [details] [diff] [review]
Remove mozilla/js/tests module as well [checked in]
Attachment #298329 - Attachment is obsolete: true
Attachment #298487 - Flags: review?(rcampbell)
Created attachment 298489 [details] [diff] [review]
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
Attachment #298355 - Attachment is obsolete: true
Attachment #298489 - Flags: review?(rcampbell)
(Reporter)

Updated

11 years ago
Attachment #298487 - Flags: review?(rcampbell) → review+
(Reporter)

Comment 26

11 years ago
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!
Attachment #298489 - Flags: review?(rcampbell) → review-
Created attachment 298648 [details] [diff] [review]
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.
Attachment #298489 - Attachment is obsolete: true
Attachment #298648 - Flags: review?(rcampbell)
(Reporter)

Comment 28

11 years ago
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.
(Reporter)

Comment 29

11 years ago
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.
Attachment #298648 - Flags: review?(rcampbell) → review+
Attachment #298487 - Attachment description: Remove mozilla/js/tests module as well → Remove mozilla/js/tests module as well [checked in]
Attachment #298648 - Attachment description: Add proper clobber support to buildbot, v4 → Add proper clobber support to buildbot, v4 [checked in]
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.
Created attachment 299111 [details]
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.
Attachment #299111 - Flags: review?(rcampbell)
Attachment #299111 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Updated

11 years ago
Attachment #299111 - Flags: review?(rcampbell) → review+
Attachment #299111 - Attachment description: Create accessory dirs before accessing them for (potentially) the first time → Create accessory dirs before accessing them for (potentially) the first time [checked in]
Instruction for clobbering can be found here:

http://wiki.mozilla.org/Build:ClobberingATinderbox
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Component: Testing → Release Engineering
Product: Core → mozilla.org
QA Contact: testing → release
Version: unspecified → other
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.