Closed Bug 311735 Opened 20 years ago Closed 20 years ago

when --disable-libxul, build a little xul.dll with the proper exports

Categories

(Toolkit Graveyard :: XULRunner, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(2 files, 2 obsolete files)

When we're building a "developer" build of libxul with --disable-libxul (to reduce the static-linking pain), we should still create a xul.dll with the XRE_* exports. This will also significantly simplify some of the pain of the toolkit/xre -> browser/app static/dynamic/libxul ifdefs.
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha1
This patch builds a little libxul with firefox dynamic and with xulrunner --disable-libxul. I couldn't get it to work with Firefox static but that's ok. It also gets rid of some copy/pasted code in xulrunner/app which was for xulrunner static builds, which is not something it makes sense to support.
Attachment #203170 - Flags: first-review?(darin)
Comment on attachment 203170 [details] [diff] [review] Build a little libxul with ffox dynamic and xulrunner --disable-libxul >Index: xpcom/base/nscore.h >+/** >+ * Import/export macros for libXUL APIs. >+ */ >+#ifdef XPCOM_GLUE >+#define XUL_API(type, name, params) type NS_FROZENCALL (*name) params >+#elif defined(IMPL_XULAPI) >+#define XUL_API(type, name, params) EXPORT_XPCOM_API(type) name params >+#else >+#define XUL_API(type, name, params) IMPORT_XPCOM_API(type) name params >+#endif Do these have to be here? Can they instead be in the header file where the LibXUL exports are defined? Or in some common LibXUL header file? XPCOM is not XUL just as it is not GFX. We don't define the GFX utility library's similar macros in nscore.h, so we shouldn't define LibXUL's there either IMO ;-) >Index: toolkit/xre/nsXULAppAPI.h >+XUL_API(int, >+ XRE_main, (int argc, char* argv[], const nsXREAppData* sAppData)); Hrm... the macro'ization of this is not very pretty, but I guess you are doing it so that you can turn these into global function pointers when XPCOM_GLUE is defined. Hrm... we don't do this for XPCOM. Are you planning similar changes to XPCOM? I also don't see these function pointers being assigned values anywhere. Is this a work in progress? All of these symbols are prefixed with "XRE_"... why not "XRE_API" then? Or, why not change the symbol names to "NS_" to reduce acronyms and unify our frozen API namespace? We have the chance to define this API from the start, so why not make it consistent in style?
> Do these have to be here? Can they instead be in the header file > where the LibXUL exports are defined? Or in some common LibXUL > header file? XPCOM is not XUL just as it is not GFX. We don't > define the GFX utility library's similar macros in nscore.h, so > we shouldn't define LibXUL's there either IMO ;-) Down with XPCOM! ok, I'll make a xulcore.h somewhere. > Hrm... the macro'ization of this is not very pretty, but I guess you > are doing it so that you can turn these into global function pointers > when XPCOM_GLUE is defined. Hrm... we don't do this for XPCOM. Are > you planning similar changes to XPCOM? Yes, these are function pointers when XPCOM_GLUE is defined, and they will be loaded by the function in bug 316098. (extra glue to do this easily not yet written, but it's not necessary for this patch) > All of these symbols are prefixed with "XRE_"... why not "XRE_API" then? > Or, why not change the symbol names to "NS_" to reduce acronyms and unify > our frozen API namespace? We have the chance to define this API from the > start, so why not make it consistent in style? I'd like to have the libxul APIs with a different prefix than the NS_ APIs which are exported from xpcom.dll, and in fact I'd like to make the Current NS_*_P symbols use the same prefix (NS_InitXPCOM3 from xpcom.dll, and XRE_InitXPCOM3 from xul.dll). I can make it XRE_API instead of XUL_API if that's preferable, or change the name of the exports to XUL_main.
Alright... I guess I'm OK with "XRE_" for all of the symbols exported from XUL.dll, and XRE_API sounds like it would be more consistent with that. I think it is unfortunate for newcomers to have to deal with "PR_", "NS_", and now "XRE_", but OK :-/
This has xrecore.h, XRE_API, and this shows how winembed (and other future standalone-glue embedders) should use the function-pointer declarations to get at the libxul symbols, modulo the XP loading code in bug 316098.
Attachment #203170 - Attachment is obsolete: true
Attachment #203292 - Flags: first-review?(darin)
Attachment #203170 - Flags: first-review?(darin)
why not name the file "xre.h" rather than "xrecore.h"?
It's parallel to "nscore.h" and "prtypes.h"... we may want an overarching "XRE.h" header which includes a bunch of subheaders at some point.
Comment on attachment 203292 [details] [diff] [review] Build a little libxul with ffox dynamic and xulrunner --disable-libxul, rev. 2 r=darin
Attachment #203292 - Flags: first-review?(darin) → first-review+
Fixed on trunk, with a couple bustage fixes (-lXUL doesn't work on mac, you have to specify the file explicitly).
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Broke build with following :- In file included from e:/mozilla/source/mozilla/embedding/tests/winEmbed/winEmbe d.cpp:46: ../../../dist/include/xulapp/nsXULAppAPI.h:182: error: ISO C++ forbids declarati on of `XRE_mainType' with no type ../../../dist/include/xulapp/nsXULAppAPI.h:189: error: ISO C++ forbids declarati on of `XRE_GetFileFromPathType' with no type ../../../dist/include/xulapp/nsXULAppAPI.h:198: error: ISO C++ forbids declarati on of `XRE_GetBinaryPathType' with no type ../../../dist/include/xulapp/nsXULAppAPI.h:204: error: ISO C++ forbids declarati on of `XRE_GetStaticComponentsType' with no type ../../../dist/include/xulapp/nsXULAppAPI.h:233: error: ISO C++ forbids declarati on of `XRE_InitEmbeddingType' with no type ../../../dist/include/xulapp/nsXULAppAPI.h:240: error: ISO C++ forbids declarati on of `XRE_TermEmbeddingType' with no type I think some extra commas need to be sorted out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
David, what build environment are you using? I have built this patch successfully.
MingW/Cygwin/Win32 However, the error seems to be indicating that INVALID C++ code is being used. I'm rather concerned that your compiler didn't notice this invalid code.
Could you point out *how* it's invalid c++? http://lxr.mozilla.org/seamonkey/source/toolkit/xre/xrecore.h#48
I think you are looking at the wrong file. http://lxr.mozilla.org/seamonkey/source/toolkit/xre/nsXULAppAPI.h#182 Line 183 has the undefined use of XRE_main. And I really don't understand the logic of int,XRE_main, ?
David, the link I pointed to defines the XRE_API macro which is used to either declare this as a function (imported/exported) or as a function pointer (for use in the standalone situation).
I'm not up to speed with macros in c++. All I can tell you is that gcc 3.4.4 in complaining about invalid code due to declarations with no type.
Attached file Build errors on mingw (obsolete) —
Yes, this broke building on MingW, attaching the errors I get.
Attachment #204300 - Attachment is obsolete: true
Attachment #204335 - Flags: first-review?(benjamin)
Comment on attachment 204335 [details] [diff] [review] Add missing type declaration Doh.
Attachment #204335 - Flags: first-review?(benjamin) → first-review+
The mingw fix has been checked in.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
*** Bug 311055 has been marked as a duplicate of this bug. ***
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: