Closed Bug 1183077 Opened 9 years ago Closed 9 years ago

include running arch in build target on Windows

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: bhearsum, Assigned: spohl)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
Robert, you're probably the main stakeholder here from the client side. Any thoughts or objections?
Flags: needinfo?(robert.strong.bugs)
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)
(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.
Depends on: 1184546
Assignee: nobody → spohl.mozilla.bugs
Attached patch Patch (not tested) (obsolete) — Splinter Review
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)
(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.
The urlConstruction.js xpcshell test already tests these values and should suffice.
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+
Attached patch PatchSplinter Review
Minor whitespace fixes.
Attachment #8636779 - Attachment is obsolete: true
Attachment #8637599 - Flags: review?(robert.strong.bugs)
Attached patch Test changesSplinter Review
Attachment #8637600 - Flags: review?(robert.strong.bugs)
(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.
(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!
Attachment #8637599 - Flags: review?(robert.strong.bugs) → review+
Attachment #8637600 - Flags: review?(robert.strong.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/2a1163cd7736
https://hg.mozilla.org/mozilla-central/rev/c22ad1335d55
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
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.

Attachment

General

Created:
Updated:
Size: