Last Comment Bug 845089 - Thunderbird build config changes in a moz.build world
: Thunderbird build config changes in a moz.build world
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 22.0
Assigned To: Gregory Szorc [:gps]
:
Mentors:
Depends on: 784841
Blocks: 846524
  Show dependency treegraph
 
Reported: 2013-02-25 15:22 PST by Gregory Szorc [:gps]
Modified: 2013-03-31 20:28 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
moz.build compatibility, v1 (145.14 KB, patch)
2013-02-25 21:29 PST, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review
moz.build compatibility, v2 (162.77 KB, patch)
2013-02-25 23:21 PST, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review
moz.build compatibility, v3 (166.17 KB, patch)
2013-02-26 17:44 PST, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review
moz.build compatibility, v4 (166.17 KB, patch)
2013-02-27 21:18 PST, Gregory Szorc [:gps]
bugspam.Callek: review+
Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2013-02-25 15:22:48 PST
I'll be providing patches to make comm-central work post bug 784841.
Comment 1 Gregory Szorc [:gps] 2013-02-25 21:29:08 PST
Created attachment 718241 [details] [diff] [review]
moz.build compatibility, v1

After looking at comm-central, the choices are either to hack up m-c's code to handle mixed Makefile.in and moz.build or to convert comm-central to moz.build files.

Given bug 787208, converting to moz.build will need to be done eventually. So, might as well do it right now instead of hacking up core build system code until comm-central is ready.

This is my initial pass at converting mostly just Makefile.in to moz.build. Next pass will include additional .mk files.

This is completely untested and really busted. Feel free to conduct drive-by reviews on the conversion of Makefile.in (but not the .mk or any other files).
Comment 2 Gregory Szorc [:gps] 2013-02-25 23:21:28 PST
Created attachment 718282 [details] [diff] [review]
moz.build compatibility, v2

Removed allmakefiles.sh and friends. Converted more build.mk files into app.mozbuild. mail app gets through configure and starts building. It's looking promising...

Requires a patch in bug 784841 to even come close to working.

Drive-bys on the mechanical conversion encouraged. build.mk files likely still in need of love. Only mail tested so far.
Comment 3 :Ms2ger 2013-02-26 02:33:34 PST
Comment on attachment 718282 [details] [diff] [review]
moz.build compatibility, v2

Review of attachment 718282 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/moz.build
@@ +4,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# This should match /suite/installer/moz.build.
> +if 'pre' in CONFIG['MOZ_APP_VERSION']:
> +    PARALLEL_DIRS += ['debugQA']

I think you want this if 'a' in ... too.
Comment 4 Justin Wood (:Callek) 2013-02-26 04:50:35 PST
Comment on attachment 718282 [details] [diff] [review]
moz.build compatibility, v2

Review of attachment 718282 [details] [diff] [review]:
-----------------------------------------------------------------

::: bridge/bridge.mk
@@ -1,1 @@
> -# This Source Code Form is subject to the terms of the Mozilla Public

As far as I can tell nothing uses the newly created bridge.mozbuild, this specific makefile was called from {mail,suite}/build.mk which is also invoked from the mozilla-central build system. The idea was to actually add these APP* tiers to the m-c build system so we can actually link them into libxul.

The caveat here is if nothing uses it, you likely won't get a build failure, however you'll be unable to run the app properly. (missing the mailnews related features, and possibly other completely broken things)
Comment 5 Joshua Cranmer [:jcranmer] 2013-02-26 11:34:25 PST
Comment on attachment 718282 [details] [diff] [review]
moz.build compatibility, v2

Review of attachment 718282 [details] [diff] [review]:
-----------------------------------------------------------------

