Closed Bug 253742 Opened 20 years ago Closed 19 years ago

No way of installing platform specific XPCOM components (dll/so) based on user OS.

Categories

(Firefox :: Installer, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox1.5

People

(Reporter: alex, Assigned: jens.b)

References

()

Details

Attachments

(1 file, 7 obsolete files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1 I am developing an extension with has an XPCOM component. I have implemented this component for Windows (.dll file) and for Linux (.so file). All other extension files (.xul, .js, .css etc) are the same for both platforms. I have found no way to specify (in install.rdf or otherwise) which files should be installed on which platform. Namely - "if Linux then install MyFile.so else install MyFile.dll" Putting both files in the "components" directory didn't help either - the component loader (nsNativeComponentLoader.cpp) looks for all the files (.so, .dll etc) on all the platforms and tries to load them. It produces an ugly error message (at least on Windows with .so). I will submit a separate bug on this. Reproducible: Always Steps to Reproduce:
Component: Extension/Theme Manager → Installer
Submitted a related bug on nsNativeComponentLoader (#253747): http://bugzilla.mozilla.org/show_bug.cgi?id=253747
Status: UNCONFIRMED → NEW
Ever confirmed: true
Currently, the Extension Manager just extracts the whole XPI. I'm proposing the following scheme: 1) We introduce <em:component> nodes to install.rdf, like this: <Description about="urn:mozilla:install-manifest"> ... <em:component osarch="ALL" file="foobarGeneric.js"/> <em:component osarch="Win32" file="foobarComponent.dll"/> <em:component osarch="Linux" file="foobarComponent.so"/> ... </Description> 2) Before extracting, the extension manager looks in the metadata and collects a "blacklist" of all components that do not apply to the current platform and do not have osarch="ALL". 3) While extracting, it then skips all files in components/ that are on the blacklist. I'm willing to take this bug and implement the above scheme myself, provided I receive some kind of "go"... Ben and/or bsmedberg, what do you think - is something like the above feasible? Alex, would this be sufficient for you?
This would be great, thanks! A few comments: 1) The "osarch" field should be a substring to match against the "navigator.platform" variable (or maybe a wildcard expression). Without it, we'll need a big enum that would need to be synced with all the possible platform names. 2) It would be nice to extend this scheme to other files as well (not only components). For example - install different skins/icons on different platforms. A related feature would be if the "file" was a wildcard expression, allowing installation of several files or even a whole directory with a single line. This feature is far less important IMHO, just my imagination running wild :) In any case, the simple solution you proposed (plus the first point above) will definitely solve most of the component related installation problems.
(In reply to comment #3) > 1) The "osarch" field should be a substring to match against the > "navigator.platform" variable (or maybe a wildcard expression). Without it, > we'll need a big enum that would need to be synced with all the possible > platform names. No, we wouldn't. There is a compile-time constant named __OSARCH__ we can simply compare against (= each build only knows its own platform). It is used in the patch for bug 272046 and we should use the same value here IMHO.
Reading the discussion on bug #272046: - 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. - alanjstr: UMO wouldn't know what to do with processor. We're really not that specific. I'll see what I can find. - Christian Biesinger: are there no binary extensions on it? linux/ppc can't run linux/x86 binaries... IMHO, this case is a bit different than the above. Here we are talking about binary components and linux/ppc linux/x86 distinction is very important.
(CCing the people involved in bug 272046, as this is clearly related) (In reply to comment #5) > Reading the discussion on bug #272046: > > - 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. > - alanjstr: > UMO wouldn't know what to do with processor. We're really not that > specific. I'll see what I can find. > - Christian Biesinger: > are there no binary extensions on it? linux/ppc can't run linux/x86 > binaries... > > IMHO, this case is a bit different than the above. Here we are talking about > binary components and linux/ppc linux/x86 distinction is very important. Does anybody know how to distinguish this? Is __OSARCH__ really the same for linux/ppc and linux/x86? There is navigator.oscpu, which returns "Windows NT 5.0" on Windows 2000... But that looks like it's too fine-grained - after all, Win32-built extensions should also run on XP and Windows 98... So we'd be back at the regex/substring thing, which seems error-prone to me...
Perhaps we should simply use both: <em:component osarch="Win32" oscpu="Windows NT 5.0" file="foobarComponent.dll"/> Then you could use osarch, oscpu, or both, depending on which is safest. Note: it seems navigator.oscpu cannot be overridden via prefs, and is therefore safe to use here.
Where do we get navigator.oscpu from, and what are the possibilities? Can it distinguish between Win32 and Win64?
(In reply to comment #8) > Where do we get navigator.oscpu from, and what are the possibilities? Can it > distinguish between Win32 and Win64? It's available globally - type navigator.oscpu into the JS console to see your value. It's provided by the OS at run-time, and is the same as in the default (not overridden) user agent string. Guessing from web server logs, the processor is in there, but I don't know about Win32/64 and linux on PPC... I even found another alternative candidate: the target constant used at http://lxr.mozilla.org/mozilla/source/toolkit/content/buildconfig.html.in#16 - for my official Mozilla1.7 build, it's "i586-pc-msvc". Although I'm not sure whether it's desired or neccessary to distinguish between the compilers used (do MinGW-built components work with MSVC mozilla builds?), and don't know if that value can be retrieved the same way as OSARCH.
Okay, more info on navigator.oscpu: It's retrieved from nsHttpHandler, which uses platform-dependent methods for retrieving the OS version, and produces more or less unified output. The code is at http://lxr.mozilla.org/mozilla/source/netwerk/protocol/http/src/nsHttpHandler.cpp#608 and looks like it will distinguish even unknown Windows versions; all Unix-based OSes are queried via uname (where linux/ppc and linux/x86 should return different values).
navigator is a property of window (i.e. you have window.navigator.oscpu). so in case ext. mgr is a js component, you can't access it directly, although you can of course ask httphandler directly.
(In reply to comment #2) > 1) We introduce <em:component> nodes to install.rdf, like this: I say make it generic, and find a way to use <em:file> if possible in a backward compatible fashion. There might be platform-specific jar files or defaults files too. Also, it should be optional metadata that controls whether or not a given file is copied, but if no metadata is supplied in install.rdf the file is just extracted, for backward compatibility with 1.0 manifests.
Okay, a generic solution is to introduce optional em:platform nodes inside em:file, which can be used for files needing chrome registration and those that do not. The EM silently ignores <em:file> nodes not having <em:package|skin|locale> child nodes, so packages would still run in Firefox 1.0 (which however would install and register all jars, but one could make a Firefox-minVersion=1.1 package to prevent that). Note that this proposal stays with the current name-only, path-less about-urn for em:file (e.g. urn:mozilla:extension:file:myComponent.dll, not components/myComponent.dll), but name collisions don't seem likely. Taking.
Assignee: bugs → jens.b
Target Milestone: --- → Firefox1.1
Attached file example em:file nodes (obsolete) (deleted) —
Attaching some examples for illustration. (Using only osarch restriction, but I'll add oscpu comparison, too)
I checked this out a bit further. Indeed, nsHttpHandler can be used to get all the needed info: var httpHandler = Components.classes["@mozilla.org/network/protocol;1?name=http"] .createInstance(Components.interfaces.nsIHttpProtocolHandler); var platform = httpHandler.platform; var oscpu = httpHandler.oscpu; "platform" can have one of the following values: - Photon - OS/2 - Windows - Macintosh - BeOS - X11 - BeOS "oscpu" contains additional info about os and cpu. Like it was already mentioned, uname is used on Unixes. More information can be found here: http://www.mozilla.org/build/revised-user-agent-strings.html Using osarch, oscpu, or both will give enough flexibility to handle complex scenarios. One important point IMHO - I think the values should be handled as regexps (at least for the oscpu part). This way it would be possible to cover several related platforms with one expression. Otherwise, the author will have to provide a long list of all the possible variations, which might be impossible. See the attachment for some examples.
Attached file platform and oscpu with regexps (obsolete) (deleted) —
>.createInstance(Components.interfaces.nsIHttpProtocolHandler); NO, use getService here!
We need to align the Platform values with what the Web Service is using.
(In reply to comment #18) > We need to align the Platform values with what the Web Service is using. Agreed. And the other way round, it might actually make sense to pass oscpu to the web service, too, even when it's not (yet) used by UMO - but that should be filed as a separate bug. (In reply to comment #15) > One important point IMHO - I think the values should be handled as regexps (at > least for the oscpu part). Is there a safe way (without using eval()) to construct a regexp object from a string?
js> x=new RegExp("foo.*bar"); /foo.*bar/
Assignee: jens.b → bugs
QA Contact: bugs → bugzilla
oops, didn't mean to change the assignee :(
Assignee: bugs → jens.b
QA Contact: bugzilla → bugs
So does platform return the same value as OSARCH?
(In reply to comment #22) > So does platform return the same value as OSARCH? No, it doesn't. Although both are #ifdef-based, it's two separate implementations (some overlapping, but not identical). Therefore I'll stick with the original OSARCH constant, plus the oscpu value from nsIHttpProtocolHandler. (In reply to comment #20) > js> x=new RegExp("foo.*bar"); > /foo.*bar/ Ah, thanks! Never saw that one...
Attached patch proposed patch (obsolete) (deleted) — Splinter Review
I finished the implementation. The install.rdf syntax now looks like: <em:file> <Description about="urn:mozilla:extension:file:myComponent.dll"> <em:platform> <Description> <em:osarch>WinNT</em:osarch> <em:oscpu>Windows NT 5.1</em:oscpu> </Description> </em:platform> </Description> </em:file> Changes and implementation notes: - em:file is reused in backwards-compatible way (comment 12). The em:file nodes match your files by name (not restricted to components directory). - The syntax I originally proposed was not XMLRF, so there are subtle differences (Description node inside em:platform, osarch/oscpu not as attributes but sub-elements) - I dropped the idea of having something like osarch=ALL, instead platform-neutral files simply have no em:platform info (like most chrome jar files), or are not listed in install.rdf - Both osarch and oscpu values are interpreted as regular expressions. It is always case-insensitive, and must match the whole string (implied ^ and $). The delimiter slashes are not included in the install.rdf. - Files are excluded when none of its em:platform rules (each consisting of osarch, oscpu or both) is met. Yes, this means you can have multiple em:platform nodes per file. The new install process is as follows: - The install.rdf is extracted separately, before the main extraction routine runs. - An exclusion list is generated by matching the em:platform info for each file. - The XPI is extracted, leaving out install.rdf and all files on the exclusion list. For those not building Mozilla: you can test this, too - the patch can be applied to the file Firefox-Dir\components\nsExtensionManager.js directly. You will only have to insert your platform, so the line with the #expand preprocessor directive is replaced by something like: const TARGET_OS = "WINNT";
Blocks: 255619
Attaching a new patch to catch up with the checkin for bug 272046.
Attachment #170055 - Attachment is obsolete: true
Attachment #170142 - Flags: review?(bsmedberg)
I don't think this is a good solution. Instead, let's make it a an alternate directory structure that gets registered automatically: <extension>/components (JS components, cross-platform) <extension>/components_Linux_i386_gcc3 <extension>/components_Linux_i386_gcc3_gtk2 <extension>/components_Linux_i386_gcc3_qt <extension>/components_Win_i386_msvc <extension>/components_Win_i386_mingw <extension>/components_Win_ia64_msvc etc... This can all be handled in nsXREDirProvider.cpp, and doesn't need any hacking of the extension manager (although we could, if we only wanted to install the components we were actually going to use).
(In reply to comment #26) > I don't think this is a good solution. Instead, let's make it a an alternate > directory structure that gets registered automatically (...) That wouldn't have the advantage of being a generic solution like Ben proposed in comment 12, and I really like that idea (platform-specific jars, pref files)... What exactly is bad about the proposed solution - the regex, or the fact it's in install.rdf? Besides, if we implement your proposal, what do you suggest as a solution for bug 255619? I set this bug blocking it because the whole manifest could also declare its platform compatibility with em:platform nodes...
> <extension>/components_Linux_i386_gcc3 what if a component works on both gcc 3 and gcc 4 mozillas, but not on gcc 2?
biesi: That kinda depends on whether the GCC3 and 4 interface vtable layouts are compatible. I suspect that we're gonna stick to the GCC3 ABI for a long time, even when compiling with GCC4. In that case, the "gcc3" directory would be correct, and gcc4 would be ignored.
Ben, what do you think? Should the generic, metadata-based per-file approach be dropped in favor of Benjamin's component-only, directory-based proposal?
I would like something to be in the metadata so that update.mozilla.org can detect it. It doesn't have to be very detailed.
Flags: blocking-aviary1.1?
Whiteboard: has-patch
Comment on attachment 170142 [details] [diff] [review] updated patch to match nsExtensionManager.js.in revision 1.79 Quick update: it's not been decided yet whether metadata or hard-coded directories will be used. In either case, the solution will depend on both platform and compiler (MingW-built components won't work with MSVC-built Mozilla releases, for example), so my patch is obsolete.
Attachment #170142 - Attachment is obsolete: true
Attachment #170142 - Flags: review?(benjamin)
Benjamin, have you and Ben agreed on a course of action for this?
I'm sorry but I think you're not going the right way in this patch (and quite possibly in the other related thinks, bug 272046, etc ...) Jens Bannmann was on the right track in comment #9, you need the info from @TARGET@ in buildconfig, because the compiler used *is* relevant. Different compilers on the same platform have different ABI, and generate incompatible XPCOM components. It's completely impossible to use a binary XPCOM component compiled with msvc on a MinGW build, it will crash Mozilla. On the other end, we don't care much about the 2000/NT/XP distinction and @TARGET@ doesn't make it. On the Unix platform, it would be best to get back the version of GCC. Some version made incompatible changes, the transition to GCC 3 made itself a reputation to this regard. But if there's no way, we'll have to do without it. The code below is a sample of reading buidconfig and getting back some values : http://lxr.mozilla.org/mozilla/source/extensions/reporter/resources/content/reporter/reportWizard.js#260
Depends on: 294835
I talked to Benjamin on IRC about the approach that we're going to take. I'll post a link to the plan description once it's in a decent state. (In reply to comment #34) > I'm sorry but I think you're not going the right way in this patch (and quite > possibly in the other related thinks, bug 272046, etc ...) Jean-Marc, the patch was already marked obsolete before you commented. The upcoming new plan addresses the exact problems you and others commented on before.
Whiteboard: has-patch
Attachment #169807 - Attachment is obsolete: true
Comment on attachment 169828 [details] platform and oscpu with regexps The plan is up at http://wiki.mozilla.org/Toolkit:Platform_Specific_Extension_Components . In a nutshell, we're going for a directory-based approach, generalized for components and other files. Comments welcome (at the wiki talk page or here), especially on the open issues.
Attachment #169828 - Attachment is obsolete: true
No longer blocks: 255619
Attaching first draft. The patch does not depend on TARGET_ABI (bug 294835), it will just skip the ABI dirs in case it's not defined.
Attachment #184623 - Flags: review?(benjamin)
Comment on attachment 184623 [details] [diff] [review] introduce platform directories for extension components I don't think we need to do this in config.mk. Just put the DEFINES += -D... in toolkit/xre/Makefile.in Other than that, this looks great!
note that the claim about incompatible libraries is only half true. they're incompatible today on win32, but that's a bug. mozilla for os/2 is capable of mixing libraries from both mingw and visdual age, and someone could and hopefully will write similar glue for windows.
For OS/2, is it glue or do they respect a platform ABI ? I hardly see how you can get the glue working generically if the compilers have significantly different conventions for packing the C++ class and virtual functions tables (or you will require each extension developer to include the specific glue code required). I think XPCOM avoids the exception and RTTI part of the problem.
+#ifdef TARGET_ABI is that really the right syntax for makefiles?
the abis don't match. afaik, basically any object of the non dominant flavor ends up being wrapped by a wrapper that understands how to convert between the two abis. it would wrap objects crossing that boundary.
Now using toolkit/xre/Makefile.in instead of config.mk, and featuring the correct makefile syntax (thanks, biesi!) Verified that the ABI directories are in fact skipped when TARGET_ABI is unknown.
Attachment #184623 - Attachment is obsolete: true
Attachment #184753 - Flags: review?(benjamin)
Attachment #184623 - Flags: review?(benjamin)
Attachment #184753 - Flags: review?(benjamin) → review+
Attachment #184753 - Flags: approval-aviary1.1a2?
Attachment #184753 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Changing TARGET_ABI to TARGET_XPCOM_ABI to match bug 294835. Still r=bsmedberg a=asa; ready for checkin.
Attachment #184753 - Attachment is obsolete: true
Checked in. Leaving this bug open until I finish some neccessary cleanup.
Status: NEW → ASSIGNED
Comment on attachment 185599 [details] [diff] [review] introduce platform directories for extension components, v3 mozilla/toolkit/xre/nsXREDirProvider.cpp 1.29 mozilla/toolkit/xre/Makefile.in 1.44
Attachment #185599 - Attachment is obsolete: true
Timeless pointed out some flaws in the last patch which I fix here.
Attachment #185709 - Flags: review?(benjamin)
Attachment #185709 - Flags: review?(benjamin) → review+
Attachment #185709 - Flags: approval-aviary1.1a2?
Attachment #185709 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Attachment #185709 - Attachment description: Cleanup for nsXREDirProvider.cpp: reduce number of created string instances → Cleanup for nsXREDirProvider.cpp: reduce number of created string instances [checked in]
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Flags: blocking-aviary1.1?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: