Last Comment Bug 1171218 - Build symbols not working correctly in Thunderbird 38
: Build symbols not working correctly in Thunderbird 38
Status: VERIFIED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Build Config (show other bugs)
: 38 Branch
: Unspecified Unspecified
-- normal (vote)
: Thunderbird 43.0
Assigned To: Kent James (:rkent)
:
:
Mentors:
: 1197490 (view as bug list)
Depends on:
Blocks: 1194989
  Show dependency treegraph
 
Reported: 2015-06-03 15:23 PDT by Kent James (:rkent)
Modified: 2015-12-25 12:52 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed
fixed


Attachments
Add win-common (3.49 KB, patch)
2015-06-03 16:14 PDT, Kent James (:rkent)
mkmelin+mozilla: review+
philipp: approval‑comm‑aurora+
philipp: approval‑comm‑beta+
philipp: approval‑comm‑esr38+
Details | Diff | Splinter Review

Description User image Kent James (:rkent) 2015-06-03 15:23:40 PDT
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 User image Kent James (:rkent) 2015-06-03 16:14:21 PDT
Created attachment 8614952 [details] [diff] [review]
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 User image Kent James (:rkent) 2015-06-03 16:21:24 PDT
I started https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=409f7d070847 with these changes.
Comment 3 User image Ted Mielczarek [:ted.mielczarek] 2015-06-03 18:38:37 PDT
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 User image Ted Mielczarek [:ted.mielczarek] 2015-06-04 05:44:56 PDT
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 User image Magnus Melin 2015-06-04 11:25:01 PDT
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 User image Magnus Melin 2015-06-04 11:25:59 PDT
Oh, you already wrote that, sorry.
Comment 7 User image Nick Thomas [:nthomas] 2015-06-04 14:41:06 PDT
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 User image Nick Thomas [:nthomas] 2015-06-04 14:42:30 PDT
Are you adding that because the comparison tool demands it ?
Comment 9 User image Kent James (:rkent) 2015-06-04 14:59:16 PDT
(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 User image Nick Thomas [:nthomas] 2015-06-04 15:12:58 PDT
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 User image Wayne Mery (:wsmwk, NI for questions) 2015-08-15 06:32:43 PDT
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 User image Nick Thomas [:nthomas] 2015-08-16 16:59:06 PDT
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 User image Magnus Melin 2015-08-16 22:46:37 PDT
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 14 User image Nick Thomas [:nthomas] 2015-08-24 03:17:28 PDT
*** Bug 1197490 has been marked as a duplicate of this bug. ***
Comment 15 User image Nick Thomas [:nthomas] 2015-08-24 03:19:08 PDT
Go ahead and land this patch if you want comm-central and comm-aurora nightlies back.
Comment 16 User image aleth [:aleth] 2015-08-24 05:56:55 PDT
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 User image aleth [:aleth] 2015-08-24 06:13:33 PDT
Needs uplift to all kinds of places?
Comment 18 User image Kent James (:rkent) 2015-08-24 10:06:08 PDT
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 User image Nick Thomas [:nthomas] 2015-09-04 02:34:02 PDT
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 User image Philipp Kewisch [:Fallen] 2015-09-04 02:53:19 PDT
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 User image Magnus Melin 2015-09-04 03:26:55 PDT
(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 23 User image Stefan Sitter 2015-09-05 04:48:17 PDT
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 User image Philipp Kewisch [:Fallen] 2015-09-05 05:20:41 PDT
Fixed with https://hg.mozilla.org/releases/comm-esr38/rev/649d1865515c
Comment 25 User image Joe Sabash [:JoeS1] 2015-12-25 07:18:15 PST
Looks like a typo from a previous checkin to me https://hg.mozilla.org/releases/comm-esr38/rev/649d1865515c#l1.15
Comment 26 User image Joe Sabash [:JoeS1] 2015-12-25 08:21:54 PST
I'm not a programmer, but shouldn't that be if and not fi
Comment 27 User image Magnus Melin 2015-12-25 12:52:26 PST
No. "fi" closes the "if" clause (and the line you mention wasn't changed)

Note You need to log in before you can comment on or make changes to this bug.