Closed Bug 673834 Opened 9 years ago Closed 9 years ago

Obsolete ReleaseRepackFactory, fold logic into CCReleaseRepackFactory

Categories

(Release Engineering :: General, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Callek, Assigned: Callek)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch [buildbotcustom] v1 (obsolete) — Splinter Review
ReleaseRepackFactory is currently broken, and is unused in current Mozilla Release Automation.

However comm-central apps (Like SeaMonkey) still use it, by extension.

It broke due to downloadBuilds expecting srcdir property when accessing the environ (for ZIP_IN) but we now use hg_tool.py when getting sources, which requires the environ.

This patch does two things, folds down the existing logic from ReleaseRepackFactory to CCReleaseRepackFactory.

Ignoring the references in /mozilla2/ per instruction from rail.
Attachment #548085 - Flags: review?(rail)
Pushed to |seamonkey-production| http://hg.mozilla.org/build/buildbotcustom/rev/7eafdcab63b5
Comment on attachment 548085 [details] [diff] [review]
[buildbotcustom] v1

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

Overall looks good. Just some small details:

::: process/factory.py
@@ +2767,4 @@
>               description='configure',
>               descriptionDone='configure done',
>               haltOnFailure=True,
> +             env = self.env,

This hunk doesn't apply

@@ +3422,5 @@
> +        self.mozRepoPath = mozRepoPath
> +        self.inspectorRepoPath = inspectorRepoPath
> +        self.venkmanRepoPath = venkmanRepoPath
> +        self.chatzillaRepoPath = chatzillaRepoPath
> +        self.cvsroot = cvsroot

The same here...

@@ +3477,5 @@
> +         workdir='build/'+self.mozillaSrcDir,
> +         description=['update mozilla',
> +                      'to %s' % self.buildRevision],
> +         haltOnFailure=True
> +        )

Can you avoid using addStep(ShellCommand, ...) and use addStep(ShellCommand(...)) instead in this patch?

@@ +3603,3 @@
>           workdir='build/'+self.objdir+'/'+self.appName+'/locales'
>          )
>  

Could you also switch to using release.paths.makeCandidatesDir from tools/lib/python here?
Attachment #548085 - Flags: review?(rail) → review-
Attached patch [buildbotcustom] v2 (obsolete) — Splinter Review
So, this does a *small* bit more than this bug is about, but other than a bit of timeout stuff in CC* things, this also does doUpload(..) method signature updates.

All of this is live on seamonkey-production atm, I manually unwrapped this patch to apply to |default| for ease of reviewing.
Attachment #548085 - Attachment is obsolete: true
Attachment #591333 - Flags: review?(rail)
Comment on attachment 591333 [details] [diff] [review]
[buildbotcustom] v2

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

BTW, pyflakes throws an error:

process/factory.py:6121: undefined name 'crashtest_leak_threshold'

Could you fix this as well?

In overall the patch looks good except some nits.

::: process/factory.py
@@ +3844,5 @@
> +
> +    # Repeated here since the Parent classes fail hgtool/checkouts due to
> +    # relbranch issues, and not actually having the tag after clone
> +    def getSources(self):
> +        self.addStep(MercurialCloneCommand,

self.addStep(MercurialCloneCommand()) please :)
it fails earlier (checkconfig time) than self.addStep(MercurialCloneCommand, ...)

@@ +3848,5 @@
> +        self.addStep(MercurialCloneCommand,
> +         name='get_enUS_src',
> +         command=['sh', '-c',
> +          WithProperties('if [ -d '+self.origSrcDir+'/.hg ]; then ' +
> +                         'hg -R '+self.origSrcDir+' pull ;'+

A nit. && instead of ; would be safer here. sh may behave differently, especially if dash is used.

@@ +3861,5 @@
> +         workdir=self.baseWorkDir,
> +         haltOnFailure=True,
> +         timeout=30*60 # 30 minutes
> +        )
> +        self.addStep(MercurialCloneCommand,

self.addStep(MercurialCloneCommand()) please.

@@ +3903,5 @@
> +            co_command.append('--inspector-rev=%s' % self.buildRevision)
> +            co_command.append('--venkman-rev=%s' % self.buildRevision)
> +            co_command.append('--chatzilla-rev=%s' % self.buildRevision)
> +        # execute the checkout
> +        self.addStep(ShellCommand,

self.addStep(ShellCommand()) please

@@ +3934,5 @@
>                       property='l10n_revision',
>                       workdir=WithProperties('build/' + self.l10nRepoPath + 
>                                              '/%(locale)s')
>          ))
> +        self.addStep(ShellCommand,

the same here

@@ +3942,5 @@
> +                      'to %s' % self.buildRevision],
> +         haltOnFailure=True
> +        )
> +        if self.venkmanRepoPath:
> +            self.addStep(ShellCommand,

the same here

@@ +3950,5 @@
> +                          'to %s' % self.buildRevision],
> +             haltOnFailure=True
> +            )
> +        if self.inspectorRepoPath:
> +            self.addStep(ShellCommand,

the same here

@@ +3958,5 @@
> +                          'to %s' % self.buildRevision],
> +             haltOnFailure=True
> +            )
> +        if self.chatzillaRepoPath:
> +            self.addStep(ShellCommand,

the same here
Attachment #591333 - Flags: review?(rail) → review-
address all nits
Attachment #591333 - Attachment is obsolete: true
Attachment #591656 - Flags: review?(rail)
Comment on attachment 591656 [details] [diff] [review]
[buildbotcustom] v2.1

Ship it! :)
Attachment #591656 - Flags: review?(rail) → review+
Comment on attachment 591656 [details] [diff] [review]
[buildbotcustom] v2.1

http://hg.mozilla.org/build/buildbotcustom/rev/29b8548e5348 (default)

I will merge to seamonkey-production at next merge of my own (after 2.7 final ships)
Attachment #591656 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.