775670 - Build Thunderbird using clang

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P1)

Description

[John Hopkins (:jhopkins)]

comm-central is currently closed due to the following error:

make -C config export
nsinstall.c
/builds/slave/tb-c-cen-osx64/build/mozilla/clang/bin/clang -o host_nsinstall.o -c  -Wall -Wpointer-arith -Wdeclaration-after-statement -Werror=return-type -Wempty-body -Wno-unused -Wno-overlength-strings -Wcast-align -isysroot /Developer/SDKs/MacOSX10.6.sdk -fno-strict-aliasing -ffunction-sections -fdata-sections -pthread -DNO_X11 -pipe  -DNDEBUG -DTRIMMED -g -O3 -fomit-frame-pointer -DXP_UNIX -DXP_MACOSX -DNO_X11 -O3  -DUNICODE -D_UNICODE  -I/builds/slave/tb-c-cen-osx64/build/mozilla/config -I. -I../dist/include  -I/builds/slave/tb-c-cen-osx64/build/objdir-tb/i386/mozilla/dist/include/nspr -I/builds/slave/tb-c-cen-osx64/build/objdir-tb/i386/mozilla/dist/include/nss     -I/builds/slave/tb-c-cen-osx64/build/objdir-tb/i386/mozilla/dist/include/nspr /builds/slave/tb-c-cen-osx64/build/mozilla/config/nsinstall.c
make[7]: /builds/slave/tb-c-cen-osx64/build/mozilla/clang/bin/clang: No such file or directory
pathsub.c
/builds/slave/tb-c-cen-osx64/build/mozilla/clang/bin/clang -o host_pathsub.o -c  -Wall -Wpointer-arith -Wdeclaration-after-statement -Werror=return-type -Wempty-body -Wno-unused -Wno-overlength-strings -Wcast-align -isysroot /Developer/SDKs/MacOSX10.6.sdk -fno-strict-aliasing -ffunction-sections -fdata-sections -pthread -DNO_X11 -pipe  -DNDEBUG -DTRIMMED -g -O3 -fomit-frame-pointer -DXP_UNIX -DXP_MACOSX -DNO_X11 -O3  -DUNICODE -D_UNICODE  -I/builds/slave/tb-c-cen-osx64/build/mozilla/config -I. -I../dist/include  -I/builds/slave/tb-c-cen-osx64/build/objdir-tb/i386/mozilla/dist/include/nspr -I/builds/slave/tb-c-cen-osx64/build/objdir-tb/i386/mozilla/dist/include/nss     -I/builds/slave/tb-c-cen-osx64/build/objdir-tb/i386/mozilla/dist/include/nspr /builds/slave/tb-c-cen-osx64/build/mozilla/config/pathsub.c
make[7]: /builds/slave/tb-c-cen-osx64/build/mozilla/clang/bin/clang: No such file or directory
make[7]: *** [host_nsinstall.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[7]: *** [host_pathsub.o] Error 1
make[6]: *** [export_tier_base] Error 2
make[5]: *** [tier_base] Error 2
make[4]: *** [default] Error 2
make[3]: *** [default] Error 2
make[2]: *** [build] Error 2
make[1]: *** [build] Error 2
make: *** [build] Error 2
program finished with exit code 2

We will need to get Thunderbird using tooltool and clang to resolve this.

See also: bug 672210, bug 774462, bug 774796

Comment 1

[Rail Aliiev [:rail]]

One of the problems which prevents us using Firefox tooltool manifests is the fact that we don't have mozilla tree checked out when we run the tooltool step (the tree is checked out by "compile" step).

Additionally tooltool work in the "build" directory by default, so it expands clang to build/clang, while the mozcofigs points us to $topsrcdir/clang which is biuld/mozilla/clang in Thunderbird case. Probably we'll need to tell setup.sh to create a symlink.

Comment 2

[John Hopkins (:jhopkins)]

Attachment: buildbot-configs patch

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 644034 [details] [diff] [review]
buildbot-configs patch

You don't need to define tooltool_manifest_src per branch anymore. Define those per platform in PLATFORM_VARS and you are done. http://hg.mozilla.org/build/tools/file/726a439dc568/scripts/tooltool/fetch_and_unpack.sh

See bug 775509, attachment 643883 [details] [diff] [review] and http://hg.mozilla.org/build/tools/file/726a439dc568/scripts/tooltool/fetch_and_unpack.sh.

Comment 4

[John Hopkins (:jhopkins)]

Attachment: comm-central patch

Comment 5

[John Hopkins (:jhopkins)]

(In reply to Rail Aliiev [:rail] from comment #3)
> You don't need to define tooltool_manifest_src per branch anymore. Define
> those per platform in PLATFORM_VARS and you are done.

What is the mechanism that ensures only comm-central and try-comm-central will receive those changes?

Comment 6

[John Hopkins (:jhopkins)]

Attachment: mozilla-central patch

Comment 7

[John Hopkins (:jhopkins)]

mconley:

Unfortunately even with the attached patches, the tree will need work before it compiles cleanly:

 http://people.mozilla.org/~jhopkins/bug775670/buildlog.html

Any idea how much work to get a clean build?  Should we land the clang changes or instead find a way to continue to build with gcc?

Comment 8

[Rail Aliiev [:rail]]

(In reply to John Hopkins (:jhopkins) from comment #5)
> (In reply to Rail Aliiev [:rail] from comment #3)
> > You don't need to define tooltool_manifest_src per branch anymore. Define
> > those per platform in PLATFORM_VARS and you are done.
> 
> What is the mechanism that ensures only comm-central and try-comm-central
> will receive those changes?

The wrapper script runs noop if there is no manifest in the source tree:
http://hg.mozilla.org/build/tools/file/726a439dc568/scripts/tooltool/fetch_and_unpack.sh#l10

Comment 9

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 644043 [details] [diff] [review]
comm-central patch

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

::: mail/config/tooltool-manifests/linux32/clang.manifest
@@ +1,3 @@
> +[
> +{
> +"clang_version": "r160364"

So here you are binding mail to a specific clang version, which means you will need to update it manually every time Firefox does so. Will the Firefox team be aware of this and also change mail configs, or can we find a way to use the Firefox configs?

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: compile with clang patch

Getting it to build is pretty easy, I did this recently when trying to uncrash my build. 

The only thing I didn't investigate is getting the mdimporter to work, which is why its commented out. I believe the problem was that the mdimporter is compiled with Xcode, which doesn't detect the SVN clang as a valid version.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

From the comm-central build log:

/builds/slave/tb-c-cen-osx64/build/mailnews/compose/src/nsMsgAppleEncode.cpp:109:44: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Wreserved-user-defined-literal]
clang: warning: argument unused during compilation: '-I ../../../../../mailnews/addrbook/src'
                                                   
/builds/slave/tb-c-cen-osx64/build/mailnews/compose/src/nsMsgAppleEncode.cpp:350:17: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Wreserved-user-defined-literal]
                                                CRLF"--%s"CRLF, 
                                                          ^

Curious that this doesn't pop up when I build locally.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mike Conley (:mconley) from comment #11)
> From the comm-central build log:
> 
> 
> Curious that this doesn't pop up when I build locally.

(with clang) - though perhaps the newer version is more strict about this sort of thing.

Comment 13

[John Hopkins (:jhopkins)]

Attachment: buildbot-configs patch

address rail's feedback.  remove a few unused platform configs rather than update them.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Raphael:

Hey - any insight on what we're hitting and why?

-Mike

Comment 15

[John Hopkins (:jhopkins)]

I spoke with mconley in IRC and he has given the ok to apply the clang config changes.

Comment 16

[Rail Aliiev [:rail]]

Comment on attachment 644055 [details] [diff] [review]
mozilla-central patch

I'd like to hear Rafael's thoughts about this way of "detecting" the proper location of clang.

Comment 17

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

(In reply to Mike Conley (:mconley) from comment #14)
> Raphael:
> 
> Hey - any insight on what we're hitting and why?

The "requires a space between literal and identifier" error? That is probably because of different versions of clang and c++11 is a recent addition.

> -Mike

Cheers,
Rafael

Comment 18

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Comment on attachment 644055 [details] [diff] [review]
mozilla-central patch

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

It is ok with me if you add a comment saying which cases each side of the 'if' is covering (regular tree or comm-central). Just wait for rail to comment too in case he knows a better way.

Comment 19

[Mike Conley (:mconley) (:⚙️)]

Attachment: comm-central: Put spaces between literals and identifiers

Ok, thanks Raphael. This patch cleared up the build problems for me locally.

Comment 20

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

(In reply to Mike Conley (:mconley) from comment #19)
> Created attachment 644340 [details] [diff] [review]
> comm-central: Put spaces between literals and identifiers
> 
> Ok, thanks Raphael. This patch cleared up the build problems for me locally.

I think that is the correct thing to do, but you should probably realign the comments in mailnews/import/eudora/src/nsEudoraFilters.cpp :-)

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Attachment: comm-central: Put spaces between literals and identifiers (v2)

Good call.

Comment 22

[Rail Aliiev [:rail]]

Comment on attachment 644055 [details] [diff] [review]
mozilla-central patch

jhopkins, could you add comments for "if" and "elif" when you land?

Comment 23

[Rail Aliiev [:rail]]

Comment on attachment 644043 [details] [diff] [review]
comm-central patch

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

::: build/macosx/common
@@ +3,5 @@
> +    export CXX=$topsrcdir/clang/bin/clang++
> +elif [ -d "$topsrcdir/../clang" ]; then
> +    export CC=$topsrcdir/../clang/bin/clang
> +    export CXX=$topsrcdir/../clang/bin/clang++
> +fi

A nit. Could you transplant the comments from the mozilla-central patch here as well.

Comment 24

[John Hopkins (:jhopkins)]

Comment on attachment 644307 [details] [diff] [review]
buildbot-configs patch

Double-landed in:

https://hg.mozilla.org/build/buildbot-configs/rev/07bc8bebd3cb
https://hg.mozilla.org/build/buildbot-configs/rev/ddb4f2ee4d15

Comment 25

[John Hopkins (:jhopkins)]

Comment on attachment 644055 [details] [diff] [review]
mozilla-central patch

Landed in http://hg.mozilla.org/mozilla-central/rev/eea94a9b40a1

Comment 26

[John Hopkins (:jhopkins)]

Comment on attachment 644342 [details] [diff] [review]
comm-central: Put spaces between literals and identifiers (v2)

Landed in http://hg.mozilla.org/comm-central/rev/ec138f1fcabf

Comment 27

[John Hopkins (:jhopkins)]

Comment on attachment 644043 [details] [diff] [review]
comm-central patch

Landed in http://hg.mozilla.org/comm-central/rev/b0d37f5b17ee

Comment 28

[John Hopkins (:jhopkins)]

Builds are green and tree is reopened.  Thanks, everyone!

Comment 29

[Nomis101]

(In reply to Philipp Kewisch [:Fallen] from comment #10)
> The only thing I didn't investigate is getting the mdimporter to work, which
> is why its commented out. I believe the problem was that the mdimporter is
> compiled with Xcode, which doesn't detect the SVN clang as a valid version.
Do you had an error for mdimporter which sayed "failed to exec real xcrun"? I had this in Bug 764964 and fixed it with xcode-select -switch.