Closed
Bug 261232
Opened 20 years ago
Closed 20 years ago
Require builders to specify application(s) to client.mk and configure when building
Categories
(SeaMonkey :: Build Config, enhancement)
SeaMonkey
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
cls
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
Since Firefox has become a mass-market phenomenon, we have lots more people
trying to build it. The unix-familiar ones download the source, run configure,
and expect it to "just work", and it doesn't because of our slightly wacky
MOZ_PHOENIX build system and the configure options in browser/config/mozconfig.
So I want to require builders to specify an application to build
--enable-application=suite|browser|mail|composer|calendar|xulrunner
I would prefer to use generic (non-branded) names instead of
firefox/thunderbird/nvu/sunbird in case the brand names change (again). This
would also allow me to get rid of the app-specific mozconfig and do some basic
sanity-checking of the build options people are trying to use.
Comment 1•20 years ago
|
||
sounds like a good idea to me.
Assignee | ||
Comment 2•20 years ago
|
||
Assignee | ||
Comment 3•20 years ago
|
||
Comment on attachment 161400 [details] [diff] [review]
part 1: Kick client.mk in the 'nads
This will of course require ample notification on the newsgroups and updating
tinderboxen, etc.
Attachment #161400 -
Flags: review?(cls)
Comment on attachment 161400 [details] [diff] [review]
part 1: Kick client.mk in the 'nads
Looks good but I have a few nits:
The directory name is 'libart_lgpl'. You used libart-lgpl in a few places.
If you're going to warn about MOZ_INTERNAL_LIBART_LGPL, you might as well just
make it a hard error. If you have MOZ_INTERNAL_LIBART_LGPL set but not
MOZ_CO_PROJECT or MOZ_CO_MODULE, it will process with the very limited pull
rather than spitting out the intended error.
You should probably have a similar error message when the other project
specific env variables are set (MOZ_PHOENIX, MOZ_THUNDERBIRD, etc).
Likewise when MOZ_CO_PROJECTS contains an unknown (or undefined) project.
IIRC, some shells try to interpret ab-DE as an arithmetic expression. The
MOZ_CO_LOCALES example should keep the quotes.
Attachment #161400 -
Flags: review?(cls) → review-
Assignee | ||
Comment 5•20 years ago
|
||
OK, to get rid of the MOZ_PHOENIX/MOZ_INTERNAL_* I can't do it incrementally,
so here's the entire shebang.
Attachment #161400 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #161799 -
Flags: review?(cls)
Comment 6•20 years ago
|
||
bsmedberg: correct me if I am wrong but on your updated patch, ("client.mk +
configure changes"), does not that cause Sunbird _not_ to build correctly...
MOZ_CALENDAR is no longer defined because you remove the --enable-calendar from
the sunbird mozconfig, and if --enable-calendar is not defined it over-rides
MOZ_CALENDAR from the app-selection...
(https://bugzilla.mozilla.org/attachment.cgi?id=161799&action=diff#mozilla/configure.in_sec5)
is where it would be undefined.
As a further nit (not my field but still a nit)
https://bugzilla.mozilla.org/attachment.cgi?id=161799&action=diff#mozilla/xpfe/bootstrap/nsAppRunner.cpp_sec1
Do we really want to hardcode MOZ_APP_NAME in nsAppRunner.cpp?
Thanks for listening (adding mostafah to CC since he is resident calendar owner)
Assignee | ||
Comment 7•20 years ago
|
||
Callek, you are thankfully wrong ;) the third argument to the
MOZ_ARG_ENABLE_BOOL macro is not used if you don't specify --enable-calendar,
it's used if you specify --disable-calendar; try it, you'll like it.
And yes, I did want to hardcode "mozilla" in xpfe/bootstrap/nsAppRunner.cpp...
nobody except seamonkey is be using this file, or should be.
Comment on attachment 161799 [details] [diff] [review]
client.mk + configure changes
> CYGWIN_WRAPPER=
> MOZ_USER_DIR=".mozilla"
>-MOZ_APP_NAME=mozilla
>+case "$MOZ_BUILD_APP" in
>+standalone)
>+ ;;
MOZ_APP_NAME should always be set. In the standalone case, it's not set and
that'll break the handful of places that use it in a standalone build (like
build/unix/Makefile.in & 'make install'). Same for MOZ_APP_VERSION.
> if test "$MOZ_SVG_RENDERER_LIBART"; then
>+ dnl libart's configure hasn't been run yet, but we know what the
>+ dnl answer should be anyway
>+ MOZ_LIBART_CFLAGS='-I${DIST}/include/libart_lgpl'
>+ case "$target_os" in
>+ msvc*|mks*|cygwin*|mingw*)
>+ MOZ_LIBART_LIBS='$(DIST)/lib/$(LIB_PREFIX)moz_art_lgpl.$(IMPORT_LIB_SUFFIX)'
>+ ;;
>+ beos*)
>+ MOZ_LIBART_LIBS='-lmoz_art_lgpl -lroot -lbe'
>+ ;;
>+ *)
>+ MOZ_LIBART_LIBS='-lmoz_art_lgpl -lm'
>+ ;;
>+ esac
>+ AC_FUNC_ALLOCA
I'm assuming that we're still not pulling libart by default for historical
licensing reasons. Since we're not pulling it by default, we need to
explicitly check for the directory here and make it clear on how to pull the
tree.
There are also MOZ_INTERNAL_LIBART_LGPL references in /minimo.mk & /Makefile.in
.
Attachment #161799 -
Flags: review?(cls) → review-
Assignee | ||
Comment 9•20 years ago
|
||
Attachment #161799 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #166185 -
Flags: review?(cls)
Assignee | ||
Comment 10•20 years ago
|
||
Chris, I am not going to mess with minimo.mk since it is a bad fork of client.mk
and I don't actually break it (it still works the way it always did).
Comment 11•20 years ago
|
||
FWIW, the minimo-arm tinderbox doesn't even pull minimo.mk. That may be a
better way to build minimo, but it isn't required. Maybe ask dougt ;-)
Comment 12•20 years ago
|
||
please also update the standalone xpcom build instructions at
http://www.mozilla.org/projects/xpcom/xpcom-standalone.html - thanks!
Comment 13•20 years ago
|
||
Comment on attachment 166185 [details] [diff] [review]
client.mk + configure changes, rev 2.1
+ if ! -f $topsrcdir/other-licenses/libart_lgpl/Makefile.in; then
That should be 'if test !'
And you still missed the MOZ_LIBART_INTERNAL_LGPL ifdef in mozilla/Makefile.in
which causes the build to fail in layout/svg .
Fix those && r=cls
Attachment #166185 -
Flags: review?(cls) → review+
Comment 14•20 years ago
|
||
Comment on attachment 166185 [details] [diff] [review]
client.mk + configure changes, rev 2.1
One little question:
>+composer)
>+ MOZ_APP_NAME=nvu
Have we actually landed N|Vu source code in CVS? There may be an unintended
slight to Disruptive Innovations or nvudev.org if we use their app name for
Mozilla code (not code ported back from N|vu).
Assignee | ||
Comment 15•20 years ago
|
||
AJ,
1) NVU is already there, I just moved the definition around
2) glazou and I work together pretty well, I think he would let me know if there
were problems.
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 16•20 years ago
|
||
I take it that this patch fixes the toolkit checkout for standalone apps, too?
Bug 253270.
Comment 17•20 years ago
|
||
Making MOZ_MAPINFO cause an error seems like a bad idea, since client.mk wasn't
the only thing that was checking the environment variable. It also wasn't a
good thing to do without updating the tinderboxes *first*.
Comment 18•20 years ago
|
||
note that it's the tinderbox script that sets MOZ_MAPINFO:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox/build-seamonkey-util.pl#403
Assignee | ||
Comment 19•20 years ago
|
||
I did go through the tinderboxen mozconfig, I didn't realize that MOZ_MAPINFO
was being set from the tinderbox script directly. I can make it a warning
temporarily until the tinderbox scripts are updated.
Comment 20•20 years ago
|
||
Have all the tinderboxes that are setting MOZ_CO_MODULE (with SeaMonkeyAll in
it), MOZ_INTERAL_LIBART_LGPL, etc., been updated? If not, this should be backed
out until they are.
I updated the tinderbox script to pull client.mk every time.
Comment 21•20 years ago
|
||
balsa is still setting MOZ_CO_MODULE the old way in both its SeaMonkey-Ports and
Firefox trees. So is brad. There are probably some others floating around doing
the same thing. (I like the semantics change, though, and I guess that one's
probably not too harmful, since it's one command line.)
Aren't we going to run into tinderbox problems for the tinderboxes that build
SVG as well?
Comment 22•20 years ago
|
||
And dare I say that this should probably not have gone in the first day after
the tree reopened when everybody else wanted to get things in?
Comment 23•20 years ago
|
||
I've also updated build-seamonkey-util.pl to set MOZ_CO_MODULE instead of
MOZ_MAPINFO.
I've updated:
balsa (script and config)
luna (script)
myotonic (script)
Comment 24•20 years ago
|
||
I've updated:
monkeypox (scripts and config)
I may do some more later...
Comment 25•20 years ago
|
||
Updated:
myotonic (mozconfig)
bismark (mozconfig and scripts)
these two weren't even updated with the two lines to build the suite. That says
to me that this wasn't ready to land, so I'm stopping now.
Comment 26•20 years ago
|
||
Fixed: speedracer worms otaku binus imola boxset gruff
Next time you do this (land something requiring tinderbox changes before the
tinderboxes are updated) I'm going to back you out. I really should have done
that today.
Comment 27•20 years ago
|
||
(The reason I didn't do that today was actually that you broke things badly
enough that backing you out wouldn't have been enough to fix the bustage.)
Comment 28•20 years ago
|
||
(The reason I didn't do that today was actually that you broke things badly
enough that backing you out wouldn't have been enough to fix the bustage,
although I've now fixed the tinderbox scripts so that it would have been.)
Comment 29•20 years ago
|
||
Sorry for """spamming""" the bug but it looks like there is some CVS checkout
problems...
I grabbed using CVS a full blank new source adding in my .mozconfig these options :
mk_add_options MOZ_CO_PROJECT=suite
ac_add_options --enable-application=suite
Interesting part of cvsco.log :
"checkout start: mer nov 24 11:22:22 CET 2004
[...]
cvs checkout: warning: new-born mozilla/.cvsignore has disappeared
U mozilla/README/mozilla/README.build
cvs checkout: warning: new-born mozilla/README.txt has disappeared
cvs checkout: warning: new-born mozilla/client.mak has disappeared
cvs checkout: warning: new-born mozilla/configure has disappeared
cvs checkout: warning: new-born mozilla/configure.in has disappeared
cvs checkout: warning: new-born mozilla/allmakefiles.sh has disappeared
[...]
checkout finish: mer nov 24 11:34:13 CET 2004"
A work around is to use an older tarball and update it with CVS.
Sorry if this bug was already reported !
Assignee | ||
Comment 30•20 years ago
|
||
This is apparently needed to work around bugs in some versions of the "cvs"
client, which cannot check out a Module and a directory-module at the same time
without getting confused.
Assignee | ||
Comment 31•20 years ago
|
||
Comment on attachment 166927 [details] [diff] [review]
Workaround "cvs" bugs
Asking chase for review, but cls or other cvs-knowers are welcome to steal the
review.
Attachment #166927 -
Flags: review?(cmp)
Comment 32•20 years ago
|
||
Sorry to "spam" this bug.
I applied this patch (patch -p0 < fix-newborn-cvs.patch), then I do a make -f
client.mk checkout to grab a blank new source.
And files in comment #29 are still missing :[
My CVS version is 1.11.17, under Fedora Core 3.
Comment 33•20 years ago
|
||
Re comment #29 -- I'm getting the same on FreeBSD, yesterday and today. The
build eventually dies because it can't find configure.
Comment 34•20 years ago
|
||
My CVS version is 1.11.2.1 under FreeBSD 5.0. I also tried
version 1.11.17 on a Fedora/Linux platform and 1.12.9 under SunOS 5.9
and all exhibited the same behaviour, new-born file "configure.in" disappeared.
Comment 35•20 years ago
|
||
Same result with Cygwin cvs 1.11.17 on Win32. The .../mozilla/ folder is empty
of all content except for client.mk and subfolders.
Assignee | ||
Comment 36•20 years ago
|
||
This fixes the problems with "new-born" files during initial checkout. It turns
out that there was a sticky tag leftover from the LDAP checkout step which was
not properly cleared when checking out the SeaMonkeyAll module. This patch
reorders the checkout steps so that the "directory" checkouts
(mozilla/accessible etc) are performed before SeaMonkeyAll. It was reviewed by
cmp and has been committed.
Attachment #166927 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #166927 -
Flags: review?(cmp)
Assignee | ||
Comment 37•20 years ago
|
||
Marking fixed. If you have further problems please file followup bugs.
Severity: minor → enhancement
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 38•20 years ago
|
||
Checkout from trunk is working OK now, thanks (FreeBSD, CVS 1.11.2.1)
Status: RESOLVED → VERIFIED
Comment 39•20 years ago
|
||
So, um, with the current code if you simply type "make -f client.mk pull_all",
as you used to be able to in the good ol' days, we happily start pulling but we
only pull about 4 targets and then stop w/o any error or anything. That means
that those who pull just client.mk and use pull_all to pull the tree are left
with only a fraction of the source code and no error message what so ever.
Seems really wrong to be.
And shouldn't we have a good default (be it SeaMonkey or Firefox) that would be
pulled if you just pull w/o going through the trouble of creating a .mozconfig
file. Personally I've never had one in a source tree, I always create them in my
obj-dirs in which I build, and I've got lots of those for a given source tree.
Can we not add make targets for pulling our different apps in stead of requiring
creation of .mozconfig files?
/me is not at all happy with this so far :(
Comment 40•20 years ago
|
||
If one now follows the steps on http://www.mozilla.org/cvs.html one gets a
mozilla tree with the following directories:
build/ client.mk CVS/ directory/ nsprpub/ security/
and no error, nothing.
Reopening, there's clearly more to be done here, and more docs need to be updated.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 41•20 years ago
|
||
Not-reporting the error is a regression from the last patch here, which added an
extra space to the module variable. That's now fixed, as is cvs.html.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Comment 42•20 years ago
|
||
cvs.html is not fixed. Yes, it's changed but now the checkout command is
described like this:
make -f client.mk checkout MOZ_CO_PROJECT=<option>
The cryptic reference to an <option> without explaining whatsoever what the
options might be is certain to confuse 90% of users (the remaining 10% are the
inhabitants of this bug and some Google Masters who will find the relevant BS
blog entry). Maybe some of the text from
http://www.saintpatrickdc.org/bsmedberg/index.php should be copied to cvs.html?
Comment 43•20 years ago
|
||
These documentation pages also need to be updated :-
http://www.mozilla.org/build/win32.html
http://www.mozilla.org/build/mac.html
They are accessed from http://www.mozilla.org/download-mozilla.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 44•20 years ago
|
||
cvs.html *is* fixed: Since several different applications are built from the
same basic source code, you must choose which application to checkout on the
command line using the MOZ_CO_PROJECT variable. The possible options are:
suite
The traditional "Mozilla" seamonkey suite of browser, mail/news, composer,
and other applications.
browser
The standalone "Firefox" browser.
mail
The standalone "Thunderbird" mail/news client.
composer
The standalone HTML composer
calendar
The standalone "Sunbird" calendar app.
xulrunner
The next-generation XUL application launcher.
macbrowser
The "Camino" native browser for Macintosh.
win32.html does not need to be updated, as it does not contain any checkout
instructions at all. And as I said, please file followup bugs, instead of
reopening this one endlessly.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Comment 45•20 years ago
|
||
Benjamin, do we live in alternate universes? I do not see this very info on
cvs.html even as it is necessary to know how to pull any mozilla.org product.
You cannot mention an "<option>" without listing the actual possible values.
I am a scholar and I can assure you that if cvs.html were a scientific paper you
would not get any reviewer to allow publishing it as it is.
Assignee | ||
Comment 46•20 years ago
|
||
Jacek, we must live in alternate universes, it's in the fifth paragraph.
Comment 47•20 years ago
|
||
I think the misunderstanding is that the options are listed in the fifth
paragraph, but when you use the <option> notation a few paragraphs later, you
don't say that it is one of the options above. Yes, it is the same command as
above, but you have forgotten that once you are at the detailed instructions.
Comment 48•20 years ago
|
||
OK. I see it now. But I would not call that user friendly. It's not easy to see
the connection between the windy bullet list above and the <option> mentioned
below, especially when you generally know what you're doing but look only for
changes in the procedure.
Comment 49•19 years ago
|
||
Comment on attachment 166185 [details] [diff] [review]
client.mk + configure changes, rev 2.1
>Index: xpfe/bootstrap/nsAppRunner.cpp
>@@ -148,12 +148,14 @@
>+#define MOZ_APP_NAME "mozilla"
erm. there are companies that rebrand mozilla.exe, and seamonkey changed the name of the app too...
Comment 50•19 years ago
|
||
That one was removed again as part of Bug 285696 (suite rebranding).
You need to log in
before you can comment on or make changes to this bug.
Description
•