418449 - Build And Automation Code Refactoring/Consolidation

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Mikeal Rogers [:mikeal] (mrogers@)]

This is a task to go through the various build/automation code bases and find common points that could be consolidated or refactored.

The two currently on my list to look through are;
http://lxr.mozilla.org/mozilla/source/tools/buildbotcustom/tryserver/
http://lxr.mozilla.org/mozilla/source/tools/buildbot-configs/testing/sisyphus

I'm sure more will be added to this bug via the comments.

I'm also still looking for input on how the new consolidated code can be available in each environment.

Comment 1

[[e-mail] (:bhearsum)]

Hey Mikeal,

You should have a look through all of the custom steps in http://lxr.mozilla.org/mozilla/source/tools/buildbot-configs. Ideally, would we want most of them generalized and put into mozilla/tools/buildbotcustom.

In some places there's lots of platform specific classes. I think these could easily be refactor into one class that takes an argument of platform. IIRC Talos/Unit test machines are a big offender here. Things like "Unpack Build" (MozillaInstall* in the Talos configs) are what I'm talking about here. Mostly, the only thing that's different is the command line.

Comment 2

[Mikeal Rogers [:mikeal] (mrogers@)]

Any objections or issues with just figuring out which platform the build is on rather than having to call it out explicitly?

Comment 3

[[e-mail] (:bhearsum)]

That's more difficult, but likely better. Any code that is a subclass of buildbot.process.buildstep.BuildStep is run on the *master* side. So doing a platform.system() call won't help you figure out what OS the slave is. It would be awesome to have something in buildbot.status.builder.SlaveStatus that told you what platform it ran on. Just a proxy to platform.system() on the slave would be greatly helpful.

I don't think this will be all too easy and it will _definitely_ require redeployment of Buildbot on the slaves, so I think I'm actually in favour of just passing the platform to the BuildStep...I'd like 

Comment 4

[Mikeal Rogers [:mikeal] (mrogers@)]

Ahh, ok. I'll just do it by keyword argument for now and then if we get platform detection in SlaveStatus we can add that in the case the param is omitted.

Comment 5

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

I'm of two minds on removing platform-specific subclasses. For one, it actually makes sense to subclass rather than wrap a bunch of platform behavior in if statements. You end up with nested conditions that can get messy to maintain over time if you need to add more logic to the step. This is actually a great use of subclasses, imo, especially considering the different invocation conventions on windows vs the 'nix platforms.

I was hoping to find buildsteps like "get file from webserver" to consolidate into the buildbotcustom package for reuse. If that includes a variant for windows, then that's ok too.

Note that Twisted's own process classes and packages are all subclassed for platform too.

Comment 6

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

also, look in:

http://lxr.mozilla.org/mozilla/source/tools/buildbot-configs/testing/unittest/mozbuild.py

and 

http://lxr.mozilla.org/mozilla/source/tools/buildbot-configs/testing/talos/perfmaster/perfrunner.py

for sources to mine.

Comment 7

[[e-mail] (:bhearsum)]

Hm. So, the reason I am partial to passing a parameter is because it can make the master.cfg simpler (by only needing one set of BuildSteps for a given task. ie, not one per platform). After looking at it again I realize that this only works if the master.cfg is done programatically, like the moz2 configs (http://hg.mozilla.org/build/buildbot-configs).

So, given this revelation I am in agreement with comment #5.

Comment 8

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

yeah, after posting that reply and thinking about it a bit, I realized we end up with simpler master.cfgs if we have single-named, non-platform-specific steps. We could double-dispatch methods inside the custom build steps for the platform.

e.g., in a ShellCommand subclass:

def start():
    if self.platform == 'win32':
        self.startWin32()
    ...

I still think this is messier over-all than having concrete subclasses per platform and would even be possible to add some smarts to whatever factories are building master.cfg files.

Comment 9

[Robert Helmer [:rhelmer]]

The release automation buildbot configs are here:

http://mxr.mozilla.org/mozilla/source/tools/buildbot-configs/automation/

They are quite a bit simpler than most of the others, because most of the work is done by http://mxr.mozilla.org/mozilla/source/tools/release/ aka "bootstrap". 

Bootstrap is our bridge between the old tinderbox world and the new Buildbot world, so we're planning on moving stuff out of there and into buildbot or the underlying build system (or new scripts/frameworks called by the build system), etc. So, in time we expect to use the same or custom similar classes as unittest, moz2, tryserver, etc. 

The way the Mozilla2 Buildbot stuff works is closer to ideal, but does not do everything needed for releases.

Just FYI :) It'd be totally hot if we could get all the various buildbot config and custom classes and junk into some kind of order (common style would be a nice start!). 

