272046 - Pass OS to web service

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[alanjstr]

The Extension Manager should tell the web service what operating system it is
seeking updates for so that only compatible listings will be shown.  We probably
don't want to rely on user agent, though, since that is changable.

Comment 1

[alanjstr]

Attachment: Patch nsExtensionManager.js.in

Comment 2

[alanjstr]

Attachment: Patch extensions.properties

Comment 3

[Benjamin Smedberg]

OS_ARCH is not sufficient. We need something that identifies the system more
uniquely than just OS: OS+processor might be sufficient for normal usage.

Comment 4

[alanjstr]

UMO wouldn't know what to do with processor.  We're really not that specific. 
I'll see what I can find.

Comment 5

[Benjamin Smedberg]

Comment on attachment 167218 [details] [diff] [review]
Patch nsExtensionManager.js.in

Whoops. You're using $OS_ARCH as if this were a makefile or a shell script. To
get this variable into the XUL preprocessor you need to add it to the DEFINES
makefiles variable.

Comment 6

[alanjstr]

Additional patch needed for toolkit/mozapps/extensions/src/Makefile.in to define
the variable.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #4)
> UMO wouldn't know what to do with processor.  We're really not that specific. 
> I'll see what I can find.

are there no binary extensions on it? linux/ppc can't run linux/x86 binaries...

Comment 8

[alanjstr]

> are there no binary extensions on it? linux/ppc can't run linux/x86 binaries...

Most seem to be cross platform.

Comment 9

[Jens Bannmann]

(In reply to comment #8)
> Most seem to be cross platform.

Correct, but only because UMO and Firefox do not yet support OS info in
manifests... For example, Enigmail needs a separate Enigmime package which *is*
platform specific. Currently, you have to separately download it from their
homepage - if we do OS info right (including processor), binary packages like
Enigmime could go to UMO, too, and be presented as a dependency (or even merged
with the main extension).

Comment 10

[alanjstr]

Enigmail/Enigmime was the only one I could think of.  

bsmedberg -
Does nsExtensionManager.js.in pass if I take care of the Makefile.in?

Comment 11

[Benjamin Smedberg]

You have to use the correct preprocessor syntax, either #expand or #filter
substitution

Comment 12

[alanjstr]

Attachment: nsExtensionManager.js.in patch

Comment 13

[alanjstr]

I'm confused about what I need in Makefile.in.  OS_ARCH is already defined, right?

Comment 14

[alanjstr]

Attachment: nsExtensionManager.js.in patch

Ok, this uses
#expand const TARGET_OS  = "__OSARCH__";
where OS_ARCH is defined in config/config.mk

Comment 15

[Benjamin Smedberg]

Alan, my review is sufficient for this to be committed, unless mconnor
specifically asked to review this himself.

Comment 16

[alanjstr]

Checked in by timeless.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This turned all the firefox tinderboxes orange.

Comment 18

[alanjstr]

They couldn't keep the tree from going orange.  Reason tbd.

Comment 19

[Jens Bannmann]

Comment on attachment 169139 [details] [diff] [review]
nsExtensionManager.js.in patch

>+#expand const TARGET_OS                                 = "__OSARCH__";

It seems the substitution adds its own quotes, I got ---> = ""WINNT"";
when I built Firefox. Therefore, we should retry with the following:

>+#expand const TARGET_OS                                 = __OSARCH__;

Comment 20

[Jens Bannmann]

Attachment: revised patch for nsExtensionManager.js.in

Attaching a revised patch for nsExtensionManager.js.in. The other attachment
does not need changes.

Does this need review again to be checked in? If not, feel free to remove the
request.

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 169938 [details] [diff] [review]
revised patch for nsExtensionManager.js.in

this should work, r=me

Comment 22

[Ian Neal]

2nd and 5th patches checked in:
Checking in mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--
 nsExtensionManager.js.in
new revision: 1.79; previous revision: 1.78
done
Checking in locales/en-US/chrome/mozapps/extensions/extensions.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v
 <--  extensions.properties
new revision: 1.6; previous revision: 1.5
done

Comment 23

[alanjstr]

Trees are green after landing

Comment 24

[Benjamin Smedberg]

I'm going to reopen this so that we can pass the entire TARGET_XPCOM_ABI, which
is necessary for any extension that wants to ship binaries.

Comment 25

[Benjamin Smedberg]

Don't mind me, jensb already did this.