1001542 - Change SystemInformation for B2G to report B2G name & version rather than kernel name and version

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Dave Hylands [:dhylands]]

Currently, PRODUCT_MODEL is available, however this is a user-facing string and often contains embedded spaces.

It would be more appropriate to allow PRODUCT_DEVICE instead.

Comment 1

[Dave Hylands [:dhylands]]

Attachment: Add PRODUCT_DEVICE to fields that can be substituted

Comment 2

[Dave Hylands [:dhylands]]

Comment on attachment 8412758 [details] [diff] [review]
Add PRODUCT_DEVICE to fields that can be substituted

Clearing review request, since some additional fields are also being requested.

Comment 3

[Dave Hylands [:dhylands]]

Attachment: Also add BOARD_PLATFORM and BUILD_VERSION_SDK

Added a couple more vars (at mwu's request)

Comment 4

[Michael Wu [:mwu]]

<bhearsum> OS_VERSION is probably the right place to put them...

A combination of BOARD_PLATFORM and BUILD_VERSION_SDK placed in OS_VERSION would also work for our purposes.

Comment 5

[Dave Hylands [:dhylands]]

But shouldn't OS_VERSION reflect the OS Version? i.e. the version of the linux kernel?

I suppose for android phones, you could probably make a case that BUILD_VERSION_SDK has some sort of equivalence to OS_VERSION, but board_platform has nothing to do with the os version. It's more about the hardware (i.e. which phone hardware is being used).

Comment 6

[Dave Hylands [:dhylands]]

If you're looking to use the same URL pattern for all platforms, then I think you'd be better off to add a totally new designator that defaults to OS_VERSION for not FxOS and is the some combination of ro.product.device and ro.build.version.sdk

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 8412768 [details] [diff] [review]
Also add BOARD_PLATFORM and BUILD_VERSION_SDK

Let's get feedback from the people that work on aus. Might want nthomas to weigh in as well.

Clearing review until this is sorted.

Comment 8

[Michael Wu [:mwu]]

If where we put BOARD_PLATFORM and BUILD_VERSION_SDK is controversial, we can punt that to a separate bug - getting PRODUCT_DEVICE reported is a bit more important right now.

Comment 9

[[e-mail] (:bhearsum)]

(In reply to Dave Hylands [:dhylands] from comment #5)
> But shouldn't OS_VERSION reflect the OS Version? i.e. the version of the
> linux kernel?

It does, but we also put additional platform information in it. For example, my Firefox Beta is:
https://aus4.mozilla.org/update/3/Firefox/29.0/20140417185217/Linux_x86_64-gcc3/en-GB/beta/Linux%203.13.0-24-generic%20(GTK%202.24.23)/default/default/update.xml?force=1

In an ideal world, we'd have completely separate fields for all of these things. The update server only supports URLs like above right now, though

Comment 10

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #9)
> (In reply to Dave Hylands [:dhylands] from comment #5)
> > But shouldn't OS_VERSION reflect the OS Version? i.e. the version of the
> > linux kernel?
> 
> It does, but we also put additional platform information in it. For example,
> my Firefox Beta is:
> https://aus4.mozilla.org/update/3/Firefox/29.0/20140417185217/Linux_x86_64-
> gcc3/en-GB/beta/Linux%203.13.0-24-generic%20(GTK%202.24.23)/default/default/
> update.xml?force=1
> 
> In an ideal world, we'd have completely separate fields for all of these
> things. The update server only supports URLs like above right now, though

Hit enter too soon.

Since the update server only supports URLs like the above, we need to put whatever information it is you want _somewhere_, and OS_VERSION seems like the most logical place, based on what we've done in the past. (The two fields after it are DISTRIBUTION and DISTRIBUTION_VERSION, which we use for partner repack information. I suspect we should reserve those for partner information for b2g, too.)

(In reply to Michael Wu [:mwu] from comment #8)
> If where we put BOARD_PLATFORM and BUILD_VERSION_SDK is controversial, we
> can punt that to a separate bug - getting PRODUCT_DEVICE reported is a bit
> more important right now.

When we're less crunched for time, I'm happy to talk about modifying the update server to support the additional information more robustly, so I'd be OK with this. It would help if someone explained what BOARD_PLATFORM and BUILD_VERSION_SDK actually are though...do they track with Android version? B2G version? Device hardware? Something else (or nothing)?

Comment 11

[Dave Hylands [:dhylands]]

I couldn't find much documentation on ro.board.platform, other than when loading hal modules, android uses the following as modifiers on the filename (in the order presented):

ro.hardware
ro.product.board
ro.board.platform
ro.arch

The ro.build.version.sdk has to do with the SDK version, which changes say between JB and KK

Comment 12

[Michael Wu [:mwu]]

The combination of ro.board.platform and ro.build.version.sdk gives us a rough idea of what BSP the device has. ro.board.platform often contains an unique identifier for the SoC class. Devices based off the same BSP will have the same combo of ro.board.platform and ro.build.version.sdk, though matching ro.board.platform and ro.build.version.sdk won't guarantee us that the BSPs are the same. However, they'll probably be similar enough and it's probably as close as we can get.

Trying to identify devices with similar/same BSP is useful for generic gecko/gaia updates - we can distribute the same update to all those devices. There's no guarantee that it would work, but it would provide a good option for users who want to try updates from us.

Comment 13

[Dave Hylands [:dhylands]]

Attachment: Modify OS_VERSION to report B2G version info rather than linux version info

Ben - Is this what you're looking for?

If so, then I can combine the 2 patches together.

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 8415598 [details] [diff] [review]
Modify OS_VERSION to report B2G version info rather than linux version info

Review of attachment 8415598 [details] [diff] [review]:
-----------------------------------------------------------------

I don't really understand the code, but if it's doing what your description says, WFM.

Comment 15

[Dave Hylands [:dhylands]]

bhearsum,

So what code does is that it replaces the 'name' property with Boot2Gecko and the 'version' property with 2.0.0.0-prerelease so that OS_VERSION that nsUpdateService.js see will wind up being: 'Boot2Gecko%202.0.0.0-prerelease'

Comment 16

[[e-mail] (:bhearsum)]

(In reply to Dave Hylands [:dhylands] from comment #15)
> bhearsum,
> 
> So what code does is that it replaces the 'name' property with Boot2Gecko
> and the 'version' property with 2.0.0.0-prerelease so that OS_VERSION that
> nsUpdateService.js see will wind up being: 'Boot2Gecko%202.0.0.0-prerelease'

That sounds perfect!

Comment 17

[Dave Hylands [:dhylands]]

So then the question is: Do we want both patches? Or just the second one? bhearsum is still showing as f? on the first attachment.

Comment 18

[[e-mail] (:bhearsum)]

(In reply to Dave Hylands [:dhylands] from comment #17)
> So then the question is: Do we want both patches? Or just the second one?
> bhearsum is still showing as f? on the first attachment.

We definitely need the PRODUCT_DEVICE part from the first patch unless we've changed our mind about PRODUCT_MODEL being something we can use. It sounds like we don't need the BOARD_PLATFORM or BUILD_VERSION_SDK though, as OS_VERSION will be used instead.

Comment 19

[Robert Kaiser]

I still don't understand how PRODUCT_MODEL is not something that can be used (why did we implement it being available there in the first place?) - esp. as it's what we use pretty much everywhere else to determine the device.

Comment 20

[Dave Hylands [:dhylands]]

(In reply to Robert Kaiser (:[e-mail]) from comment #19)
> I still don't understand how PRODUCT_MODEL is not something that can be used
> (why did we implement it being available there in the first place?) - esp.
> as it's what we use pretty much everywhere else to determine the device.

There's no reason why it couldn't be used (i.e. the patch doesn't take it away).

It's just that PRODUCT_MODEL is designed to be something user facing, like "Full Android on Emulator", which is the PRODUCT_MODEL for the emulator.

Comment 21

[Dave Hylands [:dhylands]]

Comment on attachment 8415598 [details] [diff] [review]
Modify OS_VERSION to report B2G version info rather than linux version info

Review of attachment 8415598 [details] [diff] [review]:
-----------------------------------------------------------------

This patch changes SystemInfo to report Boot2Gecko in the 'name' property and the b2g version in the 'version' property.

Prior to this patch, 'Linux' is reported in 'name' and the kernel version was reported in 'version' (I moved the kernel version to property 'kernel_version' - same thing as what android does)

Comment 22

[Dave Hylands [:dhylands]]

Comment on attachment 8412768 [details] [diff] [review]
Also add BOARD_PLATFORM and BUILD_VERSION_SDK

marking as obsolete since the os_version change seems to be what's really needed.

Comment 23

[Robert Strong (they/them - no direct email)]

Comment on attachment 8415598 [details] [diff] [review]
Modify OS_VERSION to report B2G version info rather than linux version info

Giving feedback+ since I'm not an xpcom peer

Comment 24

[Dave Hylands [:dhylands]]

Comment on attachment 8415598 [details] [diff] [review]
Modify OS_VERSION to report B2G version info rather than linux version info

Review of attachment 8415598 [details] [diff] [review]:
-----------------------------------------------------------------

So much for beliving bugzilla's recommendations :)

Comment 25

[Benjamin Smedberg]

Comment on attachment 8415598 [details] [diff] [review]
Modify OS_VERSION to report B2G version info rather than linux version info

I don't have any knowledge about whether this is correct, so I'm going to delegate this to Fabrice.

Comment 26

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/b2g-inbound/rev/8013e9203568

Comment 27

[Ed Morley [:emorley]]

Fixup for breakage caused by a bad merge last night:
https://hg.mozilla.org/integration/b2g-inbound/rev/9040b7692048

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8013e9203568

Comment 29

[[e-mail] (:bhearsum)]

Thanks everyone!

Comment 30

[[e-mail] (:bhearsum)]

I think we either the first patch or a modified version of it still. AFAIK we've settled on %PRODUCT_DEVICE% to report the device type in the update ping. Looks like Dave is out for now, needinfoing others as well...

Comment 31

[Dave Hylands [:dhylands]]

Comment on attachment 8412758 [details] [diff] [review]
Add PRODUCT_DEVICE to fields that can be substituted

I un-obsoleted the patch which adds PRODUCT_DEVICE.

Who should review this?

Comment 32

[[e-mail] (:bhearsum)]

Comment on attachment 8412758 [details] [diff] [review]
Add PRODUCT_DEVICE to fields that can be substituted

Review of attachment 8412758 [details] [diff] [review]:
-----------------------------------------------------------------

Rob is my best guess...maybe Ehsan as a fallback?

Comment 33

[Robert Strong (they/them - no direct email)]

I'll get to it today

Comment 34

[Dave Hylands [:dhylands]]

 https://hg.mozilla.org/integration/b2g-inbound/rev/7e8524eaa2ce

Comment 35

[[e-mail] (:bhearsum)]

Thanks Dave!

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7e8524eaa2ce

Comment 37

[[e-mail] (:bhearsum)]

Comment on attachment 8412758 [details] [diff] [review]
Add PRODUCT_DEVICE to fields that can be substituted

Drivers, I'd like to backport both patches here to b2g30 so that we can use our new update server, which is more reliable and enables better compatibility with Socorro for crash reports.

There were no issues when this landed on mozilla-central.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/0ecffe7cd658
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/a4a0951cd94e