Comment 10

[Mikeal Rogers [:mikeal] (mrogers@)]

I've been looking through this a bit.

Removing the subclasses sounds like it's out, but we can still satisfy the Ben's need to keep the master.cfg cleaner when generating it programmatically by just sticking a dictionary at the end of the module with all the platform specific classes keyed by platform.

I'm seeing some code overlap in individual configs, where rather than subclassing stuff is getting copy/pasted and slightly altered. I can clean this up if it's worth my time? Most of these look like they aren't getting reused often for writing new configs and install classes anyway.

Another thing is that all the talos subprojects have the same bits of code copy/pasted in to each subproject because there isn't a good way to share that code between them. Some of this could be reused outside of talos, like all the MozillaInstall stuff, so that might be ripe for addition in to buildbotcustom.

Other than that I'm failing to see any huge overlap between configs in different projects, but there is a lot of code here. 

Sure, I'm seeing some things I might do differently, or slight style differences between files, but nothing to get worked up about.

Are there any other issues I should be looking in to?

Are the aforementioned issue worth putting time in to consolidating?

Comment 11

[[e-mail] (:bhearsum)]

I am in favour of moving most, if not all of the Mozilla* classes to buildbotcustom. Even thingsn that are only useful for one task (MozillaRunPerfTests) will benefit by not being duplicated in many configs.

Comment 12

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

I've been thinking about this a bit lately and thinking that the Moz2 unittesting stuff might be a good place to start using code from buildbot custom. I agree with Ben that the Mozilla* classes (e.g., MozillaMochitest) make sense to get stuck into buildbot-custom. I also think you're on the right track with the talos stuff, Mikeal. I do think it's worthwhile, it's just kind of a messy bunch of code to deal with. Moz2 might be a place where we can consolidate this stuff moving forward and then back-port as necessary.

Comment 13

[Mikeal Rogers [:mikeal] (mrogers@)]

should i put together a patch for buildbot-custom with modules containing all the Mozilla* classes?

It's pretty easy to do and if it helps get this off the ground then it's worth doing. The only thing I'm concerned about is if we aren't planning on moving the currently running code to using buildbot-custom that those classes might get stale over time and changes aren't going to make it in.

What are your thoughts?

Comment 14

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

