For some builds GetPlatformBuildID() is returning a wrong build id

RESOLVED FIXED in Firefox 61

Status

()

RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: whimboo, Assigned: chmanchester)

Tracking

(Blocks: 2 bugs, {regression})

unspecified
mozilla61
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 wontfix, firefox51- wontfix, firefox52 wontfix, firefox-esr52 wontfix, firefox59 wontfix, firefox60 wontfix, firefox61 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
[Tracking Requested - why for this release]:
This seems to be a regression - not sure how long already. We noticed that behavior now by re-enabling one of our firefox ui unit tests. As result we get test failures like bug 1298233.

For testing I downloaded a Linux64 opt build:

https://queue.taskcluster.net/v1/task/UQoBONAuT-uHXGqytBhX_Q/runs/0/artifacts/public%2Fbuild%2Ftarget.tar.bz2

Here the script which I have executed in Scratchpad:

> Components.utils.import("resource://gre/modules/Services.jsm")
> alert(Services.appinfo['appBuildID'] + ' ' + Services.appinfo['platformBuildID'])

The build ids I get displayed are: "20160825223519 20160825215758"

application.ini:

> [App]
> BuildID=20160825223519

platform.ini:

> [Build]
> BuildID=20160825223519

So not sure where GetPlatformBuildID() is getting 20160825215758 from.

Version 51.0 is definitely affected, maybe also 50.0 which is on the aurora channel right now.
(Reporter)

Comment 1

2 years ago
I enabled test_appinfo.py on mozilla-aurora via bug 1298332. So we should get some more feedback for which builds this is happening.
(Reporter)

Comment 2

2 years ago
Nothing in the build log for the following task references 20160825215758 at all.

https://tools.taskcluster.net/task-inspector/#UQoBONAuT-uHXGqytBhX_Q/0
(Reporter)

Comment 3

2 years ago
Reported test failures via bug 1298233 have shown that also mozilla-aurora is affected.

Benjamin, do you have an idea what this could be or who actually could know more about it? Given that the build logs do not show something suspicious, it's kinda strange how this wrong buildid gets built.
status-firefox50: ? → affected
Flags: needinfo?(benjamin)
(Reporter)

Comment 4

2 years ago
It looks like that I can also replicate that with a local self-made artifact build from mozilla-central:

> AssertionError: u'20160824142336' != '20160831131135'

$ cat obj/debug/dist/bin/platform.ini 
> [Build]
> BuildID=20160831131135

Maybe one change we did to the build process can cause this misbehavior?
Flags: needinfo?(cmanchester)
(Assignee)

Comment 5

2 years ago
I think an incremental build could result in a different build id seen by Services.appinfo and application.ini if the incremental build did not end up compiling nsAppRunner.cpp, due to this: https://dxr.mozilla.org/mozilla-central/rev/506facea63169a29e04eb140663da1730052db64/toolkit/xre/Makefile.in#15

I do not know what the implications of that are.
Flags: needinfo?(cmanchester)
(Reporter)

Comment 6

2 years ago
Chris, this is not for application.ini but platform.ini.

Comment 7

2 years ago
I have nothing to add.
Flags: needinfo?(benjamin)
platform.ini is not read when doing GetPlatformBuildID(), thanks to bug 543150. And cf. Chris's comment, nsAppRunner.cpp is not rebuilt on incremental builds.
(Reporter)

Comment 9

2 years ago
So it means we are ok to live with this mismatch, and we should not make use of the platform buildid? If that is the case why don't we remove the platform.ini file? Keeping inconstent data around doesn't seem to be the best idea.
The platform.ini file still exists for the same reason the application.ini still exists: because some automation scripts rely on them being there.

And no, I'm not happy with this mismatch. OTOH, it would suck to have to rebuild libxul on every build. A possibility would be to move (well, copy) platform.ini into omni.ja and read it from there in nsAppRunner.cpp. The point of the static build id in nsAppRunner.cpp is to avoid reading one more file on startup. We read omni.ja in any case, so, it seems like a good fit. I haven't looked whether we need the platform id before omni.ja is initialized, though.
(Reporter)

Comment 11

2 years ago
I see. So I will temporary skip the failure test on bug 1298233 until we found a solution for that. Thanks.
Discussed during Platform triage - don't think we need to track this as a release blocker.
tracking-firefox51: ? → -
As this does not impact the release bits, marking it as fix-optional for 50.
status-firefox51: affected → fix-optional
Also updating status for Fx50 as "fix-optional".
status-firefox50: affected → fix-optional

