1519954 - Make |mach configure| prefer Mozilla's clang to local clang

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Nick Alexander :nalexander [he/him]]

Around https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#722-744, we determine the paths to various compilers. Right now we favour $PATH to ~/.mozbuild/clang. This ticket tracks changing the default so that we prefer ~/.mozbuild/clang. Mozilla's clang builds have patches that work around critical issues and can be easier to update (via |mach bootstrap|).

On the other hand, this leads us closer to a compiler/toolchain monoculture. Personally, I like said monoculture, 'cuz I spend a lot of time dealing with the fact that Android NDK toolchains no longer are able to build GeckoView/Firefox for Android :(

This would make Bug 1477487 simpler, but it's not a blocker (in either direction).

Comment 1

[Mike Hommey [:glandium]]

I think we should only prefer ~/.mozbuild/clang on developer builds.

Comment 2

[(Away)]

(In reply to Mike Hommey [:glandium] from comment #1)

I think we should only prefer ~/.mozbuild/clang on developer builds.

If that's what it takes to get a patch landed, so be it. Someone on IRC trips over this at least weekly, and we should really do something.

Comment 3

[Nick Alexander :nalexander [he/him]]

Note to self: this should include llvm-config as well.

Comment 4

[Nick Alexander :nalexander [he/him]]

Further note to self: for Android, it's not enough to rearrange the toolchain paths: we have logic to prefer the Android NDK toolchain prebuilt clangs, which are buggy. (I.e., I'm getting ICE building Rust code right now with r17b. Sigh.)

froydnj: perhaps you can have a crack at this? I don't know much about the Android NDK stuff in moz.configure we have right now.

Comment 5

[Nick Alexander :nalexander [he/him]]

modified   build/moz.configure/toolchain.configure
@@ -767,22 +767,24 @@ def toolchain_search_path_for(host_or_ta
         host: host_vc_compiler_path,
         target: vc_compiler_path,
     }[host_or_target]
 
-    @depends(vc_path, original_path)
+    @depends(vc_path, original_path, 'MOZ_AUTOMATION')
     @imports('os')
     @imports(_from='os', _import='environ')
-    def toolchain_search_path(vc_compiler_path, original_path):
-        result = list(original_path)
+    def toolchain_search_path(vc_compiler_path, original_path, automation):
+        vc_result = list(original_path)
 
         if vc_compiler_path:
             # The second item, if there is one, is necessary to have in $PATH for
             # Windows to load the required DLLs from there.
             if len(vc_compiler_path) > 1:
-                environ['PATH'] = os.pathsep.join(result + vc_compiler_path[1:])
+                environ['PATH'] = os.pathsep.join(vc_result + vc_compiler_path[1:])
 
             # The first item is where the programs are going to be
-            result.append(vc_compiler_path[0])
+            vc_result.append(vc_compiler_path[0])
+
+        result = list()
 
         # Also add in the location to which `mach bootstrap` or
         # `mach artifact toolchain` installs clang.
         mozbuild_state_dir = environ.get('MOZBUILD_STATE_PATH',
@@ -796,9 +798,17 @@ def toolchain_search_path_for(host_or_ta
         # Also add the rustup install directory for cargo/rustc.
         rustup_path = os.path.expanduser(os.path.join('~', '.cargo', 'bin'))
         result.append(rustup_path)
 
-        return result
+        if not automation:
+            # Prefer ~/.mozbuild/clang for local developer builds.
+            return result + vc_result
+        else:
+            return vc_result + result
+
     return toolchain_search_path

Comment 6

[Mike Hommey [:glandium]]

MOZ_AUTOMATION is not the negation of developer builds. You want developer_options.

Comment 7

[Mike Hommey [:glandium]]

Attachment: Bug 1519954 - Pick binaries from mach bootstrap first on local developer builds.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a1e7d5a5b44
Pick binaries from mach bootstrap first on local developer builds. r=nalexander

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4a1e7d5a5b44