Closed Bug 840427 Opened 7 years ago Closed 3 years ago

Migrate SeaMonkey builders to using mock

Categories

(SeaMonkey :: Release Engineering, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ewong, Assigned: ewong)

References

(Blocks 1 open bug)

Details

Attachments

(12 files, 26 obsolete files)

261.15 KB, patch
Details | Diff | Splinter Review
3.11 KB, patch
Details | Diff | Splinter Review
11.34 KB, patch
Details | Diff | Splinter Review
7.51 KB, patch
Details | Diff | Splinter Review
5.81 KB, patch
Details | Diff | Splinter Review
16.95 KB, patch
Details | Diff | Splinter Review
290.20 KB, patch
ewong
: review+
Details | Diff | Splinter Review
16.61 KB, patch
Details | Diff | Splinter Review
2.18 KB, patch
Details | Diff | Splinter Review
20.64 KB, patch
ewong
: review+
Details | Diff | Splinter Review
10.57 KB, patch
Callek
: review+
Details | Diff | Splinter Review
38.71 KB, patch
Details | Diff | Splinter Review
No description provided.
Depends on: 840429
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #713789 - Flags: feedback?(bugspam.Callek)
Attachment #713815 - Flags: feedback?(bugspam.Callek)
Attachment #713789 - Attachment is obsolete: true
Attachment #713789 - Flags: feedback?(bugspam.Callek)
Attachment #716505 - Flags: review?(bugspam.Callek)
Attachment #713815 - Attachment is obsolete: true
Attachment #713815 - Flags: feedback?(bugspam.Callek)
Attachment #8426856 - Flags: review?(bugspam.Callek)
Attachment #716505 - Attachment is obsolete: true
Attachment #716505 - Flags: review?(bugspam.Callek)
Attachment #8426857 - Flags: review?(bugspam.Callek)
Bug 1039139 made some changes to the buildbotcustom code. Unbitrotted this.
Attachment #8426857 - Attachment is obsolete: true
Attachment #8426857 - Flags: review?(bugspam.Callek)
Attachment #8456670 - Flags: review?(bugspam.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 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
Attachment #8426856 - Flags: review?(bugspam.Callek) → review+
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
Attachment #8456670 - Flags: review?(bugspam.Callek) → review-
Attachment #8426856 - Attachment is obsolete: true
Attachment #8457692 - Flags: review+
Attachment #8456670 - Attachment is obsolete: true
Attachment #8457694 - Flags: review?(bugspam.Callek)
Attachment #8457694 - Attachment is obsolete: true
Attachment #8457694 - Flags: review?(bugspam.Callek)
Attachment #8457697 - Flags: review?(bugspam.Callek)
Config passes.
Attachment #8457697 - Attachment is obsolete: true
Attachment #8457697 - Flags: review?(bugspam.Callek)
Attachment #8457752 - Flags: review?(bugspam.Callek)
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'].
Attachment #8457692 - Attachment is obsolete: true
Attachment #8457766 - Flags: review?(bugspam.Callek)
Attachment #8457752 - Attachment is obsolete: true
Attachment #8457752 - Flags: review?(bugspam.Callek)
Attachment #8457766 - Attachment is obsolete: true
Attachment #8457766 - Flags: review?(bugspam.Callek)
Depends on: 1047715
wip
Attachment #8466159 - Attachment is obsolete: true
wip
Attachment #8466160 - Attachment is obsolete: true
Attached patch buildbotcustom patch (v8) (obsolete) — Splinter Review
Attachment #8469039 - Attachment is obsolete: true
Depends on: 1050597
Depends on: 853720
Attachment #8469299 - Attachment is obsolete: true
Attachment #8474990 - Flags: review?
Attachment #8474990 - Flags: review?
Attached patch buildbotcustom patch 3 (obsolete) — Splinter Review
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...)
Attachment #8474992 - Attachment is obsolete: true
Attachment #8475003 - Flags: review?(bugspam.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
Attachment #8475003 - Flags: review?(bugspam.Callek) → review+
(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.
Attachment #8475003 - Attachment is obsolete: true
Attachment #8475681 - Flags: review+
Comment on attachment 8475681 [details] [diff] [review]
Buildbotcustom patch - checkin patch

Pushed to buildbotcustom:
https://hg.mozilla.org/build/buildbotcustom/rev/fc8352b5740e
Attachment #8469040 - Attachment is obsolete: true
Attachment #8476402 - Flags: review?
Attachment #8476402 - Flags: review? → review?(bugspam.Callek)
Attachment #8476402 - Attachment is obsolete: true
Attachment #8476402 - Flags: review?(bugspam.Callek)
Attachment #8476421 - Flags: review?(bugspam.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)
Attachment #8476421 - Flags: review?(bugspam.Callek) → review+
removed runAliveTest.
Attachment #8476421 - Attachment is obsolete: true
Attachment #8476429 - Flags: review+
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
r+ from Callek over IRC
Attachment #8476443 - Flags: review+
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
Attachment #8477184 - Flags: review?(bugspam.Callek)
Attachment #8477184 - Attachment is obsolete: true
Attachment #8477184 - Flags: review?(bugspam.Callek)
Attachment #8478058 - Flags: review?(bugspam.Callek)
Attachment #8476443 - Attachment is obsolete: true
Attachment #8478059 - Flags: review?(bugspam.Callek)
Attachment #8478058 - Attachment is obsolete: true
Attachment #8478058 - Flags: review?(bugspam.Callek)
Attachment #8478065 - Flags: review?(bugspam.Callek)
Attachment #8478065 - Attachment is obsolete: true
Attachment #8478065 - Flags: review?(bugspam.Callek)
Attachment #8478814 - Flags: review?(bugspam.Callek)
Attachment #8478059 - Attachment is obsolete: true
Attachment #8478059 - Flags: review?(bugspam.Callek)
Attachment #8478815 - Flags: review?(bugspam.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")
Attachment #8478814 - Flags: review?(bugspam.Callek) → review+
Attachment #8478815 - Flags: review?(bugspam.Callek) → review+
(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 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
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)
Attachment #8478815 - Attachment is obsolete: true
Attachment #8491983 - Flags: review?(bugspam.Callek)
Blocks: 1072713
Comment on attachment 8491983 [details] [diff] [review]
Consolidated buildbotcustom patch to fix issues from checked-in patch (v3)

Moved to bug 1072713.
Attachment #8491983 - Flags: review?(bugspam.Callek)
Fixed in bug 1072713
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.