Updated

2 years ago
Blocks: 1309177

Updated

2 years ago
No longer blocks: 1309177
status-firefox50: fix-optional → wontfix
status-firefox51: fix-optional → wontfix
status-firefox52: --- → fix-optional
status-firefox53: --- → fix-optional

Comment 15

10 months ago
We'll likely need something like #c10 so that a tup build backend can perform incremental builds without re-linking libxul on every invocation, so marking this as a blocker for that work.
Blocks: 827343
(Assignee)

Comment 16

6 months ago
After looking into various solutions and asking around about this, it looks like we can:

For automation builds, force buildid.h to be regenerated, and always #include buildid.h in nsAppRunner.cpp. This will eliminate the mismatch at the cost of re-linking libxul on every build. Given we're only infrequently (if ever) doing incremental builds in automation I think this is acceptable overhead and an improvement on the current situation.

For developer builds, always #include buildid.h in nsAppRunner.cpp, but do not force buildid.h to be re-generated for incremental builds. This means we will never have the mismatch, but it deviates from our current behavior because subsequent builds are no longer guaranteed to have a new buildid. This is already an imperfect guarantee due to this bug, and with bug 1368699 in place I can't find a specific case a developer build would suffer from not having a new buildid with every incremental build.
(Assignee)

Comment 17

6 months ago
The patch to implement comment 16 is actually quite small.
Assignee: nobody → cmanchester
Comment hidden (mozreview-request)

Comment 19

5 months ago
mozreview-review
Comment on attachment 8956995 [details]
Bug 1298328 - Fix dependency between buildid.h and libxul, do not re-generate buildid.h for every developer build.

https://reviewboard.mozilla.org/r/225960/#review232124

This is a surprisingly small patch! Yay for removing a Makefile.in!

::: Makefile.in:37
(Diff revision 1)
>  
>  ifndef MOZ_PROFILE_USE
> +# Automation builds should always have a new buildid, but for the sake of not
> +# re-linking libxul on every incremental build we do not enforce this for
> +# developer builds.
> +ifneq (,$(MOZ_AUTOMATION)$(MOZ_BUILD_DATE))

Are we punting on the automation case for tup right now or does tup have its own special-case for this?

Ideally we'd have a way to express this in moz.build at some point.
Attachment #8956995 - Flags: review?(ted) → review+
(Assignee)

Comment 20

5 months ago
mozreview-review-reply
Comment on attachment 8956995 [details]
Bug 1298328 - Fix dependency between buildid.h and libxul, do not re-generate buildid.h for every developer build.

https://reviewboard.mozilla.org/r/225960/#review232124

> Are we punting on the automation case for tup right now or does tup have its own special-case for this?
> 
> Ideally we'd have a way to express this in moz.build at some point.

Yeah, we'll need a small follow up to do the same for Tup, but that can be done independently.

Comment 21

5 months ago
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d981ccf4c2e
Fix dependency between buildid.h and libxul, do not re-generate buildid.h for every developer build. r=ted

Comment 22

5 months ago
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/09a3293b4d6f
Backed out changeset 2d981ccf4c2e for failing X tests on /experiments/test/xpcshell/test_conditions.js on a CLOSED TREE
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2d981ccf4c2e1f905698d0dcbecb0b52e0e49911&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Most likely to hit Linux x64 asan, OS X and Windows pgo & debug.

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=166865409&repo=autoland

