Closed
Bug 870565
Opened 12 years ago
Closed 11 years ago
Run all the moz.build files in comm-central from mozilla-central's config.status
Categories
(MailNews Core :: Build Config, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 24.0
People
(Reporter: jcranmer, Assigned: jcranmer)
References
Details
Attachments
(2 files, 1 obsolete file)
2.22 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
7.81 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Perhaps the only bug I've filed where it takes more effort trying to figure out how to describe it than how to fix it?
Right now, the build system of comm-central is an ersatz 3-way build system, depending on which config.status is invoked and which rules.mk is used. The plan here is to reduce one of the complexities down to a 2-way build system, depending only on the rules.mk that gets used.
In effect, the current situation gives us two "global" build files which aren't really global, which requires comm-central to hack on stuff (like xpcshell manifests) to get the global state back again. Right now, that's not a major issue, but it stands to reason that bugs like the parallelize EXPORTS or xpidl bugs, which move build logic into nonrecursive logic structures, are liable to maybe-subtly break comm-central. It turns out that the largest portion of our code (ldap, comm-central) get run from the mozilla-central directory, to link with libxul.so, but the smaller parts (suite, mail, chat, lightning) don't.
With the patches in this bug (to be uploaded shortly), all of the code runs from mozilla-central's mozbuild, leaving the only purpose of the comm-central version to make the autoconf.mk for comm-central. Judging from some WIP patches for other bugs I'm working on, we might still end up with useless extra files getting plopped in random places in the c-c objdir (e.g., an empty testing/xpcshell/xpcshell.ini file). Since this makes the various mozbuild-is-global-info patches much safer, I'm listing them as depending on this bug.
Assignee | ||
Comment 1•12 years ago
|
||
At this point, the comm-central config.status makes nothing [but we still need the empty moz.build in the root, I think], and all of tier_app is invoked by mozilla-central.
Attachment #747633 -
Flags: feedback?(mbanner)
Assignee | ||
Comment 2•12 years ago
|
||
Without this change, we'll use the config.status that is effectively a nop to rebuild the backend.mk if we change a moz.build file. Actually, what's currently in the tree is kind of right and kind of wrong, but this is right for everybody but c-c/Makefile.in.
Assignee | ||
Updated•12 years ago
|
Attachment #747641 -
Flags: review?(mbanner)
Assignee | ||
Comment 3•12 years ago
|
||
I hate tiers. It turns out that mozbuild invokes tiers in the order they're specified in the moz.build files. So, part 1, as it stands, causes tier_app to build before tier_platform, with all of the problems that causes, if you attempt to build with incomplete external linkage. The easy solution is to just always build the stuff in bridge/bridge.mozbuild in tier_platform, even in incomplete external linkage...
Comment 4•12 years ago
|
||
Can't we just include('/toolkit/toolkit.mozbuild') first?
Comment 5•12 years ago
|
||
Comment on attachment 747633 [details] [diff] [review]
Part 1: Move the code to moz.build
Review of attachment 747633 [details] [diff] [review]:
-----------------------------------------------------------------
I think this looks reasonable, it seems like it should get rid of some of our older dependency bugs.
::: mail/app.mozbuild
@@ +6,5 @@
> app_libxul_dirs = []
> app_libxul_static_dirs = []
>
> +bridge_reldir = '../'
> +include('../bridge/bridge.mozbuild')
I think we can get rid of bridge with this patch, as we'd no longer need the reldir.
@@ +19,3 @@
>
> +if not CONFIG['LIBXUL_SDK']:
> + include('/toolkit/toolkit.mozbuild')
Like Neil says, this should move to before everything else, and still be fine in theory.
Attachment #747633 -
Flags: feedback?(mbanner) → feedback+
Comment 6•12 years ago
|
||
Comment on attachment 747641 [details] [diff] [review]
Part 2: Change the config.status
Looks fine, though I've not tested it (pending completion of part 1).
Attachment #747641 -
Flags: review?(mbanner) → review+
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #5)
> I think we can get rid of bridge with this patch, as we'd no longer need the
> reldir.
For now, I'm keeping it with some minor cleanup to be a single point of reference for everything we need to build mailnews (ldap/sdks/c-sdk, ldap/xpcom, mozilla/xpfe/components/autocomplete, db/, mailnews/). That said, moving bridge/bridge.mozbuild to mailnews/mailnews.mozbuild would better reflect its purpose in these new patches.
> @@ +19,3 @@
> >
> > +if not CONFIG['LIBXUL_SDK']:
> > + include('/toolkit/toolkit.mozbuild')
>
> Like Neil says, this should move to before everything else, and still be
> fine in theory.
We need to have app_libxul_* defined before including this file.
Comment 8•12 years ago
|
||
Comment on attachment 747633 [details] [diff] [review]
Part 1: Move the code to moz.build
>+bridge_reldir = '../'
>+include('../bridge/bridge.mozbuild')
>
>+if CONFIG['MOZ_INCOMPLETE_EXTERNAL_LINKAGE']:
>+ add_tier_dir('app', app_libxul_static_dirs, static=True)
>+ add_tier_dir('app', app_libxul_dirs)
>
>+if not CONFIG['LIBXUL_SDK']:
>+ include('/toolkit/toolkit.mozbuild')
>
>+if CONFIG['MOZ_EXTENSIONS']:
>+ add_tier_dir('app', 'extensions')
>
>+if CONFIG['MOZ_COMPOSER']:
>+ add_tier_dir('app', '../editor/ui')
>
>+add_tier_dir('app', CONFIG['MOZ_BRANDING_DIRECTORY'])
>
>+if CONFIG['MOZ_CALENDAR']:
>+ add_tier_dir('app', '../calendar/lightning')
>
>+add_tier_dir('app', '../suite')
I double-checked with the existing build system, which builds dirs (*if chosen) in this order:
1. platform
2. extensions*
3. external linkage*
4. composer*
5. branding
6. calendar*
7. suite
So external linkage actually needs to be moved down, if I understand correctly.
Assignee | ||
Comment 9•12 years ago
|
||
This cleans up some of the conditionals a bit--I suspect the splitting on the MOZ_APP_COMPONENT_LIBS bit was originally started because it didn't pick up the c-c configure bits; now that we have those available, the rules can be simpler.
Attachment #747633 -
Attachment is obsolete: true
Attachment #752446 -
Flags: review?(mbanner)
Assignee | ||
Comment 10•12 years ago
|
||
whoops, I don't know how that :q! got into suite/app.mozbuild; I've removed that locally.
The backend.mk files for mozilla "look right" for TB, TB external, and SM builds, and TB external fails only on some link error, so I'll call these working.
Comment 11•11 years ago
|
||
Comment on attachment 752446 [details] [diff] [review]
Part 1: Move the code to moz.build
Review of attachment 752446 [details] [diff] [review]:
-----------------------------------------------------------------
Ok, this looks fine to me with the :q! removal. r=Standard8
Attachment #752446 -
Flags: review?(mbanner) → review+
Assignee | ||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/da699b85afb9
https://hg.mozilla.org/comm-central/rev/289eb0e1471a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
You need to log in
before you can comment on or make changes to this bug.
Description
•