1171218 - Build symbols not working correctly in Thunderbird 38

Status: VERIFIED FIXED

(Thunderbird :: Build Config, defect)

Description

[Kent James (:rkent)]

See bug 1085557.

Now I am getting a failure in comm-esr38 builds due to differences in build files associated with this line:

export SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE=/builds/crash-stats-api.token

Looking back at the history of this, these lines have been slowly added in comm-central as ad-hoc reactions to build failures. Also looking at crash stats, something is clearly wrong with symbol uploads, we don't get links to lines in comm-central from at least the Windows crashes.

Comment 1

[Kent James (:rkent)]

Attachment: Add win-common

I believe this is what we need for Windows, in comm-central and other repos.

For comm-esr38 we also need to port some of the Linux and OSX patches as well.

Comment 2

[Kent James (:rkent)]

I started https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=409f7d070847 with these changes.

Comment 3

[(not currently active) Ted Mielczarek]

You don't actually want the symbol upload token defined in your mozconfig unless you have a valid token in that file on the builder. Otherwise your symbol uploads are not going to work.

Comment 4

[(not currently active) Ted Mielczarek]

I was under the impression that Thunderbird had its own separate builders, but I'm told Thunderbird builds use the same build pool as Firefox, so putting the same path to the auth token in mozconfigs should be fine.

If you have Linux Thunderbird builds using mozharness+mock you'll need to make sure the mock config copies the token into the mock environment, but other than that it should work fine.

Comment 5

[Magnus Melin [:mkmelin]]

FTR: according to bug 1170253 comment 8, the firefox token file already present should work as we're using the same builder pool.

Comment 6

[Magnus Melin [:mkmelin]]

Oh, you already wrote that, sorry.

Comment 7

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

Comment on attachment 8614952 [details] [diff] [review]
Add win-common

>diff --git a/build/mozconfig.win-common b/build/mozconfig.win-common
>new file mode 100644
>--- /dev/null
>+++ b/build/mozconfig.win-common
>@@ -0,0 +1,16 @@
>+# This Source Code Form is subject to the terms of the Mozilla Public
>+# License, v. 2.0. If a copy of the MPL was not distributed with this
>+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
>+
>+if [ "x$IS_NIGHTLY" = "xyes" ]; then
>+  # Some nightlies (eg: Mulet) don't want these set.
>+  MOZ_AUTOMATION_UPLOAD_SYMBOLS=${MOZ_AUTOMATION_UPLOAD_SYMBOLS-1}
>+  MOZ_AUTOMATION_UPDATE_PACKAGING=${MOZ_AUTOMATION_UPDATE_PACKAGING-1}
>+  MOZ_AUTOMATION_SDK=${MOZ_AUTOMATION_SDK-1}
>+fi
>+
>+# Some builds (eg: Mulet) don't want the installer, so only set this if it
>+# hasn't already been set.
>+MOZ_AUTOMATION_INSTALLER=${MOZ_AUTOMATION_INSTALLER-1}
>+

Everything before here is going to be ignored at best, or will cause bustage.

>+export SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE=c:/builds/crash-stats-api.token

You only need this line.

Comment 8

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

Are you adding that because the comparison tool demands it ?

Comment 9

[Kent James (:rkent)]

