1059511 - Start to drop "mozilla" objdir from Thunderbird builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Mark Banner (:standard8)]

Attachment: 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.

Comment 1

[Justin Wood (:Callek)]

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.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8480158 [details] [diff] [review]
The fix

Ok, lets redirect it.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 4

[Mark Banner (:standard8)]

(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 ^^^

Comment 5

[Rail Aliiev [:rail]]

The comments are OK, but it'd be safer to file tracking bugs for those releases.

Comment 6

[Mark Banner (:standard8)]

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

Comment 7

[Jordan Lund (:jlund)]

patch(es) in production :)

Comment 8

[Joshua Cranmer [:jcranmer]]

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))

Comment 9

[Joshua Cranmer [:jcranmer]]

Temporary workaround I used in a try build: copy config/configobj.py and config/printconfigsetting.py from mozilla-central.

Comment 10

[Mark Banner (:standard8)]

(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.

Comment 11

[Mark Banner (:standard8)]

(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

Comment 12

[Mark Banner (:standard8)]

Attachment: 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.

Comment 13

[Mark Banner (:standard8)]

Attachment: 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.

Comment 14

[Mark Banner (:standard8)]

Attachment: 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!

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 18

[Mark Banner (:standard8)]

Attachment: tools - use mozilla_srcdir v2

Updated patch to fix the comments.

Comment 19

[Mark Banner (:standard8)]

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

Comment 20

[Mark Banner (:standard8)]

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

https://hg.mozilla.org/build/buildbotcustom/rev/0e1829955f1a

Comment 21

[Nick Thomas [:nthomas] (UTC+12)]

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

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

What could possibly go wrong :-)

Comment 22

[Mark Banner (:standard8)]

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

https://hg.mozilla.org/build/tools/rev/c8c5e2753e1d

Comment 23

[Mark Banner (:standard8)]

Attachment: 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.

Comment 24

[Chris Cooper [:coop] (he/him)]

Merged to production, and deployed.

Comment 25

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 26

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 27

[Mark Banner (:standard8)]

(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.

Comment 28

[Mark Banner (:standard8)]

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

https://hg.mozilla.org/build/buildbotcustom/rev/738be1ec45f9

Comment 29

[Chris Cooper [:coop] (he/him)]

Merged to production, and deployed.

Comment 30

[Mark Banner (:standard8)]

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

Comment 31

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: [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.

Comment 32

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8486078 [details] [diff] [review]
[tools] fix repacks

https://hg.mozilla.org/build/tools/rev/9592627144c5 + moved 3.1.1 tags

Comment 33

[Mark Banner (:standard8)]

jlund: Is there anything we need to change in mozharness to account for the changes of objdir for Thunderbird in this bug?

Comment 34

[Jordan Lund (: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.

Comment 35

[Massimo Gervasini [:massimo]]

Hi Jordan,

You're right, there are no mozharness repacks for Thunderbird.

Comment 36

[Mark Banner (:standard8)]

Bug 1085112 is the merge day / release update follow-up.