Closed Bug 1298328 Opened 8 years ago Closed 6 years ago

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

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox50 --- wontfix
firefox51 - wontfix
firefox52 --- wontfix
firefox-esr52 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: whimboo, Assigned: chmanchester)

References

Details

(Keywords: regression)

Attachments

(1 file)

[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.
I enabled test_appinfo.py on mozilla-aurora via bug 1298332. So we should get some more feedback for which builds this is happening.
Nothing in the build log for the following task references 20160825215758 at all.

https://tools.taskcluster.net/task-inspector/#UQoBONAuT-uHXGqytBhX_Q/0
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.
Flags: needinfo?(benjamin)
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)
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)
Chris, this is not for application.ini but platform.ini.
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.
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.
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.
As this does not impact the release bits, marking it as fix-optional for 50.
Blocks: 1309177
No longer blocks: 1309177
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: buildtup
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.
The patch to implement comment 16 is actually quite small.
Assignee: nobody → cmanchester
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+
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.
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
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)
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)
Attachment #8956995 - Flags: review+ → review?(ted)
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+
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
https://hg.mozilla.org/mozilla-central/rev/48b8e6530f1b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.