Closed Bug 845089 Opened 11 years ago Closed 11 years ago

Thunderbird build config changes in a moz.build world

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 22.0

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(1 file, 3 obsolete files)

I'll be providing patches to make comm-central work post bug 784841.
Attached patch moz.build compatibility, v1 (obsolete) — Splinter Review
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).
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attached patch moz.build compatibility, v2 (obsolete) — Splinter Review
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.
Attachment #718241 - Attachment is obsolete: true
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 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 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.
Attached patch moz.build compatibility, v3 (obsolete) — Splinter Review
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...
Attachment #718282 - Attachment is obsolete: true
(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.
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.
(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.
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.
Attachment #718785 - Attachment is obsolete: true
Attachment #719320 - Flags: review?(bugspam.Callek)
Attachment #719320 - Flags: review?(Pidgeot18)
Attachment #719320 - Flags: feedback?(ted)
Attachment #719320 - Flags: feedback?(mh+mozilla)
Attachment #719320 - Flags: feedback?(Ms2ger)
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?
Attachment #719320 - Flags: feedback?(mh+mozilla)
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?
Attachment #719320 - Flags: review?(bugspam.Callek)
Attachment #719320 - Flags: review?(Pidgeot18)
Attachment #719320 - Flags: review+
Attachment #719320 - Flags: feedback?(ted)
Attachment #719320 - Flags: feedback?(Ms2ger)
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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).
(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.
Depends on: 846524
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.
Blocks: 846524
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
No longer depends on: 846524
Resolution: --- → FIXED
Summary: comm-central build config changes in a moz.build world → Thunderbird build config changes in a moz.build world
Target Milestone: --- → Thunderbird 22.0
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)
You need to log in before you can comment on or make changes to this bug.