Closed Bug 1182727 Opened 4 years ago Closed 4 years ago

Update the build-clang.py script to bring it into the 21st century

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(15 files)

5.78 KB, patch
rail
: review+
Details | Diff | Splinter Review
3.57 KB, patch
rail
: review+
Details | Diff | Splinter Review
1.89 KB, patch
rail
: review+
Details | Diff | Splinter Review
5.10 KB, patch
rail
: review+
Details | Diff | Splinter Review
5.45 KB, patch
rail
: review+
Details | Diff | Splinter Review
1.27 KB, patch
rail
: review+
Details | Diff | Splinter Review
2.26 KB, patch
rail
: review+
Details | Diff | Splinter Review
3.26 KB, patch
rail
: review+
Details | Diff | Splinter Review
7.59 KB, patch
rail
: review+
Details | Diff | Splinter Review
5.56 KB, patch
rail
: review+
Details | Diff | Splinter Review
1.09 KB, patch
rail
: review+
Details | Diff | Splinter Review
7.42 KB, patch
rail
: review+
Details | Diff | Splinter Review
4.59 KB, patch
rail
: review+
Details | Diff | Splinter Review
2.33 KB, patch
rail
: review+
Details | Diff | Splinter Review
1.20 KB, patch
Nika
: review+
Details | Diff | Splinter Review
The current script builds using configure and make, and the LLVM project is going to drop support for that build system soon.  Plus, it's 2015.  ;-)  It's time to use cmake and ninja.  That actually helps a lot with build speed (ninja is about 1.5 times faster than make on my machine.)

Also, I think it's better to do three stage builds, but I want to also make it possible to build with any number of stages requested.
I'd love to help make this script buildable in a way that satisfies bug 1167649 while we're at it. It's just a matter of finding or creating a Docker image with the build prerequisites, and writing a tiny bit of glue around it to make things work in Taskcluster.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> I'd love to help make this script buildable in a way that satisfies bug
> 1167649 while we're at it.

Sorry, I wasn't clear.  I am working on this!  Stay tuned.  I believe that my work will not hurt bug 1167649 if not help it, I'm not sure exactly what that bug wants to achieve.  But after my work, you should be able to build clang in multiple configurations configurable from a config file for Mac and Linux.

> It's just a matter of finding or creating a
> Docker image with the build prerequisites, and writing a tiny bit of glue
> around it to make things work in Taskcluster.

I don't know what the above means for OSX, but for Linux, we currently use our ancient CentOS setup in a way that is not quite the same as our builders, but I'm not sure in what ways.  You should probably speak to Rail about this.
Assignee: nobody → ehsan
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #3)
> Sorry, I wasn't clear.  I am working on this!  Stay tuned.  I believe that
> my work will not hurt bug 1167649 if not help it, I'm not sure exactly what
> that bug wants to achieve.  But after my work, you should be able to build
> clang in multiple configurations configurable from a config file for Mac and
> Linux.

If you look at bug 1181722 and the driver script I wrote in https://github.com/luser/breakpad-taskcluster/blob/master/build-in-tc.py you can see where I'd like us to be. Those scripts let me kick off builds of minidump_stackwalk in taskcluster, so that a single script invocation does all the building in an automated way and produces binaries that are ready to use. The script invocation is a bit hairy right now, but that could be simplified. It looks like:
python build-in-tc.py --gecko-repository=https://hg.mozilla.org/users/tmielczarek_mozilla.com/mc --gecko-rev=36d25a29ad6f --breakpad-repository=https://github.com/luser/google-breakpad-mirror.git/branches/fix-mingw  ~/taskcluster-auth.json ~/tooltool-upload-token /build/mozilla-central/

...at the end of that process, new binaries will be in tooltool and the manifests in that mozilla-central clone will be updated with the new hashes.
OK, then I think you should be able to use my new script in this way.  But I'm still fighting CentOS related issues...
This script had some work done in the bugs I just marked as deps.
Depends on: 1123386, 1203388
Depends on: 1210397
OK, I finally managed to get a fully working clang here on all platforms: <https://treeherder.mozilla.org/#/jobs?repo=try&revision=204032d9f967> (and then <https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f71ba3b7878> for the OSX build bustage with the fix in part 15.)  I'll submit the patches now.
Since CMake generated build systems can run cmake if necessary, this
will make it possible to pick up changes from the source directory if
any and resume as much of the build as possible.

This builds the foundation for removing the need to blow away any of the
work done by the previous runs of the script.
Attachment #8668771 - Flags: review?(rail)
This will completely remove the need to blow away any of the work
previously done.
Attachment #8668772 - Flags: review?(rail)
This adds a stages config option, which can be used to select 1, 2, and
3 stage builds.  It also marks the default trunk configuration to do 3
stage builds, and defaults to that.
Attachment #8668773 - Flags: review?(rail)
Attachment #8668776 - Flags: review?(rail)
Attachment #8668778 - Flags: review?(rail)
Attachment #8668779 - Flags: review?(rail)
For some mysterious reason, the plugin crashes when loaded under
the -std=gnu89 that we use by default for C.
Attachment #8668784 - Flags: review?(michael)
Comment on attachment 8668784 [details] [diff] [review]
Part 15: Build the clang-plugin C test in C11 mode

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

LGTM
Attachment #8668784 - Flags: review?(michael) → review+
Comment on attachment 8668770 [details] [diff] [review]
Part 1: Switch to using cmake and ninja in order to build clang

I squashed all patches and reviewed the result. All patches except 13 applied correctly. #13 can be stamped by anyone, hard to review it for reals. :)
Attachment #8668770 - Flags: review?(rail) → review+
Attachment #8668771 - Flags: review?(rail) → review+
Attachment #8668772 - Flags: review?(rail) → review+
Attachment #8668773 - Flags: review?(rail) → review+
Attachment #8668774 - Flags: review?(rail) → review+
Attachment #8668775 - Flags: review?(rail) → review+
Attachment #8668776 - Flags: review?(rail) → review+
Attachment #8668777 - Flags: review?(rail) → review+
Attachment #8668778 - Flags: review?(rail) → review+
Attachment #8668779 - Flags: review?(rail) → review+
Attachment #8668780 - Flags: review?(rail) → review+
Attachment #8668781 - Flags: review?(rail) → review+
Attachment #8668783 - Flags: review?(rail) → review+
Comment on attachment 8668782 [details] [diff] [review]
Part 13: Update the current clang builds to the new 3-stage clang 3.7

If it applies, lgtm
Attachment #8668782 - Flags: review?(rail) → review+
Thanks for the reviews!  I think the reason that part 13 didn't apply for you may be that your tree doesn't have changes from bug 1205242?  Anyways, it applies cleanly on inbound.  :-)
Looks like I made a mistake, the new compiler still crashes in the C test.  I'm backing out the toolchain changes for now to investigate.
Keywords: leave-open
Note that there's something weird between linux32/clang.manifest and linux64/clang.manifest, where both have the same size/digest, but not the same filename (bz2 vs. xz).
Yeah, some of these file names were wrong, I'll fix it.
OK, turns out that we need to build the toolchain on a 10.7 machine.  That way, I got to a clang package that works!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7856145fd244
Keywords: leave-open
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.