Drive-by review
(and, wow, splinter sucks since it doesn't show fullpath info)

Also, I would be surprised if this worked without modifying our config.mk and rules.mk clones to pull in the new moz.build stuff.

::: bridge/bridge.mozbuild
@@ +2,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +app_libxul_dirs += ['/mozilla/xpfe/components/autocomplete']

I'm not convinced this is correct. I'm not an expert on the build system, but, IIRC, when we call bridge.mk, we are running under mozilla's build system, so the relative root from which these pseudo-absolute paths are being computed from is actually the mozilla-central root, not the comm-central root.

I *think* s+'/+'/../+g would fix you here, but I don't know for sure.

@@ +8,5 @@
> +component_libs = CONFIG['MOZ_APP_COMPONENT_LIBS'].split()
> +
> +if 'mozldap' not in component_libs and not CONFIG['MOZ_LDAP_XPCOM']:
> +    libxul_static_dirs += ['/ldap/sdks/c-sdk']
> +    libxul_dirs += ['/ldap/xpcom']

Shouldn't this be app_libxul_* ?

::: mail/app.mozbuild
@@ +6,5 @@
> +if not CONFIG['COMM_BUILD']:
> +    app_libxul_dirs = []
> +
> +    if CONFIG['MOZ_APP_COMPONENT_LIBS']:
> +        app_libxul_dirs += ['../mail/components']

You need to include the bridge.mk, at the very least.

@@ +10,5 @@
> +        app_libxul_dirs += ['../mail/components']
> +
> +    if not CONFIG['LIBXUL_SDK']:
> +        app_libxul_static_dirs = []
> +        include('/toolkit/toolkit.mozbuild')

Reading toolkit.mozbuild...

It looks like you
a) Assume app_libxul_dirs contains a single directory (add_tier_dir('platform', app_libxul_dirs))
b) Don't use app_libxul_static_dirs

@@ +13,5 @@
> +        app_libxul_static_dirs = []
> +        include('/toolkit/toolkit.mozbuild')
> +
> +    if CONFIG['MOZ_EXTENSIONS']:
> +        add_tier_dir('app', 'extensions')

What directory is this relative to? This should be m-c/extensions, for the record.

@@ +18,5 @@
> +
> +else:
> +    if CONFIG['MOZ_COMPOSER']:
> +        add_tier_dir('app', 'editor/ui')
> +

And we need the bridge.mk here as well, too, I think. But only if MOZ_INCOMPLETE_EXTERNAL_LINKAGE ?

::: mailnews/mime/cthandlers/moz.build
@@ +10,5 @@
> +
> +# pgpmime depends on glue.
> +DIRS += ['pgpmime']
> +
> +if not CONFIG['BUILD_SMIME']:

You probably need to add the following:
if 'MOZ_PSM' in CONFIG:
  CONFIG['BUILD_SMIME'] = 1

or at least do what you did in mailnews/extensions.
Comment 6 Gregory Szorc [:gps] 2013-02-26 17:44:06 PST
Created attachment 718785 [details] [diff] [review]
moz.build compatibility, v3

Getting closer. Still trying to track down a failure in ldap/xpcom:

make -C ../ldap/xpcom libs
nsLDAPProtocolModule.cpp
/usr/local/bin/ccache /usr/local/llvm/bin/clang++ -o nsLDAPProtocolModule.o -c  -fvisibility=hidden -DMOZ_PREF_EXTENSIONS -DMOZ_PSM -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DMOZ_THUNDERBIRD=1 -DNO_NSPR_10_SUPPORT  -I/Users/gps/src/comm-central/ldap/xpcom/src -I. -I../../../mozilla/dist/include  -I/Users/gps/src/comm-central/obj-thunderbird/mozilla/dist/include/nspr -I/Users/gps/src/comm-central/obj-thunderbird/mozilla/dist/include/nss      -fPIC -Qunused-arguments  -Qunused-arguments -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-invalid-offsetof -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -fcolor-diagnostics -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -DNO_X11 -pipe  -DNDEBUG -DTRIMMED -g -O3 -fomit-frame-pointer  -Qunused-arguments  -DMOZILLA_CLIENT -include ../../../mozilla/mozilla-config.h -MD -MF .deps/nsLDAPProtocolModule.o.pp  /Users/gps/src/comm-central/ldap/xpcom/src/nsLDAPProtocolModule.cpp
/Users/gps/src/comm-central/ldap/xpcom/src/nsLDAPProtocolModule.cpp:24:10: fatal error: 'ldappr.h' file not found
#include "ldappr.h"
         ^
