Closed Bug 1519954 Opened 1 year ago Closed 9 months ago

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

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox69 fixed)

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: nalexander, Assigned: glandium)

References

(Regressed 2 open bugs, )

Details

Attachments

(1 file)

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).

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

(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.

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

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.

Flags: needinfo?(nfroyd)
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

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

Assignee: nobody → mh+mozilla
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/4a1e7d5a5b44
Pick binaries from mach bootstrap first on local developer builds. r=nalexander
Blocks: 1557213
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
See Also: → 1557419
Regressions: 1559339
Regressions: 1563571
Flags: needinfo?(nfroyd)
Regressions: 1574797
You need to log in before you can comment on or make changes to this bug.