The default bug view has changed. See this FAQ.

Start to drop "mozilla" objdir from Thunderbird builds

RESOLVED FIXED

Status

Release Engineering
General Automation
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8480158 [details] [diff] [review]
The fix

Bug 1040009 has finally made the Thunderbird builds put all their files in objdir, and not a two-level thing.

Hence our dist directory is now in <objdir>/dist rather than <objdir>/mozilla/dist

For starters, this is only on central, so here's a patch with some merge day annotations.

Unfortunately I'm unable to test the config locally, but if it runs through python and looks reasonable, then I'm happy for this to be pushed out to begin with, rather than do staging builds.
Attachment #8480158 - Flags: review?(bugspam.Callek)
(Assignee)

Updated

3 years ago
Depends on: 1040009

Updated

3 years ago
Assignee: nobody → standard8
Status: NEW → ASSIGNED
Mark,

I'm on PTO until tuesday, so if this is *needed* for central now, I'd rather have the review redirected, if its not absolutely needed for central I'll get to it when I'm back.
(Assignee)

Comment 2

3 years ago
Comment on attachment 8480158 [details] [diff] [review]
The fix

Ok, lets redirect it.
Attachment #8480158 - Flags: review?(bugspam.Callek) → review?(nthomas)
Comment on attachment 8480158 [details] [diff] [review]
The fix

>diff --git a/mozilla/staging_release-thunderbird-comm-beta.py b/mozilla/staging_release-thunderbird-comm-beta.py