1 error generated.
make[5]: *** [nsLDAPProtocolModule.o] Error 1
make[4]: *** [src_libs] Error 2
make[3]: *** [libs_tier_platform] Error 2
make[2]: *** [tier_platform] Error 2
make[1]: *** [default] Error 2
make: *** [default] Error 2

Looking in my obj directory: 

mozilla/dist/public/
mozilla/dist/public//ldap
mozilla/dist/public//ldap/disptmpl.h
mozilla/dist/public//ldap/iutil.h
mozilla/dist/public//ldap/lber.h
mozilla/dist/public//ldap/ldap-deprecated.h
mozilla/dist/public//ldap/ldap-extension.h
mozilla/dist/public//ldap/ldap-platform.h
mozilla/dist/public//ldap/ldap-standard.h
mozilla/dist/public//ldap/ldap-to-be-deprecated.h
mozilla/dist/public//ldap/ldap.h
mozilla/dist/public//ldap/ldap_ssl.h
mozilla/dist/public//ldap/ldappr.h
mozilla/dist/public//ldap/ldif.h
mozilla/dist/public//ldap/srchpref.h
mozilla/dist/public//ldap-private
mozilla/dist/public//ldap-private/lber-int.h
mozilla/dist/public//ldap-private/ldap-int.h
mozilla/dist/public//ldap-private/ldaplog.h
mozilla/dist/public//ldap-private/ldaprot.h
mozilla/dist/public//ldap-private/portable.h

I'm not sure why the -I is getting lost...
Comment 7 Joshua Cranmer [:jcranmer] 2013-02-26 21:04:51 PST
(In reply to Gregory Szorc [:gps] from comment #6)
> I'm not sure why the -I is getting lost...

The -I for this comes from the LDAP_CFLAGS variable, which is set by the c-c configure and gets slopped into mail's autoconf.mk. If you've managed to somehow make the ldap/xpcom directory use the m-c build system instead of the c-c build system, you wouldn't pick up the include.
Comment 8 Philipp Kewisch [:Fallen] 2013-02-26 23:50:15 PST
Please also try a build with ac_add_options --enable-calendar to make sure the calendar bits also work. I don't think we have anything funky, just maybe the libical bits.
Comment 9 :Ms2ger 2013-02-27 00:04:13 PST
(In reply to Joshua Cranmer [:jcranmer] (busy until March 8) from comment #5)
> @@ +10,5 @@
> > +        app_libxul_dirs += ['../mail/components']
> > +
> > +    if not CONFIG['LIBXUL_SDK']:
> > +        app_libxul_static_dirs = []
> > +        include('/toolkit/toolkit.mozbuild')
> 
> Reading toolkit.mozbuild...
> 
> It looks like you
> a) Assume app_libxul_dirs contains a single directory
> (add_tier_dir('platform', app_libxul_dirs))

add_tier_dir takes either a string or a list of strings.

> b) Don't use app_libxul_static_dirs

This is added in attachment 718280 [details] [diff] [review] in bug 784841.
Comment 10 Gregory Szorc [:gps] 2013-02-27 21:18:25 PST
Created attachment 719320 [details] [diff] [review]
moz.build compatibility, v4

Not sure who to hit for review, so tagging both Callek and jcranmer. Feel free to cancel or reassign!

I appear to have successfully built the mail app with this patch! I'll keep testing other apps and will upload new patches as required. I just figured I'd start off the review process with something that at least appears to work!