(In reply to Nick Thomas [:nthomas] from comment #8)
> Are you adding that because the comparison tool demands it ?

Yes. I'm not sure of the how or why of the comparison tool, but we'll get errors if we have the same file with different content.

Ignoring is OK, breakage is bad. Do you know which it is?

Comment 10

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

It's probably ignore. Those environment variables are used when the automation calls 'mach build' in mozharness, and AFAIK Thunderbird isn't doing that. For reference some to build system for this is
 https://dxr.mozilla.org/mozilla-central/source/build/mozconfig.automation
 https://dxr.mozilla.org/mozilla-central/source/build/moz-automation.mk

Comment 11

[Wayne Mery (:wsmwk)]

crash-stats is missing symbols for Thunderbird 38.2.0 crashes. Perhaps because of this bug?? 
I've filed bug 1194989 just in case it's not this bug

https://crash-stats.mozilla.com/search/?product=Thunderbird&version=38.2.0&_facets=signature&_columns=date&_columns=signature&_columns=platform&_columns=platform_version&_columns=user_comments&_columns=email#crash-reports

Comment 12

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

Yes. Without the export of SOCORRO_SYMBOL_UPLOAD_TOKEN_FILE the symbols are uploaded to the old location, and presumably the sync from old to new is disabled now. I've fixed up 38.2.0, see bug 1194989 comment #3.

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 8614952 [details] [diff] [review]
Add win-common

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

Stealing this - looks ok to me so r=mkmelin

Comment 15

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

Go ahead and land this patch if you want comm-central and comm-aurora nightlies back.

Comment 16

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/95e56121a3ffb301cfb65b55992a9e9db7bb43e3
Bug 1171218 - Build symbols not working correctly in Thunderbird 38. r=mkmelin a=bustage fix on CLOSED TREE

Comment 17

[aleth [:aleth]]

Needs uplift to all kinds of places?

Comment 18

[Kent James (:rkent)]

Comment on attachment 8614952 [details] [diff] [review]
Add win-common

Yes presumably this needs uplift. It would be good to see at least one working nightly before we do that.

Comment 19

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

There should be a green Window nightlies on comm-central today. We actually could have verified this earlier though, as the cause of the orange occurs after the symbol upload. It's been working:

Uploading symbol file "dist/thunderbird-43.0a1.en-US.win32.crashreporter-symbols-full.zip" to "https://crash-stats.mozilla.com/symbols/upload"
Attempt 1 of 5...
Uploaded successfully!

(from http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2015/09/2015-09-03-03-02-01-comm-central/comm-central-win32-nightly-bm84-build1-build18.txt.gz0

Would be very helfpul to uplift this, particularly for 41.0b1 build2 and 38.3.0.

Comment 20

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8614952 [details] [diff] [review]
Add win-common

I'm going to plus the approvals here, we should definitely use the new system asap.

Comment 21

[Magnus Melin [:mkmelin]]

(using checkin-needed for branch checkins is confusing, I think you just have to look at the flags for what approved but not fixed)

Comment 22

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

https://hg.mozilla.org/releases/comm-aurora/rev/be7afe3e6d1d
https://hg.mozilla.org/releases/comm-beta/rev/fef892adccdf
https://hg.mozilla.org/releases/comm-esr38/rev/78b0395f4064

Comment 23

[Stefan Sitter [:ssitter]]

This seem to have caused test bustage on comm-esr38:

> Linux opt Build (B) https://treeherder.mozilla.org/logviewer.html#?job_id=2080&repo=comm-esr38
> 
> TEST-UNEXPECTED-FAIL | check-sync-dirs.py | build file copies are not in sync
> TEST-INFO | check-sync-dirs.py | file(s) found in: /builds/slave/tb-c-esr38-lx-0000000000000000/build/build
> TEST-INFO | check-sync-dirs.py | differ from their originals in: /builds/slave/tb-c-esr38-lx-0000000000000000/build/mozilla/build
> TEST-INFO | check-sync-dirs.py | differing file: ./mozconfig.win-common

Comment 24

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Fixed with https://hg.mozilla.org/releases/comm-esr38/rev/649d1865515c

Comment 25

[Joe Sabash [:JoeS1]]

Looks like a typo from a previous checkin to me https://hg.mozilla.org/releases/comm-esr38/rev/649d1865515c#l1.15

Comment 26

[Joe Sabash [:JoeS1]]

I'm not a programmer, but shouldn't that be if and not fi

Comment 27

[Magnus Melin [:mkmelin]]

No. "fi" closes the "if" clause (and the line you mention wasn't changed)