Please modify the .template copy of this file too (we don't automatically update the non-template for staging so lets do both). And looks like you could remove the block for alder too
 http://mxr.mozilla.org/build/source/buildbot-configs/mozilla/thunderbird_project_branches.py#6
r+ to do both on landing.

Please check with rail about our merge process as I'm not sure if we grep everything for MERGE DAY. Having that in release configs is unusual.
Attachment #8480158 - Flags: review?(nthomas) → review+
(Assignee)

Comment 4

3 years ago
(In reply to Nick Thomas [:nthomas] from comment #3)
> Please check with rail about our merge process as I'm not sure if we grep
> everything for MERGE DAY. Having that in release configs is unusual.

rail ^^^
Flags: needinfo?(rail)
The comments are OK, but it'd be safer to file tracking bugs for those releases.
Flags: needinfo?(rail)
(Assignee)

Comment 6

3 years ago
Comment on attachment 8480158 [details] [diff] [review]
The fix

Landed for now, I'll file bugs before closing this.

https://hg.mozilla.org/build/buildbot-configs/rev/9fc603d86b87
Attachment #8480158 - Flags: checked-in+

Comment 7

3 years ago
patch(es) in production :)
Builds are now purple:
retry: Calling <function run_with_timeout at 0x7f5b75a02b90> with args: (['python', '/builds/slave/tb-c-cen-lx-000000000000000000/tools/buildfarm/utils/graph_server_post.py', '--server', 'graphs.mozilla.org', '--selector', '/server/collect.cgi', '--branch', 'Thunderbird', '--buildid', "python: can't open file 'build/config/printconfigsetting.py': [Errno 2] No such file or directory", '--sourcestamp', "python: can't open file 'build/config/printconfigsetting.py': [Errno 2] No such file or directory", '--resultsname', 'TB_Linux_comm-central', '--properties-file', 'properties.json', '--timestamp', '1409351759'], 120, None, None, False, True), kwargs: {}, attempt #2
Executing: ['python', '/builds/slave/tb-c-cen-lx-000000000000000000/tools/buildfarm/utils/graph_server_post.py', '--server', 'graphs.mozilla.org', '--selector', '/server/collect.cgi', '--branch', 'Thunderbird', '--buildid', "python: can't open file 'build/config/printconfigsetting.py': [Errno 2] No such file or directory", '--sourcestamp', "python: can't open file 'build/config/printconfigsetting.py': [Errno 2] No such file or directory", '--resultsname', 'TB_Linux_comm-central', '--properties-file', 'properties.json', '--timestamp', '1409351759']
results not added, response: 
'python: can't open file 'build/config/printconfigsetting.py': [Errno 2] No such file or directory' failed string validation
  File "/var/www/html/graphs/server/pyfomatic/collect.py", line 271, in handleRequest
    metadata = MetaDataFromTalos(databaseCursor, databaseModule, inputStream)
  File "/var/www/html/graphs/server/pyfomatic/collect.py", line 62, in __init__
    self.readFromSource(dataSource)
  File "/var/www/html/graphs/server/pyfomatic/collect.py", line 83, in readFromSource
    raise ImproperFormatException(str(x))
Temporary workaround I used in a try build: copy config/configobj.py and config/printconfigsetting.py from mozilla-central.
(Assignee)

Comment 10

3 years ago
(In reply to Joshua Cranmer [:jcranmer] from comment #9)
> Temporary workaround I used in a try build: copy config/configobj.py and
> config/printconfigsetting.py from mozilla-central.

Can we land that as a temporary work around? I see what the issue is, but I haven't got time to fix it before Tuesday, and it may need a couple of elrounds to get it all right. If the files are there then we should still be able to fix this separately and confirm on try/via log examination.
(Assignee)

Comment 11

3 years ago
(In reply to Mark Banner (:standard8) from comment #10)
> (In reply to Joshua Cranmer [:jcranmer] from comment #9)
> > Temporary workaround I used in a try build: copy config/configobj.py and
> > config/printconfigsetting.py from mozilla-central.
> 
> Can we land that as a temporary work around? I see what the issue is, but I
> haven't got time to fix it before Tuesday, and it may need a couple of
> elrounds to get it all right. If the files are there then we should still be
> able to fix this separately and confirm on try/via log examination.

I went ahead and landed it, so hopefully we can get the tree reasonably green.

https://hg.mozilla.org/comm-central/rev/c9811faef6cb
(Assignee)

Comment 12

3 years ago
Created attachment 8481908 [details] [diff] [review]
buildbot-configs - define mozilla_srcdir

Ok, the temporary bustage fix worked for dep builds, but not nightlies. The nightlies suffer from the same issue - dropping the "mozillaDir" affected looking up files in the src directory (for Thunderbird, some src directories got changed depending on the mozillaDir, which I was thinking was just for the objdir).

These patches add a 'mozilla_srcdir' configuration option, and spread it to the appropriate places throughout the buildbot code.
Attachment #8481908 - Flags: review?(nthomas)
(Assignee)

Comment 13

3 years ago
Created attachment 8481909 [details] [diff] [review]
buildbot-custom - use mozilla_srcdir

I think this should do the right thing. I'm a little bit uncertain of the release changes (or if more changes will be required), but maybe we can do a staging run during the next cycle.

I've not been able to test these patches btw.
Attachment #8481909 - Flags: review?(nthomas)
(Assignee)

Comment 14

3 years ago
Created attachment 8481910 [details] [diff] [review]
tools - use mozilla_srcdir

This updates the repack code, it has the added benefit of using the mozillaDir value from the release configs, and not trying to work it out from the application name!
Attachment #8481910 - Flags: review?(nthomas)
Comment on attachment 8481908 [details] [diff] [review]
buildbot-configs - define mozilla_srcdir

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

::: mozilla/thunderbird_config.py
@@ +734,4 @@
>  # This is a path, relative to HGURL, where the repository is located
>  # HGURL + repo_path should be a valid repository
>  BRANCHES['comm-central']['moz_repo_path'] = 'mozilla-central'
> +BRANCHES['comm-central']['mozilla_srcdir'] = 'mozilla'

Looks like you're setting this for all branches, so an alternative would be to put it in GLOBAL_VARS just once.
Attachment #8481908 - Flags: review?(nthomas) → review+
Comment on attachment 8481909 [details] [diff] [review]
buildbot-custom - use mozilla_srcdir

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

Reads like it would work for what you need, from 30000ft at least. Needs some fixes before landing.

::: misc.py
@@ +2356,4 @@
>                  project=pf['product_name'],
>                  appName=pf['app_name'],
>                  enUSBinaryURL=config['enUS_binaryURL'],
> +                mozillaSrcDir=config.get('mozilla_srcdir', None),

Why is this one a replacement instead of an additional variable ?

::: process/factory.py
@@ +3074,5 @@
>              self.mozillaDir = ''
> +            # Thunderbird doesn't have a mozilla in the objdir, but it
> +            # still does for the srcdir.
> +            if mozillaSrcDir:
> +                self.mozillaSrcDir = %s/%s % (self.origSrcDir, mozillaSrcDir)

Missing quoting around '%s/%s', needed to pass buildbot checkconfig.

@@ +4009,5 @@
> +            # Thunderbird now has a different srcdir to mozillaDir
> +            if mozillaSrcDir:
> +                self.mozillaSrcDir = self.origSrcDir
> +            else:
> +                self.mozillaSrcDir= '%s/%s' % (self.origSrcDir, mozillaSrcDir)

The assignment looks backwards here.
Attachment #8481909 - Flags: review?(nthomas) → review+
Comment on attachment 8481910 [details] [diff] [review]
tools - use mozilla_srcdir

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

::: lib/python/build/l10n.py
@@ +100,5 @@
>                   l10nIni, compareLocalesRepo, env, absObjdir, merge=True,
>                   productName=None, platform=None,
>                   version=None, partialUpdates=None,
> +                 buildNumber=None, stageServer=None,
> +                 mozillaDir=None, mozillaSrcDir = None):

Nit, remove padding around =.

::: scripts/l10n/create-release-repacks.py
@@ +308,5 @@
>          for i in range(0, len(makeDirs)):
>              makeDirs[i] = path.join(releaseConfig['mozilla_dir'], makeDirs[i])
> +        mozillaDir = releaseConfig['mozilla_dir']
> +        mozillaSrcDir = releaseConfig['mozilla_dir']
> +    else if 'mozilla_srcdir' in releaseConfig:

Syntax error, elif.

@@ +365,1 @@
>      )

I don't see these added to the definition for createRepacks(), so this will blow up all products. And you have them in the definition for repackLocale() but not the call.
Attachment #8481910 - Flags: review?(nthomas) → review-
(Assignee)

Comment 18

3 years ago
Created attachment 8482522 [details] [diff] [review]
tools - use mozilla_srcdir v2

Updated patch to fix the comments.
Attachment #8481910 - Attachment is obsolete: true
Attachment #8482522 - Flags: review?(nthomas)
(Assignee)

Comment 19

3 years ago
Comment on attachment 8481908 [details] [diff] [review]
buildbot-configs - define mozilla_srcdir

I'm landing this and the buildbotcustom patch to get them on the builders, and they don't need the tools patch yet, as that is self-contained:

https://hg.mozilla.org/build/buildbot-configs/rev/d4d9bc08f27c
Attachment #8481908 - Flags: checked-in+
(Assignee)

Comment 20

3 years ago
Comment on attachment 8481909 [details] [diff] [review]
buildbot-custom - use mozilla_srcdir

https://hg.mozilla.org/build/buildbotcustom/rev/0e1829955f1a
Attachment #8481909 - Flags: checked-in+
Comment on attachment 8482522 [details] [diff] [review]
tools - use mozilla_srcdir v2

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

What could possibly go wrong :-)
Attachment #8482522 - Flags: review?(nthomas) → review+
(Assignee)

Updated

3 years ago
Depends on: 1061789
(Assignee)

Comment 22

3 years ago
Comment on attachment 8482522 [details] [diff] [review]
tools - use mozilla_srcdir v2

https://hg.mozilla.org/build/tools/rev/c8c5e2753e1d
Attachment #8482522 - Flags: checked-in+
(Assignee)

Comment 23

3 years ago
Created attachment 8483041 [details] [diff] [review]
buildbotcustom - fix l10n linking of tools dir

I didn't spot this before, because I assumed it was needed for objdir/mozilla only. However, tests with latest l10n nightlies reveal that we need do need the tools dir symlinked.

Therefore, this ensures we get the link if either mozillaDir or mozillaSrcDir is set.
Attachment #8483041 - Flags: review?(nthomas)
(Assignee)

Updated

3 years ago
Attachment #8483041 - Attachment is patch: true
Merged to production, and deployed.
Comment on attachment 8483041 [details] [diff] [review]
buildbotcustom - fix l10n linking of tools dir

>             if mozillaSrcDir:
>+                self.linkTools

Missing assingment ' = True', by the looks.

>+        if self.linkTools:
>             self.addStep(MockCommand(**self.processCommand(
>                 name='link_tools',
>                 env=self.env,
>                 command=['sh', '-c', 'if [ ! -e tools ]; then ln -s ../tools; fi'],
>                 workdir='build',

Are you confident that this will work in both cases ? I'm failing to grok all the differences here.
Attachment #8483041 - Flags: review?(nthomas) → review+
It would be good to check with jlund and mgerva to see if any of these changes need to go into the mozharness equivalent of this code.
(Assignee)

Comment 27

3 years ago
(In reply to Nick Thomas [:nthomas] from comment #25)
> >+        if self.linkTools:
> >             self.addStep(MockCommand(**self.processCommand(
> >                 name='link_tools',
> >                 env=self.env,
> >                 command=['sh', '-c', 'if [ ! -e tools ]; then ln -s ../tools; fi'],
> >                 workdir='build',
> 
> Are you confident that this will work in both cases ? I'm failing to grok
> all the differences here.

This should now be enabled for: mozilladir being set (e.g. old Thunderbird builds) or mozilla_srcdir being set (new Thunderbird builds).

Builds that haven't got either of those set, won't add the link.
(Assignee)

Comment 28

3 years ago
Comment on attachment 8483041 [details] [diff] [review]
buildbotcustom - fix l10n linking of tools dir

https://hg.mozilla.org/build/buildbotcustom/rev/738be1ec45f9
Attachment #8483041 - Flags: checked-in+
Merged to production, and deployed.
(Assignee)

Comment 30

3 years ago
Comment on attachment 8483041 [details] [diff] [review]
buildbotcustom - fix l10n linking of tools dir

Unfortunately, I forgot to qrefresh, so the review correction didn't land. Surprisingly it didn't break anything either from what I can tell:

https://hg.mozilla.org/build/buildbotcustom/rev/5950c5c62a4c
Created attachment 8486078 [details] [diff] [review]
[tools] fix repacks

In 31.1.1 we're getting l10n repack failures when creating a complete mar. The utilities put mar at
 /builds/slave/tb-rel-c-esr31-osx64_rpk_1-000/comm-esr31/obj-l10n/mozilladist/host/bin/mar
then the script expects it at 
 /builds/slave/tb-rel-c-esr31-l64_rpk_1-00000/comm-esr31/obj-l10n/mozilla/dist/host/bin/mar

Would be the same problem for mbsdiff if we got to partials.
Attachment #8486078 - Flags: review?(rail)
Attachment #8486078 - Flags: review?(rail) → review+
Comment on attachment 8486078 [details] [diff] [review]
[tools] fix repacks

https://hg.mozilla.org/build/tools/rev/9592627144c5 + moved 3.1.1 tags
Attachment #8486078 - Flags: checked-in+
(Assignee)

Comment 33

3 years ago
jlund: Is there anything we need to change in mozharness to account for the changes of objdir for Thunderbird in this bug?
Flags: needinfo?(jlund)
(In reply to Mark Banner (:standard8) from comment #33)
> jlund: Is there anything we need to change in mozharness to account for the
> changes of objdir for Thunderbird in this bug?

thanks for checking!

skimming over patches, it looks like this is only touching repack jobs. Also, if this only affects Thunderbird, I think we are safe; AFAIK, Thunderbird builds/repacks have yet to be ported to mozharness.

to be safe, I'm flipping needinfo over to mgerva, our new repack expert, since the patches do touch factories shared by more than just thunderbird.
Flags: needinfo?(jlund) → needinfo?(mgervasini)
Hi Jordan,

You're right, there are no mozharness repacks for Thunderbird.
Flags: needinfo?(mgervasini)
(Assignee)

Updated

3 years ago
Blocks: 1085112
(Assignee)

Comment 36

3 years ago
Bug 1085112 is the merge day / release update follow-up.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.