870565 - Run all the moz.build files in comm-central from mozilla-central's config.status

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Joshua Cranmer [:jcranmer]]

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.

Comment 1

[Joshua Cranmer [:jcranmer]]

Attachment: Part 1: Move the code to moz.build

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.

Comment 2

[Joshua Cranmer [:jcranmer]]

Attachment: Part 2: Change the config.status

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.

Comment 3

[Joshua Cranmer [:jcranmer]]

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

[[e-mail]]

Can't we just include('/toolkit/toolkit.mozbuild') first?

Comment 5

[Mark Banner (:standard8)]

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.

Comment 6

[Mark Banner (:standard8)]

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).

Comment 7

[Joshua Cranmer [:jcranmer]]

(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

[[e-mail]]

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.

Comment 9

[Joshua Cranmer [:jcranmer]]

Attachment: Part 1: Move the code to moz.build

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.

Comment 10

[Joshua Cranmer [:jcranmer]]

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

[Mark Banner (:standard8)]

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

Comment 12

[Joshua Cranmer [:jcranmer]]

https://hg.mozilla.org/comm-central/rev/da699b85afb9
https://hg.mozilla.org/comm-central/rev/289eb0e1471a