840427 - Migrate SeaMonkey builders to using mock

Status: RESOLVED FIXED

(SeaMonkey :: Release Engineering, defect)

Comment 1

[Edmund Wong (:ewong)]

Attachment: Config changes to mockify the builders.

Comment 2

[Edmund Wong (:ewong)]

Attachment: Config changes to mockify the builders. (v2)

Comment 3

[Edmund Wong (:ewong)]

Attachment: Buildbotcustom changes to enable mock for SeaMonkey builders (v1)

Comment 4

[Edmund Wong (:ewong)]

Attachment: Config changes to support mock with SeaMonkey builders (v3)

Comment 5

[Edmund Wong (:ewong)]

Attachment: Buildbotcustom changes to support Mock with SeaMonkey builders. (v2)

Comment 6

[Edmund Wong (:ewong)]

Attachment: Updated patch post-bug 1039139. (v3)

Bug 1039139 made some changes to the buildbotcustom code. Unbitrotted this.

Comment 7

[Justin Wood (:Callek)]

Comment on attachment 8456670 [details] [diff] [review]
Updated patch post-bug 1039139. (v3)

Review of attachment 8456670 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/factory.py
@@ +434,5 @@
>              clobberURL=None, clobberTime=None, buildsBeforeReboot=None,
>              branchName=None, baseWorkDir='build', hashType='sha512',
>              baseMirrorUrls=None, baseBundleUrls=None, signingServers=None,
> +            enableSigning=True, env={}, enable_pymake=False, use_mock=False,
> +            mock_target=None, mock_packages=None, mock_copying_files=None, **kwargs):

should be mock_copyin_files, not mock_copying_files

Comment 8

[Justin Wood (:Callek)]

Comment on attachment 8426856 [details] [diff] [review]
Config changes to support mock with SeaMonkey builders (v3)

Review of attachment 8426856 [details] [diff] [review]:
-----------------------------------------------------------------

Other than that I'm worried this could break machines/jobs/whatever in terms of running tests on mock machines, but I can't come up with how to fix that concern top of my head today....

Our first step anyway is building successfully.

Please adjust linux path to have:

                'PATH': '/tools/buildbot/bin:/usr/local/bin:/usr/lib/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin',

::: seamonkey/config.py
@@ +8,5 @@
>      'macosx64': #['cb-sea-miniosx64-%02i' % x for x in [1,2,3]] +
>                  ['sea-mini-osx64-%i' % x for x in range(1,5)],
> +    'mock': ['sea-vm-linux32-%i' % x for x in range(1,7)],
> +            ['sea-hp-linux64-%i' % x for x in range(2,7)],
> +            ['sea-vm-linux64-%i' % x for x in [1]],

can't use sea-vm-* here....  (can't duplicate actual slaves)

@@ +186,5 @@
> +                         'imake', # required for makedepend!?!
> +                         'gcc45_0moz3', 'yasm', 'ccache', # <-- from releng repo
> +                         'valgrind',
> +                         'pulseaudio-libs-devel',
> +                         ],

I almost suspect this list is too short, but lets see where this gets us before we fight with it.

@@ +187,5 @@
> +                         'gcc45_0moz3', 'yasm', 'ccache', # <-- from releng repo
> +                         'valgrind',
> +                         'pulseaudio-libs-devel',
> +                         ],
> +            'mock_copying_files': [('/home/seabld/.ssh', '/home/seabld/.hgrc'),]

should be mock_copyin_files (elsewhere too)

@@ +377,5 @@
> +                         'gcc45_0moz3', 'yasm', 'ccache', # <-- from releng repo
> +                         'valgrind',
> +                         'pulseaudio-libs-devel',
> +                         ],
> +                         [

typo/syntax error

Comment 9

[Justin Wood (:Callek)]

Comment on attachment 8456670 [details] [diff] [review]
Updated patch post-bug 1039139. (v3)

Review of attachment 8456670 [details] [diff] [review]:
-----------------------------------------------------------------

So after addressing all comments I made here, so far, I also hit:

  File "/builds/buildbot/master01/master/master.cfg", line 43, in <module>
    branchObjects = generateCCBranchObjects(BRANCHES[branch], branch)
  File "/builds/buildbot/master01/lib/python2.7/site-packages/buildbotcustom/misc.py", line 2324, in generateCCBranchObjects
    mozilla2_dep_factory = factory_class(**factory_kwargs)
  File "/builds/buildbot/master01/lib/python2.7/site-packages/buildbotcustom/process/factory.py", line 2781, in __init__
    mozconfigBranch='default', **kwargs)
  File "/builds/buildbot/master01/lib/python2.7/site-packages/buildbotcustom/process/factory.py", line 2311, in __init__
    MercurialBuildFactory.__init__(self, **kwargs)
  File "/builds/buildbot/master01/lib/python2.7/site-packages/buildbotcustom/process/factory.py", line 1018, in __init__
    mock=self.use_mock, target=self.mock_target))
  File "/builds/buildbot/master01/lib/python2.7/site-packages/buildbotcustom/steps/mock.py", line 53, in __init__
    assert 'workdir' in kwargs.keys(), "You *must* specify workdir"