I imagine there's little things here and there that are broken. e.g. I haven't hooked up all the rules.mk changes yet. Ms2ger or someone else could likely port things from m-c if I don't get around it tonight :)

In my opinion, things are good enough to unblock merging the build system changes on m-c. If there are issues with c-c, I'm committed to being around all day Thursday to iron them out. I think fixing them is largely a matter of having people in an IRC room chatting about what broke and cranking out patches.

Please note you'll need the latest patch from bug 784841 applied on top of build-system sitting under /mozilla for this to work.
Comment 11 Mike Hommey [:glandium] 2013-02-27 23:50:06 PST
Comment on attachment 719320 [details] [diff] [review]
moz.build compatibility, v4

Review of attachment 719320 [details] [diff] [review]:
-----------------------------------------------------------------

Skimmed over it.

::: calendar/app.mozbuild
@@ +11,5 @@
> +    if not CONFIG['LIBXUL_SDK']:
> +        include('/toolkit/toolkit.mozbuild')
> +
> +    if CONFIG['MOZ_EXTENSIONS']:
> +        add_tier_dir('app', '/toolkit/extensions')

Leading / or not?
Comment 12 Justin Wood (:Callek) 2013-02-28 06:03:40 PST
Comment on attachment 719320 [details] [diff] [review]
moz.build compatibility, v4

Review of attachment 719320 [details] [diff] [review]:
-----------------------------------------------------------------

I admit most of this was a skim, but since its mostly boilerplate moving I'm not as concerned anymore after knowing you've tested.

Few very minor comments below.

::: calendar/moz.build
@@ +1,5 @@
> +# vim: set filetype=python:
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +

this corresponding Makefile looks pretty different than this moz.build (e.g. it had the tier_app related stuff and all that jazz for bridge.mk -- however Sunbird is deprecated so I won't block any landings on this fix, as long as you quickly check if "lightning" should be fine)

::: config/rules.mk
@@ -4,1 @@
>  # License, v. 2.0. If a copy of the MPL was not distributed with this file,

rs+ on rules.mk with a skim, I didn't give it much deep thought.

::: mail/app.mozbuild
@@ +24,5 @@
> +    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)

does add_tier_dir (and friends) work fine if the dirs are actually an array? or does it only properly accept a single dir?
Comment 14 Gregory Szorc [:gps] 2013-02-28 13:35:11 PST
Gonna reopen this until Windows builds turn green on TBPL.

I pushed a minor change to configure to normalize the path passed into mozilla's configure. I expect the same failure or a slight variant of it to occur. IRC r+ by Callek.

https://hg.mozilla.org/comm-central/rev/61c78da21439
Comment 15 Frank Wein [:mcsmurf] 2013-03-01 02:03:05 PST
Should I file a new bug for the build failures at http://tbpl.drapostles.org/?tree=SeaMonkey ? Looks like SeaMonkey builds fail in configure already (see Linux and Mac OS X builds, ignore Windows for now).
Comment 16 :Ms2ger 2013-03-01 02:14:56 PST
(In reply to Frank Wein [:mcsmurf] from comment #15)
> Should I file a new bug for the build failures at
> http://tbpl.drapostles.org/?tree=SeaMonkey ? Looks like SeaMonkey builds
> fail in configure already (see Linux and Mac OS X builds, ignore Windows for
> now).

That's bug 846524.
Comment 17 Gregory Szorc [:gps] 2013-03-01 09:39:56 PST
https://tbpl.mozilla.org/?tree=Thunderbird-Trunk is happy after a fixup landed in m-c. Calling this one done. Bug 846524 will track the Seamonkey portion of c-c.
Comment 18 Philip Chee 2013-03-31 20:28:48 PDT
Neil pushed a followup fix:

http://hg.mozilla.org/comm-central/rev/6cad2cc671ff
(Followup to copy a missing code block from mail/app.mozbuild to suite/app.mozbuild rs=Callek)

Note You need to log in before you can comment on or make changes to this bug.