1328199 - Add a TaskCluster job to build clang-tidy for OSX

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[(no longer active)]

I have added support for cross-compiling LLVM for OS X on our Linux64 builders.  We can now run them through TaskCluster!

Initially I used toolchain-script similar to the Linux version of this script, but one issue that I have faced is that I needed to download the OS X 10.10 SDK from tooltool with visibility: internal but I couldn't get that to work because I couldn't find a way to download the /builds/relengapi.tok authentication file.  I have an unfinished patch for that and will file it separately.

Since I knew that mozharness jobs run through taskcluster can download internal tooltool resources (since our Mac cross-compilers download the internal OSX 10.7 SDK package) I switched to using mozharsness and the tooltool download issue is bypassed that way.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c008085a785c385dcff7cd2e21d58257b67ee6b

Comment 1

[(no longer active)]

Attachment: Part 1: Add support for cross-compiling clang for OS X on Linux

Comment 2

[(no longer active)]

Attachment: Part 2: Add a TaskCluster job to build clang-tidy for OSX using mozharness

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8823158 [details] [diff] [review]
Part 2: Add a TaskCluster job to build clang-tidy for OSX using mozharness

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

I'm really only capable of looking at the taskcluster/ci/** stuff, but that looks good to me.  I don't mind adding a new use of mozharness, but if it's easier to just use the relengapi proxy directly from a shell script, then I'm happy with that too.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8823157 [details] [diff] [review]
Part 1: Add support for cross-compiling clang for OS X on Linux

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

::: build/build-clang/README
@@ +45,5 @@
>  * patches: Optional list of patches to apply per platform.  Supported platforms: macosx64, linux32, linux64.  The default is Release.
>  * build_type: The type of build to make.  Supported types: Release, Debug, RelWithDebInfo or MinSizeRel.
>  * build_libcxx: Whether to build with libcxx.  The default is false.
>  * import_clang_tidy: Whether to import Mozilla checks into clang-tidy before building.  The default is false.
> +* osx_cross_compile: Whether to invoke CMake for OS X cross compile builds.

I am not the biggest fan of the name "osx_cross_compile", because it seems very specific, but I'm fine with it.

::: build/build-clang/build-clang.py
@@ +229,5 @@
>      if is_windows():
>          cmake_args.insert(-1, "-DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON")
> +    if ranlib is not None:
> +        cmake_args += ["-DCMAKE_RANLIB=%s" % slashify_path(ranlib)]
> +    if osx_cross_compile:

Can we assert that we're on linux here? I doubt that this codepath would work well otherwise.

@@ +241,5 @@
> +                       "-DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY=ONLY",
> +                       "-DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE=ONLY",
> +                       "-DCMAKE_MACOSX_RPATH=@executable_path",
> +                       "-DCMAKE_OSX_ARCHITECTURES=x86_64",
> +                       "-DDARWIN_osx_ARCHS=x86_64",

This name looks like it might be a mistake (the lowercase "osx"), but I don't know enough off of the top of my head about clang's buildsystem to be able to say. A quick grep in my (admittedly outdated) clone of llvm+clang didn't find this variable mentioned anywhere.

Comment 5

[(no longer active)]

(In reply to Dustin J. Mitchell [:dustin] from comment #3)
> Comment on attachment 8823158 [details] [diff] [review]
> Part 2: Add a TaskCluster job to build clang-tidy for OSX using mozharness
> 
> Review of attachment 8823158 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm really only capable of looking at the taskcluster/ci/** stuff, but that
> looks good to me.  I don't mind adding a new use of mozharness, but if it's
> easier to just use the relengapi proxy directly from a shell script, then
> I'm happy with that too.

Bug 1328172 was the only reason why I used MozHarness here.  If we can easily fix that, I'd be happy to avoid MozHarness in this job similar to its sibling jobs.

Comment 6

[(no longer active)]

(In reply to Michael Layzell [:mystor] from comment #4)
> Comment on attachment 8823157 [details] [diff] [review]
> Part 1: Add support for cross-compiling clang for OS X on Linux
> 
> Review of attachment 8823157 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: build/build-clang/README
> @@ +45,5 @@
> >  * patches: Optional list of patches to apply per platform.  Supported platforms: macosx64, linux32, linux64.  The default is Release.
> >  * build_type: The type of build to make.  Supported types: Release, Debug, RelWithDebInfo or MinSizeRel.
> >  * build_libcxx: Whether to build with libcxx.  The default is false.
> >  * import_clang_tidy: Whether to import Mozilla checks into clang-tidy before building.  The default is false.
> > +* osx_cross_compile: Whether to invoke CMake for OS X cross compile builds.
> 
> I am not the biggest fan of the name "osx_cross_compile", because it seems
> very specific, but I'm fine with it.

Well, this variable is only intended to be used for OXS targeting cross compiles on Linux, so the specificity of the name is intentional.  Can you think of a better name?

> ::: build/build-clang/build-clang.py
> @@ +229,5 @@
> >      if is_windows():
> >          cmake_args.insert(-1, "-DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON")
> > +    if ranlib is not None:
> > +        cmake_args += ["-DCMAKE_RANLIB=%s" % slashify_path(ranlib)]
> > +    if osx_cross_compile:
> 
> Can we assert that we're on linux here? I doubt that this codepath would
> work well otherwise.

Definitely a good idea.

> @@ +241,5 @@
> > +                       "-DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY=ONLY",
> > +                       "-DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE=ONLY",
> > +                       "-DCMAKE_MACOSX_RPATH=@executable_path",
> > +                       "-DCMAKE_OSX_ARCHITECTURES=x86_64",
> > +                       "-DDARWIN_osx_ARCHS=x86_64",
> 
> This name looks like it might be a mistake (the lowercase "osx"), but I
> don't know enough off of the top of my head about clang's buildsystem to be
> able to say. A quick grep in my (admittedly outdated) clone of llvm+clang
> didn't find this variable mentioned anywhere.

It isn't a typo, it's referenced here: <https://llvm.org/svn/llvm-project/compiler-rt/trunk/cmake/builtin-config-ix.cmake>

See the default assignment which screws us over, since our cctools are too old for x86_64h:

  if(NOT DARWIN_osx_ARCHS)
    set(DARWIN_osx_ARCHS i386 x86_64 x86_64h)
  endif()

Comment 7

[(no longer active)]

Attachment: Part 2: Add a TaskCluster job to build clang-tidy for OSX on Linux

With the fix to bug 1328172, this can now be done using toolchain-script like other similar jobs, which is much simpler.  I decided to just redo things here.

Green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2699b38c721752976231d747c66652b2221cc015

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8823522 [details] [diff] [review]
Part 2: Add a TaskCluster job to build clang-tidy for OSX on Linux

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

::: taskcluster/scripts/misc/build-clang-tidy-macosx.sh
@@ +1,4 @@
> +#!/bin/bash
> +set -x -e -v
> +
> +# This script is for building clang for Linux.

..for Mac OS X :)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f6512e0df57
Part 1: Add support for cross-compiling clang for OS X on Linux; r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/0026f22490e2
Part 2: Add a TaskCluster job to build clang-tidy for OSX on Linux; r=dustin

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/229a81285dff
follow-up: Fix the incorrect filename

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8f6512e0df57
https://hg.mozilla.org/mozilla-central/rev/0026f22490e2
https://hg.mozilla.org/mozilla-central/rev/229a81285dff