AssertionError: You *must* specify workdir
make: *** [checkconfig] Error 1

trying to do a make checkconfig on our actual master with this applied.

::: misc.py
@@ +2311,5 @@
>                  'enable_pymake': enable_pymake,
> +                'use_mock': pf.get('use_mock'),
> +                'mock_target': pf.get('mock_target'),
> +                'mock_packages': pf.get('mock_packages'),
> +                'mock_copyin_files' pf.get('mock_copyin_files'),

syntax error/typo:  ': pf.get(

::: process/factory.py
@@ +458,5 @@
>          self.enable_pymake = enable_pymake
> +        self.use_mock = use_mock
> +        self.mock_target = mock_target
> +        self.mock_packages = mock_packages
> +        self.mock_copying_files = mock_copying_files

ditto here (and elsewhere)

@@ +4252,5 @@
>           description=['running', 'client.py', 'checkout'],
>           descriptionDone=['client.py', 'checkout'],
>           haltOnFailure=True,
>           workdir='%s/%s' % (self.baseWorkDir, self.origSrcDir),
> +         timeout=60*60*3 # 3 hours (crazy, but necessary for now),

syntax error, put the comma before the comment

::: steps/mock.py
@@ +1,1 @@
> +***** BEGIN LICENSE BLOCK *****

syntax error: need a comment marker at line start

@@ +289,5 @@
> +
> +MockMozillaCheck = addMockCommand(unittest_steps.MozillaCheck)
> +RetryingMockCommand = addRetryEvaluateCommand(MockCommand)
> +RetryingMockProperty = addRetryEvaluateCommand(MockProperty)
> +RetryingMockScratchboxProperty = addRetryEvaluteCommand(buidlbotcustom.steps.misc.ScratchboxProperty)