[task 2018-03-08T23:32:03.876Z] 23:32:03     INFO -  TEST-START | browser/experiments/test/xpcshell/test_conditions.js
[task 2018-03-08T23:32:05.308Z] 23:32:05  WARNING -  TEST-UNEXPECTED-FAIL | browser/experiments/test/xpcshell/test_conditions.js | xpcshell return code: 0
[task 2018-03-08T23:32:05.311Z] 23:32:05     INFO -  TEST-INFO took 1436ms
[task 2018-03-08T23:32:05.312Z] 23:32:05     INFO -  >>>>>>>
[task 2018-03-08T23:32:05.313Z] 23:32:05     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2018-03-08T23:32:05.314Z] 23:32:05     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2018-03-08T23:32:05.314Z] 23:32:05     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2018-03-08T23:32:05.315Z] 23:32:05     INFO -  running event loop
[task 2018-03-08T23:32:05.317Z] 23:32:05     INFO -  browser/experiments/test/xpcshell/test_conditions.js | Starting test_setup
[task 2018-03-08T23:32:05.318Z] 23:32:05     INFO -  (xpcshell/head.js) | test test_setup pending (2)
[task 2018-03-08T23:32:05.319Z] 23:32:05     INFO -  PID 9362 | 1520551924504	addons.xpi	WARN	Failed to add built-in install location app-system-defaults: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FileUtils.jsm :: FileUtils_getDir :: line 60"  data: no] Stack trace: FileUtils_getDir()@resource://gre/modules/FileUtils.jsm:60
[task 2018-03-08T23:32:05.320Z] 23:32:05     INFO -  PID 9362 | addBuiltInInstallLocation()@resource://gre/modules/addons/XPIProvider.jsm:2047
[task 2018-03-08T23:32:05.321Z] 23:32:05     INFO -  PID 9362 | startup()@resource://gre/modules/addons/XPIProvider.jsm:2124
[task 2018-03-08T23:32:05.322Z] 23:32:05     INFO -  PID 9362 | callProvider()@resource://gre/modules/AddonManager.jsm:253
[task 2018-03-08T23:32:05.323Z] 23:32:05     INFO -  PID 9362 | _startProvider()@resource://gre/modules/AddonManager.jsm:728
[task 2018-03-08T23:32:05.323Z] 23:32:05     INFO -  PID 9362 | startup()@resource://gre/modules/AddonManager.jsm:892
[task 2018-03-08T23:32:05.324Z] 23:32:05     INFO -  PID 9362 | startup()@resource://gre/modules/AddonManager.jsm:2976
[task 2018-03-08T23:32:05.324Z] 23:32:05     INFO -  PID 9362 | observe()@jar:file:///builds/worker/workspace/build/application/firefox/omni.ja!/components/addonManager.js:63
[task 2018-03-08T23:32:05.325Z] 23:32:05     INFO -  PID 9362 | startAddonManagerOnly()@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/head.js:154
[task 2018-03-08T23:32:05.326Z] 23:32:05     INFO -  PID 9362 | test_setup()@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/test_conditions.js:44
[task 2018-03-08T23:32:05.326Z] 23:32:05     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2018-03-08T23:32:05.328Z] 23:32:05     INFO -  "CONSOLE_MESSAGE: (info) 1520551924504	addons.xpi	WARN	Failed to add built-in install location app-system-defaults: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FileUtils.jsm :: FileUtils_getDir :: line 60"  data: no] Stack trace: FileUtils_getDir()@resource://gre/modules/FileUtils.jsm:60
[task 2018-03-08T23:32:05.328Z] 23:32:05     INFO -  addBuiltInInstallLocation()@resource://gre/modules/addons/XPIProvider.jsm:2047
[task 2018-03-08T23:32:05.329Z] 23:32:05     INFO -  startup()@resource://gre/modules/addons/XPIProvider.jsm:2124
[task 2018-03-08T23:32:05.329Z] 23:32:05     INFO -  callProvider()@resource://gre/modules/AddonManager.jsm:253
[task 2018-03-08T23:32:05.330Z] 23:32:05     INFO -  _startProvider()@resource://gre/modules/AddonManager.jsm:728
[task 2018-03-08T23:32:05.330Z] 23:32:05     INFO -  startup()@resource://gre/modules/AddonManager.jsm:892
[task 2018-03-08T23:32:05.331Z] 23:32:05     INFO -  startup()@resource://gre/modules/AddonManager.jsm:2976
[task 2018-03-08T23:32:05.332Z] 23:32:05     INFO -  observe()@jar:file:///builds/worker/workspace/build/application/firefox/omni.ja!/components/addonManager.js:63
[task 2018-03-08T23:32:05.333Z] 23:32:05     INFO -  startAddonManagerOnly()@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/head.js:154
[task 2018-03-08T23:32:05.334Z] 23:32:05     INFO -  test_setup()@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/test_conditions.js:44"
[task 2018-03-08T23:32:05.335Z] 23:32:05     INFO -  PID 9362 | JavaScript strict warning: resource://gre/modules/TelemetryEnvironment.jsm, line 1461: ReferenceError: reference to undefined property "@mozilla.org/sandbox/sandbox-settings;1"
[task 2018-03-08T23:32:05.336Z] 23:32:05     INFO -  "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "@mozilla.org/sandbox/sandbox-settings;1"" {file: "resource://gre/modules/TelemetryEnvironment.jsm" line: 1461}]"
[task 2018-03-08T23:32:05.337Z] 23:32:05     INFO -  (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2018-03-08T23:32:05.338Z] 23:32:05     INFO -  (xpcshell/head.js) | test test_setup finished (2)
[task 2018-03-08T23:32:05.339Z] 23:32:05     INFO -  browser/experiments/test/xpcshell/test_conditions.js | Starting test_simpleFields
[task 2018-03-08T23:32:05.340Z] 23:32:05     INFO -  (xpcshell/head.js) | test test_simpleFields pending (2)
[task 2018-03-08T23:32:05.342Z] 23:32:05     INFO -  (xpcshell/head.js) | test run_next_test 1 finished (2)
[task 2018-03-08T23:32:05.343Z] 23:32:05     INFO -  Unexpected exception Error: Unsupported build ID: MOZ_BUILDID at /builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/test_conditions.js:88
[task 2018-03-08T23:32:05.344Z] 23:32:05     INFO -  addDate@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/test_conditions.js:88:11
[task 2018-03-08T23:32:05.345Z] 23:32:05     INFO -  prevDate@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/test_conditions.js:105:10
[task 2018-03-08T23:32:05.346Z] 23:32:05     INFO -  test_simpleFields@/builds/worker/workspace/build/tests/xpcshell/tests/browser/experiments/test/xpcshell/test_conditions.js:156:42
[task 2018-03-08T23:32:05.349Z] 23:32:05     INFO -  async*asyncFunction@resource://gre/modules/Task.jsm:236:18
[task 2018-03-08T23:32:05.350Z] 23:32:05     INFO -  Task_spawn@resource://gre/modules/Task.jsm:161:12
[task 2018-03-08T23:32:05.350Z] 23:32:05     INFO -  _run_next_test@/builds/worker/workspace/build/tests/xpcshell/head.js:1428:9
[task 2018-03-08T23:32:05.351Z] 23:32:05     INFO -  run@/builds/worker/workspace/build/tests/xpcshell/head.js:685:9
[task 2018-03-08T23:32:05.352Z] 23:32:05     INFO -  _do_main@/builds/worker/workspace/build/tests/xpcshell/head.js:221:3
[task 2018-03-08T23:32:05.353Z] 23:32:05     INFO -  _execute_test@/builds/worker/workspace/build/tests/xpcshell/head.js:533:5
[task 2018-03-08T23:32:05.353Z] 23:32:05     INFO -  @-e:1:1
[task 2018-03-08T23:32:05.354Z] 23:32:05     INFO -  exiting test
[task 2018-03-08T23:32:05.355Z] 23:32:05     INFO -  PID 9362 | JavaScript error: resource://gre/modules/osfile/osfile_async_front.jsm, line 408: Error: OS.File has been shut down. Rejecting post to DirectoryIterator_prototype_close
[task 2018-03-08T23:32:05.356Z] 23:32:05     INFO -  <<<<<<<
Flags: needinfo?(cmanchester)
(Assignee)

