Closed
Bug 299997
Opened 19 years ago
Closed 19 years ago
Build Firefox --with-libxul-sdk=/path
Categories
(Firefox Build System :: General, defect, P2)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla3
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
Attachments
(2 files)
(deleted),
patch
|
darin.moz
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/plain
|
Details |
Very early in 1.9 I want to add a build option that builds Firefox (and any
other xulapps/extensions that wants to come along for the ride) on top of
xulrunner in a separate objdir. They would be built with --with-libxul-sdk=/path
client.mk would be taught to build xulrunner+firefox all in one pass
This will not be the default build configuration until some point in the future
when the installer code is up and running and we can actually deliver usable
nightly builds with an installer in this configuration.
Assignee | ||
Updated•19 years ago
|
Priority: -- → P2
Target Milestone: --- → Future
Assignee | ||
Updated•19 years ago
|
Target Milestone: Future → Firefox 3
Assignee | ||
Comment 1•19 years ago
|
||
Turns out this patch isn't as scary to read as I thought it was going to be. It can basically be divided into a few parts:
The configuration flag --with-libxul-sdk tells firefox to build on top of an existing XULRunner build. It sets the MOZ_SDK makefile variable.
Linking against the XPCOM and other various other gecko libs is controlled with a new variable GECKO_DIST. This variable will either be MOZ_SDK if that's defined, or DIST if not.
REQUIRES includes include/module from both the current appdir and the SDK. Same thing for xpidl, look for IDLs in both places.
The root makefile is hacked to only build tier 0 (configuration junk like nsinstall) and tier 99 (app-specific code) when MOZ_SDK is set.
The PREF_JS_EXPORTS changes are the result of a "feature" in gmake and the way we use $(DEPTH)... the dependency that was supposed to create the "firefox" defaults/pref was finding the "xulrunner" defaults/pref. I changed it to do the creation inline in the pref rule itself.
And the browser/app changes build the application.ini for the XULRunner case instead of the binary code.
simple ;-)
Attachment #209892 -
Flags: review?(darin)
Assignee | ||
Comment 2•19 years ago
|
||
This .mozconfig builds xulrunner + ffox in one pass.
Assignee | ||
Comment 3•19 years ago
|
||
cls, I'd appreciate any review you can give here also.
Comment 4•19 years ago
|
||
From reading the comment #1 alone, I wonder why MOZ_SDK and GECKO_DIST do not share the same prefix, namely GECKO_. Is that because GECKO_SDK is already defined to mean something else (i.e., dist/sdk/ only)?
Review comments to follow...
Comment 5•19 years ago
|
||
Comment on attachment 209892 [details] [diff] [review]
The Great Dalmuti, rev. 1
>Index: configure.in
>+MOZ_ARG_WITH_STRING(libxul-sdk,
>+[ --with-libxul-sdk=PFX Use the XULRunner SDK at <PFX>],
>+ MOZ_SDK_DIR=$withval)
Change this to "Use the LibXUL SDK..."
Perhaps the variable should be called "MOZ_LIBXUL_SDK" instead?
Or, are you wishing to refer to the uber-sdk as the mozilla sdk?
If so, perhaps we should change the configure argument to
--with-mozilla-sdk :-)
>+if test "$MOZ_SDK_DIR" = "yes"; then
>+ AC_MSG_ERROR([--with-libxul-sdk must specify a path])
>+elif test -n "$MOZ_SDK_DIR" -a "$MOZ_SDK_DIR" != "no"; then
>+ MOZ_SDK=`cd "$MOZ_SDK_DIR" && pwd`
>+
>+ if ! test -f "$MOZ_SDK/sdk/include/xpcom-config.h"; then
>+ AC_MSG_ERROR([$MOZ_SDK/sdk/include/xpcom-config.h doesn't exist])
>+ fi
>+
>+ if "$MOZ_BUILD_APP" = "xulrunner"; then
>+ AC_MSG_ERROR([Building XULRunner --with-libxul-sdk doesn't make sense; XULRunner *is* the libxul SDK.])
"XULRunner *is part of* the libxul SDK."
>Index: Makefile.in
>+ifndef MOZ_SDK
>+# only build gecko if --with-libxul-sdk isn't passed
Here we have three terms all referring to the same thing. It makes
reading this code confusing to noobies I bet. Is there anything you
can do to reduce this to one term?
Also, it might be nice if the variable was prefixed with HAVE_ or
something like that to self-document the fact that we don't need to
build the "something" because we already have that something.
>Index: browser/app/Makefile.in
>+# GRE_BUILD_ID is only available in nsBuildID.h in a form that we can't use
>+# directly. So munge it. Beware makefile and shell escaping
>+AWK_EXPR = '/\#define GRE_BUILD_ID/ { print gensub(/"/, "", "g", $$3) }'
>+AWK_CMD = awk $(AWK_EXPR) < $(DEPTH)/config/nsBuildID.h
>+
>+GRE_BUILD_ID = $(shell $(AWK_CMD))
That's great fun. We should generate a file in config/ that has
the BuildID in a more friendly format so this won't be necessary
here or elsewhere (e.g., mail/app).
>Index: browser/app/application.ini
>+#filter substitution
>+[App]
>+Vendor=Mozilla
>+Name=Firefox
>+Version=@APP_VERSION@
>+BuildID=@BUILD_ID@
>+Copyright=Copyright (c) 1998 - 2006 mozilla.org
>+ID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}
>+
>+[Gecko]
>+MinVersion=@GRE_BUILD_ID@
>+MaxVersion=@GRE_BUILD_ID@
This should define EnableProfileMigrator and EnableExtensionManager.
Unlike simpler XUL apps, we don't want to disable those features ;-)
Otherwise, this looks great. r=darin
Attachment #209892 -
Flags: review?(darin) → review+
Assignee | ||
Comment 6•19 years ago
|
||
> Perhaps the variable should be called "MOZ_LIBXUL_SDK" instead?
> Or, are you wishing to refer to the uber-sdk as the mozilla sdk?
> >+ifndef MOZ_SDK
> >+# only build gecko if --with-libxul-sdk isn't passed
>
> Here we have three terms all referring to the same thing. It makes
> reading this code confusing to noobies I bet. Is there anything you
> can do to reduce this to one term?
I have changed the variables to:
LIBXUL_DIST (always present)
LIBXUL_SDK (only present in --with-libxul-sdk=foo)
I don't want to use a HAVE_ variable because HAVE_ variables are by convention "1" or empty, they don't point to a directory.
Assignee | ||
Comment 7•19 years ago
|
||
Fixed on trunk. I'm going to get some additional build automation in place to package up xulrunner+firefox in a tarball of sorts before requesting that the build team set up tinderboxes to track this configuration.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 8•19 years ago
|
||
> I have changed the variables to:
>
> LIBXUL_DIST (always present)
> LIBXUL_SDK (only present in --with-libxul-sdk=foo)
>
> I don't want to use a HAVE_ variable because HAVE_ variables are by convention
> "1" or empty, they don't point to a directory.
OK
Assignee | ||
Comment 9•19 years ago
|
||
Turns out this doesn't build on windows, not because of the build-config changes but because of various dependencies of inspector and the win32-specific embedding goop. I'll deal with that in separate bugs.
Comment 10•19 years ago
|
||
This seems breaks Solaris boxes:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1138503943.3387.gz
Please take a look.
Comment 11•19 years ago
|
||
+AWK_EXPR = '/\#define GRE_BUILD_ID/ { print gensub(/"/, "", "g", $$3) }'
A small note:
the gensub function is only available with gnu awk. So it does not work on Ubuntu which comes with mawk by default for instance. (the configure script accepts both gnu awk or mawk).
Assignee | ||
Comment 12•19 years ago
|
||
Sylvian, my portable-awk knowledge is poor, can you suggest an alternate awk command that has the same effect?
Comment 13•19 years ago
|
||
(In reply to comment #12)
> Sylvian, my portable-awk knowledge is poor, can you suggest an alternate awk
> command that has the same effect?
AWK_EXPR = '/\#define GRE_BUILD_ID/ { gsub(/"/, "", $$3); print $$3 }'
Comment 14•19 years ago
|
||
(In reply to comment #13)
> AWK_EXPR = '/\#define GRE_BUILD_ID/ { gsub(/"/, "", $$3); print $$3 }'
Good catch ;-)
BTW, this may be useful: http://www.guides.sk/shell/goodcoding/awkcompat.html
Assignee | ||
Comment 15•19 years ago
|
||
portable awk fix committed.
Comment 16•19 years ago
|
||
(In reply to comment #15)
> portable awk fix committed.
gsub third arg "g" should be dropped too.
Comment 17•19 years ago
|
||
+ if ! test -f "$MOZ_SDK/sdk/include/xpcom-config.h"; then
+ AC_MSG_ERROR([$MOZ_SDK/sdk/include/xpcom-config.h doesn't exist])
+ fi
This file is not likely to exist on hosts where xulrunner has been installed, in which case includes are in /usr/include/xulrunner-x.y (and libs in /usr/lib/xulrunner-x.y)
Moreover, you provide several .pc files. Why not just use pkg-config to get the includes for the xulrunner-xpcom module ?
Assignee | ||
Comment 18•19 years ago
|
||
We are planning two distribution methods: XULRunner itself and XULRunner + XUL Developer Kit. The XUL Developer Kit will be structured much like the existing dist/* structure in-tree. There will not be separate /usr/lib/xulrunner-* and /usr/include/xulrunner-* structures when installed into a linux system.
Updated•6 years ago
|
Component: Build Config → General
Product: Firefox → Firefox Build System
Updated•6 years ago
|
Target Milestone: Firefox 3 → mozilla3
You need to log in
before you can comment on or make changes to this bug.
Description
•