Closed
Bug 1183077
Opened 9 years ago
Closed 9 years ago
include running arch in build target on Windows
Categories
(Toolkit :: Application Update, defect)
Toolkit
Application Update
Tracking
()
VERIFIED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: bhearsum, Assigned: spohl)
References
Details
Attachments
(2 files, 1 obsolete file)
5.91 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
1.11 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
We'll be shipping a 64-bit Windows build on the release channel starting with 40.0 in August. At some point we'll may be doing 32-bit -> 64-bit updates. In order to more easily facilitate this, it would be really helpful to include information about the running arch in the build target field of the update. We've got much more flexibility in how we configure updates on the server with Build Target (compared to OS Version, which already has this information). We already do something similar on OS X, where we include both the running arch and the packade archs of a build, so this isn't without precedent. Windows is a bit different because there's only one packaged arch, though. We started talking about this in e-mail, and here's a couple of ideas that were thrown around: > Something like: > WINNT_x86-msvc-u-i386 (32-bit build running on 32-bit hardware) > WINNT_x86_64-msvc-u-i386 (32-bit build running on 64-bit hardware) > WINNT_x86_64-msvc-u-x86_64 (64-bit build running on 64-bit hardware) And from Nick: > I'm not sure that I care about consistency with that scheme because the > problem space is a little different, so we could go with: > WINNT_x86-msvc-u-x86 (32-bit build running on 32-bit > hardware) > WINNT_x86_64-msvc-u-x64 (64-bit build running on 64-bit hardware) > WINNT_x86-msvc-u-x64 (32-bit build running on 64-bit > hardware) > using this bit of code, which populates the '(<arch>)' part of %OS_VERSION%: > > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#336 > > Alternatively we could just modify in the cross-arch case (pick one): > WINNT_x86-msvc-on-x86_64 (32-bit build running on 64-bit hardware) > WINNT_x86-msvc-x64 > [moar bikesheds here] > and leave these alone: > WINNT_x86-msvc (32-bit build running on 32-bit > hardware) > WINNT_x86_64-msvc (64-bit build running on 64-bit > hardware) > > How BUILD_TARGET is generated is a little complicated, if you try to go > beyond these > > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#3480 > > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#230 > The %OS_VERSION% part of the query gets expanded using > > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#252 > As I mentioned above, the x64/x86 part in OS_VERSION is fairly simple, see > > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#336 > Appending for just Windows, or in just the cross-arch case, would be fairly > straight forward, I think. > > Re a watershed, we'd could limit that to build target of WINNT_x86-msvc and > '(x64)' in OS_VERSION. If that's most of our users then maybe simpler to > just do everyone. Do we have any data on this already ? Maybe get a couple > of days of logs from Zeus to crunch if we don't. I'm not too particular about what it looks like, but if there's no objections and we can decided on something, it would be great to get this in before 42 hits Aurora. It's invasive enough that jumping the trains is probably a bad idea.
Reporter | ||
Comment 1•9 years ago
|
||
Robert, you're probably the main stakeholder here from the client side. Any thoughts or objections?
Flags: needinfo?(robert.strong.bugs)
Comment 2•9 years ago
|
||
Should be somewhat trivial and I have no objections. There are a couple of other bugs taking precedence atm and I'll make sure this happens this cycle if someone else doesn't pick it up. Is aus setup to handle both formats and if not, when will it be?
Flags: needinfo?(robert.strong.bugs)
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #2) > Should be somewhat trivial and I have no objections. There are a couple of > other bugs taking precedence atm and I'll make sure this happens this cycle > if someone else doesn't pick it up. > > Is aus setup to handle both formats and if not, when will it be? It's not set-up to yet, but trivial to add once we agree on a convention. I'll make sure it happens prior to client side changes landing.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → spohl.mozilla.bugs
Assignee | ||
Comment 4•9 years ago
|
||
I have yet to test this, but does this seem reasonable to you? Also, what's the best way to print the build target in a build? Is this reflected somewhere like the about dialog, or can I print it in the browser console?
The three possible build targets with this patch should be:
> WINNT_x86-msvc-x86 (32-bit build running on 32-bit hardware)
> WINNT_x86_64-msvc-x64 (64-bit build running on 64-bit hardware)
> WINNT_x86-msvc-x64 (32-bit build running on 64-bit hardware)
Attachment #8636779 -
Flags: feedback?(robert.strong.bugs)
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #4) > Created attachment 8636779 [details] [diff] [review] > Patch (not tested) > > I have yet to test this, but does this seem reasonable to you? Also, what's > the best way to print the build target in a build? Is this reflected > somewhere like the about dialog, or can I print it in the browser console? I find the simplest way to be to enable app.update.log, open the browser console, and check for updates.
Comment 6•9 years ago
|
||
The urlConstruction.js xpcshell test already tests these values and should suffice.
Comment 7•9 years ago
|
||
Comment on attachment 8636779 [details] [diff] [review] Patch (not tested) Just did a quick once over and looks fine so far!
Attachment #8636779 -
Flags: feedback?(robert.strong.bugs) → feedback+
Assignee | ||
Comment 8•9 years ago
|
||
Minor whitespace fixes.
Attachment #8636779 -
Attachment is obsolete: true
Attachment #8637599 -
Flags: review?(robert.strong.bugs)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8637600 -
Flags: review?(robert.strong.bugs)
Assignee | ||
Comment 10•9 years ago
|
||
Try is looking green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6028fb9c564
Reporter | ||
Comment 11•9 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #10) > Try is looking green: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6028fb9c564 Woohoo! I just put together the RelEng patch needed to make sure nightlies continue to update over in bug 1184546. It would be good to avoid landing on mozilla-central until that does, otherwise users will get stuck on an older build until it lands. Hopefully it will land tomorrow or Monday.
Assignee | ||
Comment 12•9 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #11) > (In reply to Stephen A Pohl [:spohl] from comment #10) > > Try is looking green: > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6028fb9c564 > > Woohoo! I just put together the RelEng patch needed to make sure nightlies > continue to update over in bug 1184546. It would be good to avoid landing on > mozilla-central until that does, otherwise users will get stuck on an older > build until it lands. Hopefully it will land tomorrow or Monday. Great! I'll definitely hold off until bug 1184546 lands. Thanks!
Updated•9 years ago
|
Attachment #8637599 -
Flags: review?(robert.strong.bugs) → review+
Updated•9 years ago
|
Attachment #8637600 -
Flags: review?(robert.strong.bugs) → review+
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a1163cd7736 https://hg.mozilla.org/integration/mozilla-inbound/rev/c22ad1335d55
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2a1163cd7736 https://hg.mozilla.org/mozilla-central/rev/c22ad1335d55
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Reporter | ||
Comment 15•9 years ago
|
||
I verified this with a 32-bit on 64-bit update today.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•