Comment 24

5 months ago
I see, this patch sets the buildid to "MOZ_BUILDID". The #include for buildid.h appears after the assignment to gToolkitBuilID, so the result of NS_STRINGIFY(MOZ_BUILDID) is "MOZ_BUILDID".

Updated patch coming up.
Flags: needinfo?(cmanchester)
Comment hidden (mozreview-request)
(Assignee)

Updated

5 months ago
Attachment #8956995 - Flags: review+ → review?(ted)

Comment 26

5 months ago
mozreview-review
Comment on attachment 8956995 [details]
Bug 1298328 - Fix dependency between buildid.h and libxul, do not re-generate buildid.h for every developer build.

https://reviewboard.mozilla.org/r/225960/#review232786

Well that's just a terrible failure mode. :) Hooray for C++!
Attachment #8956995 - Flags: review?(ted) → review+

Comment 27

5 months ago
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48b8e6530f1b
Fix dependency between buildid.h and libxul, do not re-generate buildid.h for every developer build. r=ted

Comment 28

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/48b8e6530f1b
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(Assignee)

Updated

5 months ago
Blocks: 1445398
status-firefox52: fix-optional → wontfix
status-firefox53: fix-optional → ---
status-firefox59: --- → wontfix
status-firefox60: --- → wontfix
status-firefox-esr52: --- → wontfix
Keywords: regressionwindow-wanted
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1297373
You need to log in before you can comment on or make changes to this bug.