Closed
Bug 696498
Opened 13 years ago
Closed 13 years ago
Clean up *makefiles.sh, add omitted makefiles & use more conditionals
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla11
People
(Reporter: emorley, Assigned: emorley)
References
Details
(Whiteboard: [buildfaster:?])
Attachments
(10 files)
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
Bug 689043, bug 689040 & bug 689036 cleaned up some of the smaller makefile generation scripts in the tree; this bug covers most of the rest (allmakefiles.sh, browser/makefiles.sh, mobile/makefiles.sh, toolkit/toolkit-makefiles.sh).
I've created a script that scans the in-tree makefiles and compares the result with those already included in the generation scripts, producing a list of those that:
a) have been omitted
b) are no longer in the tree and can be removed (eg those under a conditional and so don't appear in the standard configure warning for makefile not found)
c) appear more than once
This bug will deal with cases A+B+C as well as add further conditionals (OS-specific, enable tests, feature based; within reason) to reduce the number of unnecessarily generated makefiles on each platform.
For some context, a cursory examination using a draft version of the script, found ~250 makefiles missing from the generation scripts (which as a proportion of the 1143 makefiles in the tree, is quite high). I also believe there to be 150-200 unnecessarily created makefiles in my local obj-dir, due to missing conditionals.
I don't know how much difference each of the above makes to build times (until this patch series is finished at least), but I guess every little helps.
Assignee | ||
Comment 1•13 years ago
|
||
Script combines the Makefiles listed in *makefiles.sh + NSPR configure, and compares to those present in the tree after applying a whitelist. A list of invalid (ie since been removed from the tree), omitted (from the generation scripts, so need to be added) and duplicate Makefiles is saved to makefiles-generated-*.txt in topsrcdir. See inline comments for more info.
Assignee | ||
Comment 2•13 years ago
|
||
The output from the script, combined into one file for easier attachment.
Summary:
* 26 invalid
* 6 duplicate
* 235 omitted
Assignee | ||
Comment 3•13 years ago
|
||
Clean up allmakefiles.sh:
* Adds 11 omitted makefiles, under appropriate conditionals
* Moves some of the existing makefile entries to platform/compiler conditionals
Attachment #568988 -
Flags: review?(khuey)
Assignee | ||
Comment 4•13 years ago
|
||
Clean up browser/makefiles.sh:
* Adds 21 omitted makefiles, under appropriate conditionals
* Moves some of the existing makefile entries to platform/feature conditionals
Attachment #568989 -
Flags: review?(khuey)
Assignee | ||
Comment 5•13 years ago
|
||
Clean up mobile/makefiles.sh:
* Removes various non-mobile Makefile entries, since they already exist in
toolkit-makefiles.sh (which is always run unless --with-libxul-sdk used)
* Adds missing $MOZ_BRANDING_DIRECTORY/content/Makefile
* Moves mobile/chrome/tests/Makefile to ENABLE_TESTS conditional
Attachment #568990 -
Flags: review?(khuey)
Attachment #568988 -
Flags: review?(khuey) → review+
Attachment #568989 -
Flags: review?(khuey) → review+
Attachment #568990 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #569054 -
Flags: review?(khuey)
Assignee | ||
Comment 7•13 years ago
|
||
* Remove duplicate |extensions/pref/Makefile| entry, since already present higher up in the file
* Remove l10n/* makefile entries since they no longer exist in the tree
Attachment #569058 -
Flags: review?(khuey)
Attachment #569054 -
Flags: review?(khuey) → review+
Attachment #569058 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 8•13 years ago
|
||
* Remove MAKEFILES_plugin since not assigned anything
* Rename MAKEFILES_libpr0n to MAKEFILES_imagelib, to match bug 66984
* Remove the following NPOTB makefiles:
parser/htmlparser/tests/logparse/Makefile
intl/unicharutil/tools/Makefile
xpcom/reflect/xptcall/src/md/test/Makefile
xpfe/components/autocomplete/Makefile
xpfe/components/autocomplete/public/Makefile
xpfe/components/autocomplete/src/Makefile
* Remove the following commented out makefiles:
parser/htmlparser/tests/outsinks/Makefile
xpcom/reflect/xptcall/tests/Makefile
xpcom/reflect/xptinfo/tests/Makefile
* Remove embedding/tests/winEmbed/Makefile since it belongs in xulrunner/makefiles.sh (will sort this out in a followup bug, since the logic seems squiffy [http://mxr.mozilla.org/mozilla-central/source/xulrunner/build.mk#48], so will sort out at the same time)
(Broken these changes out from the part D patch, to make it's diff a little bit more readable).
Attachment #571170 -
Flags: review?(khuey)
Assignee | ||
Comment 9•13 years ago
|
||
* Adds missing platform, feature and/or ENABLE_TESTS based conditionals to ~170
entries that were missing them and so sometimes being generated unnecessarily.
* Pre-emptively assumes RDF and SMIL are always built, since bug 597789 and
bug 698630 will land soon removing the option to disable them.
* Moves all conditionally included makefiles to the end of the file.
Please let me know your/ted's preference as to what order the tests vs platform vs feature sections should be in, as well as the ordering of each block inside the feature specific section. I didn't want to move things around too much in the initial pass, to try and keep the diff as readable as possible - so left the existing feature conditionals where they were, and added the new ones alphabetically above them.
(One final patch after this to come, which will be adding the ~200 omitted makefiles.)
Attachment #571173 -
Flags: review?(khuey)
Assignee | ||
Updated•13 years ago
|
Whiteboard: [buildfaster:?]
Attachment #571170 -
Flags: review?(khuey) → review+
Attachment #571173 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 10•13 years ago
|
||
Adds ~160 makefiles omitted from tookit-makefiles.sh, under conditionals where required.
Attachment #575338 -
Flags: review?(khuey)
Assignee | ||
Comment 11•13 years ago
|
||
Attachment #575338 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 12•13 years ago
|
||
Landed after rebasing plus:
- Adjusting android conditionals for recent gonk landings (OS_ARCH vs MOZ_WIDGET_TOOLKIT etc)
- Removed browser/themes/<theme>/browser/Makefile post bug 701212
- s_mobile/_mobile/xul_g
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbbe2010c4ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/79f0181b02f8
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c529c4c2e04
https://hg.mozilla.org/integration/mozilla-inbound/rev/c43c8775dd02
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7f5fbaafedd
https://hg.mozilla.org/integration/mozilla-inbound/rev/957949faf984
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c50a161c823
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4bdf970fe2b
Still a few things to clean up (eg new Makefiles landed in the tree since this patch series was reviewed, generated makefiles in js/src/configure and nsprpub/configure), but will deal with them elsewhere.
Assignee | ||
Updated•13 years ago
|
Flags: in-testsuite-
Target Milestone: --- → mozilla11
Comment 13•13 years ago
|
||
> https://hg.mozilla.org/integration/mozilla-inbound/rev/79f0181b02f8
This is causing the following compile error and a bunch of similar ones for me on 64-bit debug Linux builds:
In file included from /home/njn/moz/mi0/gfx/cairo/cairo/src/cairoint.h:70:0,
from /home/njn/moz/mi0/gfx/cairo/cairo/src/cairo-bentley-ottmann-rectilinear.c:39:
/home/njn/moz/mi0/gfx/cairo/cairo/src/cairo.h:42:28: fatal error: cairo-features.h: No such file or directory
compilation terminated.
I bisected to determine this. The same error also occurs in later revisions after all the other build system clean-ups landed.
I don't understand the patch, but it would be nice if I could build the browser again!
Assignee | ||
Comment 14•13 years ago
|
||
Ok, thanks to Nick emailing me the log, the above was tracked down to:
{
sed: can't read /home/njn/moz/mi3/gfx/cairo/cairo/src/cairo-features.h/Makefile.in: No such file or directory
...
rm: cannot remove `gfx/cairo/cairo/src/cairo-features.h': Is a directory
./config.status: 2800: cannot create gfx/cairo/cairo/src/cairo-features.h: Is a directory
...
creating }
sed: can't read /home/njn/moz/mi3/}.in: No such file or directory
}
Which implied that the braces in the |browser/app/profile/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/Makefile| entry added in 79f0181b02f8 causes (certain versions of) sed to choke, breaking make-makefiles thus meaning cairo-features.h isn't present later in the build.
For reference:
Local win32 build using MozillaBuild (sed 3.02): works
Builds on multiple platforms on try/mozilla-inbound (sed-4.1.5-5.fc6): works
njn's local build on linux64 (sed 4.2.1): doesn't work
Typically, only after all this, I spotted this changeset from 2009, having to revert exactly the same thing:
https://hg.mozilla.org/mozilla-central/rev/3441f3c51ae2
Anyway, I've removed the entry for the default theme's makefile again, and added a comment to ensure it doesn't happen a third time:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43e18ca6cc8c
Comment 15•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bbbe2010c4ba
https://hg.mozilla.org/mozilla-central/rev/79f0181b02f8
https://hg.mozilla.org/mozilla-central/rev/1c529c4c2e04
https://hg.mozilla.org/mozilla-central/rev/c43c8775dd02
https://hg.mozilla.org/mozilla-central/rev/d7f5fbaafedd
https://hg.mozilla.org/mozilla-central/rev/957949faf984
https://hg.mozilla.org/mozilla-central/rev/8c50a161c823
https://hg.mozilla.org/mozilla-central/rev/f4bdf970fe2b
https://hg.mozilla.org/mozilla-central/rev/43e18ca6cc8c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•13 years ago
|
||
Added a handful of makefiles that have been created in the tree since the patches in this bug were reviewed. Not attached/reviewed, since trivial additions have a standing rs=build.
https://hg.mozilla.org/integration/mozilla-inbound/rev/17e8e969fa09
Assignee | ||
Comment 17•13 years ago
|
||
(In reply to Ed Morley [:edmorley] from comment #16)
> Added a handful of makefiles that have been created in the tree since the
> patches in this bug were reviewed.
https://hg.mozilla.org/mozilla-central/rev/17e8e969fa09
Comment 18•7 years ago
|
||
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/6098be142a9f
Fix comm-central app bustage from bug 696498 - change the depth of the makefiles to account for being generated from the comm-central build system now that the old xpfe autocomplete files are no longer generated within the mozilla-central part of the build system. rs=Neil over irc. NPOTFFB DONTBUILD
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•