(In reply to comment #13)
> should i put together a patch for buildbot-custom with modules containing all
> the Mozilla* classes?

Please!

> It's pretty easy to do and if it helps get this off the ground then it's worth
> doing. The only thing I'm concerned about is if we aren't planning on moving
> the currently running code to using buildbot-custom that those classes might
> get stale over time and changes aren't going to make it in.

yeah, that's my concern as well. I expect there's going to be quite a bit of activity on 1.9 for awhile to come and we don't want to end up with a bunch of dead-end code that nobody knows how to operate and patch.

So we should probably plan on back-porting the buildbot-custom stuff at some point.

Comment 15

[Mikeal Rogers [:mikeal] (mrogers@)]

Attachment: add Mozilla* classes to buildbotcustom patch1

Before applying patch.

$ mkdir talos unittest
$ touch talos/__init__.py unittest/__init__.py

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 308737 [details] [diff] [review]
add Mozilla* classes to buildbotcustom patch1

>Index: talos/perfrunner.py

>+MozillaEnvironments = { }
>+
>+# platform SDK location.  we can build both from one generic template.
>+# modified from vc8 environment
>+MozillaEnvironments['vc8perf'] = {
>+    "MOZ_CRASHREPORTER_NO_REPORT": '1',
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "CYGWINBASE": 'C:\\cygwin',
>+    "PATH": 'C:\\Python24;' + 
>+            'C:\\Python24\\Scripts;' + 
>+            'C:\\cygwin\\bin;' + 
>+            'C:\\WINDOWS\\System32;' + 
>+            'C:\\program files\\gnuwin32\\bin;' + 
>+            'C:\\WINDOWS;'
>+}
>+
>+MozillaEnvironments['linux'] = {
>+    "MOZ_CRASHREPORTER_NO_REPORT": '1',
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "DISPLAY": ":0",
>+}
>+
>+MozillaEnvironments['mac'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "MOZ_CRASHREPORTER_NO_REPORT": '1',
>+    # for extracting dmg's
>+    "PAGER": '/bin/cat',
>+}

>Index: unittest/mozbuild.py

>+MozillaEnvironments = { }
>+
>+MozillaEnvironments['linux'] = {
>+    "DISPLAY": ':2',
>+    "MOZ_NO_REMOTE": '1'
>+}
>+
>+MozillaEnvironments['centos'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "CVS_RSH": 'ssh'
>+}
>+
>+MozillaEnvironments['osx'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "CVS_RSH": 'ssh'
>+}
>+
>+# standard vc8 express build env; vc8 normal will be very similar, 
>+# just different platform SDK location.  we can build both from one 
>+# generic template.
>+MozillaEnvironments['vc8'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "VCVARS": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\bin\\vcvars32.bat',
>+    "MOZ_TOOLS": 'C:\\moztools',
>+    "CYGWINBASE": 'C:\\cygwin',
>+    "CVS_RSH": 'ssh',
>+    "VSINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8',
>+    "VCINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "FrameworkDir": 'C:\\WINDOWS\\Microsoft.NET\\Framework',
>+    "FrameworkVersion": 'v2.0.50727',
>+    "FrameworkSDKDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0',
>+    "MSVCDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "DevEnvDir": "C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE",
>+    "PATH": 'C:\\Python24;' + \
>+            'C:\\Python24\\Scripts;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\BIN;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\bin;' + \
>+            'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\VCPackages;' + \
>+            'C:\\cygwin\\bin;' + \
>+            'C:\\WINDOWS\\System32;' + \
>+            'C:\\WINDOWS;' + \
>+            'C:\\WINDOWS\System32\Wbem;' + \
>+            'C:\\moztools\\bin;' + \
>+            'C:\\Utilities;',
>+    "INCLUDE": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\include;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\include;' + \
>+               '%INCLUDE%',
>+    "LIB": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\lib;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\lib;',
>+    "LIBPATH": 'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB'
>+}
>+
>+MozillaEnvironments['mozbuild'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "MOZ_AIRBAG": '1',
>+    "MOZ_CRASHREPORTER_NO_REPORT": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "VCVARS": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\bin\\vcvars32.bat',
>+    "MOZ_MSVCVERSION": '8',
>+    "MOZILLABUILD": 'C:\\mozilla-build',
>+    "MOZILLABUILDDRIVE": 'C:',
>+    "MOZILLABUILDPATH": '\\mozilla-build\\',
>+    "MOZ_TOOLS": 'C:\\mozilla-build\\moztools',
>+    "CVS_RSH": 'ssh',
>+    "VSINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8',
>+    "VCINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "FrameworkDir": 'C:\\WINDOWS\\Microsoft.NET\\Framework',
>+    "FrameworkVersion": 'v2.0.50727',
>+    "FrameworkSDKDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0',
>+    "MSVCDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "DevEnvDir": "C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE",
>+    "PATH": 'C:\\mozilla-build\\msys\\local\\bin;' + \
>+            'C:\\mozilla-build\\wget;' + \
>+            'C:\\mozilla-build\\7zip;' + \
>+            'C:\\mozilla-build\\blat261\\full;' + \
>+            'C:\\mozilla-build\\svn-win32-1.4.2\\bin;' + \
>+            'C:\\mozilla-build\\upx203w;' + \
>+            'C:\\mozilla-build\\xemacs\\XEmacs-21.4.19\\i586-pc-win32;' + \
>+            'C:\\mozilla-build\\info-zip;' + \
>+            'C:\\mozilla-build\\nsis-2.22;' + \
>+            '.;' + \
>+            'C:\\mozilla-build\\msys\\bin;' + \
>+            'C:\\Python24;' + \
>+            'C:\\Python24\\Scripts;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\BIN;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\bin;' + \
>+            'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\VCPackages;' + \
>+            'C:\\WINDOWS\\System32;' + \
>+            'C:\\WINDOWS;' + \
>+            'C:\\WINDOWS\\System32\\Wbem;' + \
>+            'C:\\mozilla-build\\moztools\\bin;',
>+    "INCLUDE": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\include;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\include;' + \
>+               '%INCLUDE%',
>+    "LIB": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\lib;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\lib;',
>+    "LIBPATH": 'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB'
>+}


I keep going back and forth on whether or not these should be in buildbotcustom.env. I'm leaning towards "yes" especially for the unit test ones. In fact, I think they should be derived from the existing buildbotcustom.env (by making a copy of the existing env and extending/overriding where necessary). I'm interested to know what others think.

r+, basically, but depending how others feel we may want to re-work the env stuff a bit.

Comment 17

[Robert Helmer [:rhelmer]]

Comment on attachment 308737 [details] [diff] [review]
add Mozilla* classes to buildbotcustom patch1

Sorry, I don't have time to review this.. robcee and bhearsum should be more than enough :)

Comment 18

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

Comment on attachment 308737 [details] [diff] [review]
add Mozilla* classes to buildbotcustom patch1

sorry, should have been a bit more specific. First, I don't see a pathname in the diff. This should be something like mozilla/tools/buildbotcustom/steps/performance.py for the perfrunner stuff and buildbotcustom/steps/unittest.py for the mozbuild.py bits. Not sure we really want to include the environments in these either. There is a top-level env.py under buildbotcustom, but these are going to be somewhat specific and volatile depending where they're deployed. Probably best to leave them in the stripped down mozbuild.py and perfrunner.py files.

hope this clarifies a little!

Comment 19

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

I missed Ben's suggestion from comment #17 before. I agree that the unittest envs should probably be in the top-level "env.py" file. 

Comment 20

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

I'm going to grab this back for a second. I'm hitting this in the moz2 unittest setup and figured I'd throw a patch in.

Comment 21

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

Attachment: buildbot custom unittest steps.patch

this is an attempt at adding in the unittest steps to buildbotcustom. I've added license headers to files I've touched and copied the env into env.py.

I think the unittest steps still need some better explanation in comments, but this is what I've got so far.

Comment 22

[[e-mail] (:bhearsum)]

Comment on attachment 311819 [details] [diff] [review]
buildbot custom unittest steps.patch

>Index: env.py
>===================================================================
>+MozillaEnvironments['win32-vc8-cygwin-unittest'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "VCVARS": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\bin\\vcvars32.bat',
>+    "MOZ_TOOLS": 'C:\\moztools',
>+    "CYGWINBASE": 'C:\\cygwin',
>+    "CVS_RSH": 'ssh',
>+    "VSINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8',
>+    "VCINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "FrameworkDir": 'C:\\WINDOWS\\Microsoft.NET\\Framework',
>+    "FrameworkVersion": 'v2.0.50727',
>+    "FrameworkSDKDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0',
>+    "MSVCDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "DevEnvDir": "C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE",
>+    "PATH": 'C:\\Python24;' + \
>+            'C:\\Python24\\Scripts;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\BIN;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\bin;' + \
>+            'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\VCPackages;' + \
>+            'C:\\cygwin\\bin;' + \
>+            'C:\\WINDOWS\\System32;' + \
>+            'C:\\WINDOWS;' + \
>+            'C:\\WINDOWS\System32\Wbem;' + \
>+            'C:\\moztools\\bin;' + \
>+            'C:\\Utilities;',
>+    "INCLUDE": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\include;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\include;' + \
>+               '%INCLUDE%',
>+    "LIB": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\lib;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\lib;',
>+    "LIBPATH": 'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB'
>+}
>+
>+MozillaEnvironments['win32-vc8-mozbuild-unittest'] = {
>+    "MOZ_NO_REMOTE": '1',
>+    "NO_EM_RESTART": '1',
>+    "MOZ_AIRBAG": '1',
>+    "MOZ_CRASHREPORTER_NO_REPORT": '1',
>+    "XPCOM_DEBUG_BREAK": 'warn',
>+    "VCVARS": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\bin\\vcvars32.bat',
>+    "MOZ_MSVCVERSION": '8',
>+    "MOZILLABUILD": 'C:\\mozilla-build',
>+    "MOZILLABUILDDRIVE": 'C:',
>+    "MOZILLABUILDPATH": '\\mozilla-build\\',
>+    "MOZ_TOOLS": 'C:\\mozilla-build\\moztools',
>+    "CVS_RSH": 'ssh',
>+    "VSINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8',
>+    "VCINSTALLDIR": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "FrameworkDir": 'C:\\WINDOWS\\Microsoft.NET\\Framework',
>+    "FrameworkVersion": 'v2.0.50727',
>+    "FrameworkSDKDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0',
>+    "MSVCDir": 'C:\\Program Files\\Microsoft Visual Studio 8\\VC',
>+    "DevEnvDir": "C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE",
>+    "PATH": 'C:\\mozilla-build\\msys\\local\\bin;' + \
>+            'C:\\mozilla-build\\wget;' + \
>+            'C:\\mozilla-build\\7zip;' + \
>+            'C:\\mozilla-build\\blat261\\full;' + \
>+            'C:\\mozilla-build\\svn-win32-1.4.2\\bin;' + \
>+            'C:\\mozilla-build\\upx203w;' + \
>+            'C:\\mozilla-build\\xemacs\\XEmacs-21.4.19\\i586-pc-win32;' + \
>+            'C:\\mozilla-build\\info-zip;' + \
>+            'C:\\mozilla-build\\nsis-2.22;' + \
>+            '.;' + \
>+            'C:\\mozilla-build\\msys\\bin;' + \
>+            'C:\\Python24;' + \
>+            'C:\\Python24\\Scripts;' + \
>+            'C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\IDE;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\BIN;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\Common7\\Tools\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\bin;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\bin;' + \
>+            'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+            'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\VCPackages;' + \
>+            'C:\\WINDOWS\\System32;' + \
>+            'C:\\WINDOWS;' + \
>+            'C:\\WINDOWS\\System32\\Wbem;' + \
>+            'C:\\mozilla-build\\moztools\\bin;',
>+    "INCLUDE": 'C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\include;' + \
>+               'C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\include\\atl;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\INCLUDE;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\include;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\include;' + \
>+               '%INCLUDE%',
>+    "LIB": 'C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\lib;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\LIB;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\PlatformSDK\\lib;' + \
>+           'C:\\Program Files\\Microsoft Visual Studio 8\\SDK\\v2.0\\lib;',
>+    "LIBPATH": 'C:\\WINDOWS\\Microsoft.NET\\Framework\\v2.0.50727;' + \
>+               'C:\\Program Files\\Microsoft Visual Studio 8\\VC\\ATLMFC\\LIB'
>+}

I'm really torn on this...there's a lot of repetition here. I've always had this idea in my head the env vars could inherit from one another. These two are so similar...how do you feel about having a "base dict" that contains everything except the PATH; these two classes could deepcopy() it and add PATH. I'm worried that we'll end up with tiny variations for specific tasks which could cause a lot of pain when, eg. an SDK changes.

I'm not entirely set on this..it's a bit of during-review-brainstorming and it could just be the lingering OOP student in me speaking.

>Index: steps/misc.py
>===================================================================
>+class CreateDir(ShellCommand):

I guess this class is to work around win32's "BuildStep will fail if directory exists". What a pain. I hope he we can rid ourselves of it at some point. Someone (you, I think) suggested a "run Python code passed from master" BuildStep -- we could probably do it without a custom class if that exists.

>+class TinderboxShellCommand(ShellCommand):

Love the naming on this one.



Going to r+, but I'm very interested to hear your thoughts on the env vars.

Comment 23

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

yeah, I was actually thinking about the environments last night after adding this patch and wondering if we shouldn't name task-specific environment dictionaries differently.

E.g., MozUnittestEnv['win32-vc8-mozbuild-unittest'].

Then we could do a from env import MozUnittestEnv and not get all the others in a master.cfg.

But that's separate from what you're saying. Ideally, we shouldn't really need any changes from the refplatform image once we start using them for running unittests, so I actually expect these environments to go away. I just copied them wholesale out of the existing unittest mozbuild.py file and did a little renaming. Certainly the cygwin version is deprecated and can probably be removed for moz2 and future work.

What do you think? Should I remove these altogether and make deepCopies as-needed for additional required unittesting functionality?

Comment 24

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

Comment on attachment 311819 [details] [diff] [review]
buildbot custom unittest steps.patch

Checking in env.py;
/cvsroot/mozilla/tools/buildbotcustom/env.py,v  <--  env.py
new revision: 1.5; previous revision: 1.4
done
Checking in steps/misc.py;
/cvsroot/mozilla/tools/buildbotcustom/steps/misc.py,v  <--  misc.py
new revision: 1.4; previous revision: 1.3
done
RCS file: /cvsroot/mozilla/tools/buildbotcustom/unittest/__init__.py,v
done
Checking in unittest/__init__.py;
/cvsroot/mozilla/tools/buildbotcustom/unittest/__init__.py,v  <--  __init__.py
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/tools/buildbotcustom/unittest/steps.py,v
done
Checking in unittest/steps.py;
/cvsroot/mozilla/tools/buildbotcustom/unittest/steps.py,v  <--  steps.py
initial revision: 1.1
done

Comment 25

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

Attachment: [checked in] buildbotcustom runtests patch

realized we should be converting these to use runtests.py instead of the perl version.

Comment 26

[[e-mail] (:bhearsum)]

Comment on attachment 314077 [details] [diff] [review]
[checked in] buildbotcustom runtests patch

Looks good.

Comment 27

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

Comment on attachment 314077 [details] [diff] [review]
[checked in] buildbotcustom runtests patch

Checking in unittest/steps.py;
/cvsroot/mozilla/tools/buildbotcustom/unittest/steps.py,v  <--  steps.py
new revision: 1.2; previous revision: 1.1
done

Comment 28

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

added unittest classes, throwing this back on the pile. Still to do: add talos classes and environments.