typo, should be: addRetryEvaluateCommand
typo, should be (buildbotcustom.steps

Comment 10

[Edmund Wong (:ewong)]

Attachment: config changes to support mock (v4)

Comment 11

[Edmund Wong (:ewong)]

Attachment: buildbotcustom changes to support Mock (v4)

Comment 12

[Edmund Wong (:ewong)]

Attachment: buildbotcustom changes to support Mock (v4)

Comment 13

[Edmund Wong (:ewong)]

Attachment: buildbotcustom changes to support Mock (v5)

Config passes.

Comment 14

[Edmund Wong (:ewong)]

Attachment: buildbot config changes to support Mock (v5)

Added new packages to the list of mock packages and moved the list of packages to GLOBAL_VARS, so that each platform that requires mock_packages only
refer to GLOBAL_VARS['mock_packages'].

Comment 15

[Edmund Wong (:ewong)]

Attachment: buildbotcustom changes to support Mock (v6)

Comment 16

[Edmund Wong (:ewong)]

Attachment: buildbot config changes to support Mock (v6)

Comment 17

[Edmund Wong (:ewong)]

Attachment: buildbotcustom changes to support Mock (v7)

wip

Comment 18

[Edmund Wong (:ewong)]

Attachment: buildbot config changes to support Mock (v7)

wip

Comment 19

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch (v8)

Comment 20

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch 1

Comment 21

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch 2

Comment 22

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch 3

Comment 23

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch 4

Comment 24

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch 5

Comment 25

[Edmund Wong (:ewong)]

Attachment: misc patch for some misc changes.

Comment 26

[Edmund Wong (:ewong)]

Patch sequence:  bcust.diff (patch 1), bcust2.diff (patch 2), bcust3.diff (patch3), misc.diff (misc), bcust4.diff (patch 4) and bcust5.diff (patch 5) in that order.

(here's where it gets confusing, which I'm sorry about): bcust3.diff is actually
the patch from bug 1050597.  (due to the dependencies.. it was required; though I probably could've approached this bug more organized...)

Comment 27

[Edmund Wong (:ewong)]

Attachment: buildbotcustom patch 3

Comment 28

[Edmund Wong (:ewong)]

Attachment: folded bcust*.diff + misc.diff into this patch for review. (v1)

Comment 29

[Justin Wood (:Callek)]

Comment on attachment 8475003 [details] [diff] [review]
folded bcust*.diff + misc.diff into this patch for review. (v1)

Review of attachment 8475003 [details] [diff] [review]:
-----------------------------------------------------------------

land ye, large patch, my eyes do falter so.
Its hard to understand you, but what do I know.

I merely see the outcome, real builds on beta,
Though how come, it took so long to get its data.

~A <s>haiku</s> poem written in exasperation
(felt fitting)

::: misc.py
@@ +2312,5 @@
> +                'use_mock': pf.get('use_mock'),
> +                'mock_target': pf.get('mock_target'),
> +                'mock_packages': pf.get('mock_packages'),
> +                'mock_copyin_files' : pf.get('mock_copyin_files'),
> +                'runAliveTests': pf.get('runAliveTests'),

runAliveTests is no longer needed

::: process/factory.py
@@ +1395,5 @@
> +#         warnOnWarnings=True,
> +#         workdir="build/%s" % self.objdir,
> +#         timeout=5*60, # 5 minutes.
> +#         env=env,
> +#        )

is this env addition needed? (either way delete the commented out piece)

@@ +2604,5 @@
>      def __init__(self, skipBlankRepos=False, mozRepoPath='',
>                   inspectorRepoPath='', venkmanRepoPath='',
> +                 chatzillaRepoPath='', cvsroot='', runAliveTests=False,
> +                 use_mock=False, mock_target=None, mock_packages=None,
> +                 mock_copyin_files=None, **kwargs):

runAliveTests is no longer needed

@@ +2611,5 @@
>          self.inspectorRepoPath = inspectorRepoPath
>          self.venkmanRepoPath = venkmanRepoPath
>          self.chatzillaRepoPath = chatzillaRepoPath
>          self.cvsroot = cvsroot
> +        self.runAliveTests = runAliveTests

runAliveTests is no longer needed

@@ +7302,5 @@
>          self.thisChunk = thisChunk
>          self.chunkByDir = chunkByDir
> +        self.use_mock = kwargs.get('use_mock')
> +        self.mock_target = kwargs.get('mock_target')
> +        self.mock_packages = kwargs.get('mock_packages')

not really a fan of setting mock stuff for unittests things, but at this point I'm not as deeply invested in unittests as I am for builds so leave-in is fine

::: steps/unittest.py
@@ +809,5 @@
>  
> +        env_os = kwargs.get('env', {})
> +        pythonCmd = 'python'
> +        if env_os['OS'].fmtstring == "WIN":
> +            pythonCmd = 'python2.7'

this feels hackish, but again not going to block this on test changes atm

Comment 30

[Edmund Wong (:ewong)]

(In reply to Justin Wood (:Callek) from comment #29)
> ::: steps/unittest.py
> @@ +809,5 @@
> >  
> > +        env_os = kwargs.get('env', {})
> > +        pythonCmd = 'python'
> > +        if env_os['OS'].fmtstring == "WIN":
> > +            pythonCmd = 'python2.7'
> 
> this feels hackish, but again not going to block this on test changes atm

This is related to the issue(I think) from bug 908090.

Comment 31

[Edmund Wong (:ewong)]

Attachment: Buildbotcustom patch - checkin patch

Comment 32

[Edmund Wong (:ewong)]

Comment on attachment 8475681 [details] [diff] [review]
Buildbotcustom patch - checkin patch

Pushed to buildbotcustom:
https://hg.mozilla.org/build/buildbotcustom/rev/fc8352b5740e

Comment 33

[Edmund Wong (:ewong)]

Attachment: Buildbot-config patch : p1

Comment 34

[Edmund Wong (:ewong)]

Attachment: Buildbot-config patch: p2

Comment 35

[Edmund Wong (:ewong)]

Attachment: buildbot-config patch: for review

Comment 36

[Edmund Wong (:ewong)]

Attachment: buildbot-config patch: for review (incl new machines)

Comment 37

[Justin Wood (:Callek)]

Comment on attachment 8476421 [details] [diff] [review]
buildbot-config patch: for review (incl new machines)

Review of attachment 8476421 [details] [diff] [review]:
-----------------------------------------------------------------

::: seamonkey/config.py
@@ +111,5 @@
>      'weekly_tinderbox_tree': 'Testing',
>      'l10n_tinderbox_tree': 'Mozilla-l10n',
>      'tinderbox_tree': 'MozillaTest',
>      'pgo_strategy': None,
> +    'runAliveTests': False,

no longer needed if you account for my comments from the other patch (same applies elsewhere in here)

Comment 38

[Edmund Wong (:ewong)]

Attachment: buildbot-config patch: for checkin

removed runAliveTest.

Comment 39

[Edmund Wong (:ewong)]

Comment on attachment 8476429 [details] [diff] [review]
buildbot-config patch: for checkin

Pushed to buildbot-configs:
 https://hg.mozilla.org/build/buildbot-configs/rev/0d4a7712775c

Comment 40

[Edmund Wong (:ewong)]

Attachment: bugfix for the buildbotcustom checked in code.

r+ from Callek over IRC

Comment 41

[Edmund Wong (:ewong)]

Comment on attachment 8476443 [details] [diff] [review]
bugfix for the buildbotcustom checked in code.

Pushed to buildbotcustom:
https://hg.mozilla.org/build/buildbotcustom/rev/1d60e4f2efa8

Comment 42

[Edmund Wong (:ewong)]

Attachment: patch to fix the |make complete mar| step.

Comment 43

[Edmund Wong (:ewong)]

Attachment: consolidated buildbot-config patch to fix issues from previous patches.

Comment 44

[Edmund Wong (:ewong)]

Attachment: consolidated buildbotcustom patch to fix issues from checked-in patch.

Comment 45

[Edmund Wong (:ewong)]

Attachment: consolidated buildbot-config patch to fix issues from previous patches. (v2)

Comment 46

[Edmund Wong (:ewong)]

Attachment: consolidated buildbot-config patch to fix issues from previous patches. (v3)

Comment 47

[Edmund Wong (:ewong)]

Attachment: consolidated buildbotcustom patch to fix issues from checked-in patch. (v2)

Comment 48

[Justin Wood (:Callek)]

Comment on attachment 8478814 [details] [diff] [review]
consolidated buildbot-config patch to fix issues from previous patches. (v3)

Review of attachment 8478814 [details] [diff] [review]:
-----------------------------------------------------------------

I know this is post-landing review, but the nits can be landed as followup, at your leisure.

::: seamonkey/ccfactory.py
@@ +152,4 @@
>          # the autoconf and actual tarring steps
>          # should be replaced by calling the build target
>          for dir in autoconfDirs:
> +            self.addStep(MockCommand(name='autoconf2.13',

ignorable nit: name='...' on a newline aligned with the other args.

::: seamonkey/config.py
@@ +150,4 @@
>                              # version is available, but we don't want it for Firefox builds.
>                              'freetype-2.3.11-6.el6_1.8.i686', 'freetype-devel-2.3.11-6.el6_1.8.i686',
>                              'freetype-2.3.11-6.el6_1.8.x86_64',
> +                            'cvs', 'rsh',

nit: comment worded something like  "seamonkey needs these for update runs until Bug XXX is fixed" where XXX is the "no longer use cvs" bug

@@ +168,4 @@
>                              'gstreamer-devel', 'gstreamer-plugins-base-devel',
>                              'freetype-2.3.11-6.el6_1.8.x86_64',
>                              'freetype-devel-2.3.11-6.el6_1.8.x86_64',
> +                            'cvs', 'rsh',

nit: same comment as above

::: seamonkey/release_master.py
@@ +445,4 @@
>      })
>      
>  releaseChannel = releaseConfig.get('releaseChannel', branchConfig['update_channel'])
> +linuxConfig = branchConfig['platforms']['linux']

nit: might as well s/linuxBranch/linuxConfig/ above and remove this one.  (as in, set this var up above and don't use the var name "linuxBranch")

Comment 49

[Edmund Wong (:ewong)]

(In reply to Justin Wood (:Callek) from comment #48)
> Comment on attachment 8478814 [details] [diff] [review]
> consolidated buildbot-config patch to fix issues from previous patches. (v3)
> 
> Review of attachment 8478814 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I know this is post-landing review, but the nits can be landed as followup,
> at your leisure.
> 
It isn't a post-landing review. :) I haven't pushed this bugfix patch nor
the consolidated bugfix buildbotcustom patch.  (I know, kinda getting a bit
confusing.  )

All these patches are applied manually.

Comment 50

[Edmund Wong (:ewong)]

Comment on attachment 8478814 [details] [diff] [review]
consolidated buildbot-config patch to fix issues from previous patches. (v3)

Pushed to buildbot-configs:
https://hg.mozilla.org/build/buildbot-configs/rev/3bfb9c7b97e8

Comment 51

[Edmund Wong (:ewong)]

Attachment: Consolidated buildbotcustom patch to fix issues from checked-in patch (v3)

This is an updated patch from (v2).  v2 was missing some stuff.

The difference is the additional env and updatemockverify.

(sorry for the bug churn)

Comment 52

[Edmund Wong (:ewong)]

Comment on attachment 8491983 [details] [diff] [review]
Consolidated buildbotcustom patch to fix issues from checked-in patch (v3)

Moved to bug 1072713.

Comment 53

[Edmund Wong (:ewong)]

Fixed in bug 1072713