1373367 - prefix values in SYSTEM_CAPABILITIES field

Status: RESOLVED FIXED

(Toolkit :: Application Update, enhancement, P5)

Description

[[e-mail] (:bhearsum)]

Attachment: prefix values in SYSTEM_CAPABILITIES

We added memory to the SYSTEM_CAPABILITIES field of the update URL in bug 1366917 by making it a simple comma separated field. This is OK, but it makes parsing it on the server a bit more dangerous (especially if we ever make the mistake of having a different field order depending on Firefox version, which could happen if we ever remove something from that field). To make this easier, it would be good to prefix values with a name.

This patch should do that, and produces URLs such as:
https://aus5.mozilla.org/update/6/Firefox/56.0a1/20170615175447/Linux_x86-gcc3/en-US/default/Linux%204.8.0-51-generic%20(GTK%203.20.9%2Clibpulse%209.0.0)/ISET:NA,MEM:15914/default/default/update.xml?force=1

Comment 1

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

Comment on attachment 8878142 [details] [diff] [review]
prefix values in SYSTEM_CAPABILITIES

You need to update the test as well
https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/tests/xpcshell/test_UpdateUtils_url.js#348

Comment 2

[[e-mail] (:bhearsum)]

Attachment: add prefix + fix tests

I'll need to update Balrog to support both styles before this can be landed safely. I'll do that as part of bug 1367054.

Comment 3

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

Comment on attachment 8878526 [details] [diff] [review]
add prefix + fix tests

Would be good to run the tests first or push it to try... just in case.

I'll also need to let Avast know about this change.

Comment 4

[[e-mail] (:bhearsum)]

I pushed this to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cdf1e4b4ff1b98cc9cba28be89758f1fda31de2 and https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c72e201a24175a8a6189408b46cc65eaf0bf925

Other than what appear to be unrelated non-xpcshell intermittent oranges, things are passing.

Comment 5

[[e-mail] (:bhearsum)]

This is now supported in production Balrog, eg:
https://aus5.mozilla.org/update/6/Firefox/48.0.2/20161031133903/WINNT_x86-msvc-x86/pt-BR/release/Windows_NT%2010.0.0.0%20(x86)(nowebsense)/ISET:SSE,MEM:800/default/default/update.xml and https://aus5.mozilla.org/update/6/Firefox/48.0.2/20161031133903/WINNT_x86-msvc-x86/pt-BR/release/Windows_NT%2010.0.0.0%20(x86)(nowebsense)/SSE/default/default/update.xml return the same thing

As do https://aus5.mozilla.org/update/6/Firefox/48.0.2/20161031133903/WINNT_x86-msvc-x86/pt-BR/release/Windows_NT%2010.0.0.0%20(x86)(nowebsense)/ISET:SSE2,MEM:800/default/default/update.xml and https://aus5.mozilla.org/update/6/Firefox/48.0.2/20161031133903/WINNT_x86-msvc-x86/pt-BR/release/Windows_NT%2010.0.0.0%20(x86)(nowebsense)/SSE2,800/default/default/update.xml

I'm going to get this landed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57164b113286
prefix values in SYSTEM_CAPABILITIES field. r=rstrong

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 8878526 [details] [diff] [review]
add prefix + fix tests

https://hg.mozilla.org/integration/mozilla-inbound/rev/57164b11328641c6ad9f89ca6a941418b474d99a

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/57164b113286