Bug 1515579 - Use absolute paths for compilers, etc. r=ted
☠☠ backed out by d2bc993e14ea ☠ ☠
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 21 Dec 2018 23:05:40 +0000
changeset 451787 fcf2cca505d4cb8392d8ba42d827cc9b308f116f
parent 451786 1d645417b0829d67161fba8f60d61c90fc703be4
child 451788 7d919c848dc8055ee83953b1f851180e364d954b
push id35253
push userccoroiu@mozilla.com
push dateSat, 22 Dec 2018 04:12:04 +0000
treeherdermozilla-central@d30b4fd63e17 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1515579, 1259382, 1290040
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1515579 - Use absolute paths for compilers, etc. r=ted In bug 1259382, some workarounds were added to make the build system alter PATH and not use absolute paths for toolchain programs, because autoconf and the build system doesn't deal with spaces in those very well. But later in bug 1290040, we made find_program return Windows short paths (without spaces), which alleviates the need for those workarounds. We still, however, and unfortunately, need to alter PATH to account for the fact that MSVC DLLs are not necessarily alongside the compiler executables... Depends on D15181 Differential Revision: https://phabricator.services.mozilla.com/D15182
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -752,44 +752,66 @@ def vc_compiler_path(host, target, vs_ma
         # Choose the newest version.
         data = all_versions[-1][1]
     paths = data.get(vc_target)
     if not paths:
     return paths
+@imports(_from='os', _import='environ')
+def original_path():
+    return environ['PATH']
+@depends(vc_compiler_path, original_path)
 @imports(_from='os', _import='environ')
-def toolchain_search_path(vc_compiler_path):
-    result = [environ.get('PATH')]
+def toolchain_search_path(vc_compiler_path, original_path):
+    result = [original_path]
     if vc_compiler_path:
-        result.extend(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:])
+        # The first item is where the programs are going to be
+        result.append(vc_compiler_path[0])
     # Also add in the location to which `mach bootstrap` or
     # `mach artifact toolchain` installs clang.
     mozbuild_state_dir = environ.get('MOZBUILD_STATE_PATH',
                                      os.path.expanduser(os.path.join('~', '.mozbuild')))
     bootstrap_clang_path = os.path.join(mozbuild_state_dir, 'clang', 'bin')
     bootstrap_cbindgen_path = os.path.join(mozbuild_state_dir, 'cbindgen')
-    if vc_compiler_path:
-        # We're going to alter PATH for good in windows.configure, but we also
-        # need to do it for the valid_compiler() check below. This is only needed
-        # on Windows, where MSVC needs PATH set to find dlls.
-        environ['PATH'] = os.pathsep.join(result)
     return result
+# If we modified the PATH environment variable for Windows DLLs, set the PATH
+# variable for the build.
+@depends(toolchain_search_path, original_path)
+@imports(_from='os', _import='environ')
+def altered_path(toolchain_search_path, original_path):
+    # Touch the dependency for the linter. We don't actually need the value,
+    # only its side effects.
+    toolchain_search_path
+    if environ['PATH'] != original_path:
+        return environ['PATH']
+set_config('PATH', altered_path)
 def default_c_compilers(host_or_target, other_c_compiler=None):
     '''Template defining the set of default C compilers for the host and
     target platforms.
     `host_or_target` is either `host` or `target` (the @depends functions
     from init.configure.
     `other_c_compiler` is the `target` C compiler when `host_or_target` is `host`.
@@ -994,36 +1016,16 @@ def compiler(language, host_or_target, c
             flags = provided_compiler.flags
             flags = []
         if not flags and macos_sdk and host_or_target.os == 'OSX':
             flags = ['-isysroot', macos_sdk]
-        # Ideally, we'd always use the absolute path, but unfortunately, on
-        # Windows, the compiler is very often in a directory containing spaces.
-        # Unfortunately, due to the way autoconf does its compiler tests with
-        # eval, that doesn't work out. So in that case, check that the
-        # compiler can still be found in $PATH, and use the file name instead
-        # of the full path.
-        if quote(compiler) != compiler:
-            full_path = os.path.abspath(compiler)
-            compiler = os.path.basename(compiler)
-            found_compiler = find_program(compiler)
-            if not found_compiler:
-                die('%s is not in your $PATH'
-                    % quote(os.path.dirname(full_path)))
-            if os.path.normcase(find_program(compiler)) != os.path.normcase(
-                    full_path):
-                die('Found `%s` before `%s` in your $PATH. '
-                    'Please reorder your $PATH.',
-                    quote(os.path.dirname(found_compiler)),
-                    quote(os.path.dirname(full_path)))
         info = check_compiler(wrapper + [compiler] + flags, language,
         # Check that the additional flags we got are enough to not require any
         # more flags. If we get an exception, just ignore it; it's liable to be
         # invalid command-line flags, which means the compiler we're checking
         # doesn't support those command-line flags and will fail one or more of
         # the checks below.
@@ -1460,17 +1462,18 @@ def pgo_flags(compiler, build_env, targe
 set_config('PROFILE_GEN_CFLAGS', pgo_flags.gen_cflags)
 set_config('PROFILE_GEN_LDFLAGS', pgo_flags.gen_ldflags)
 set_config('PROFILE_USE_CFLAGS', pgo_flags.use_cflags)
 set_config('PROFILE_USE_LDFLAGS', pgo_flags.use_ldflags)
 llvm_profdata = check_prog('LLVM_PROFDATA', ['llvm-profdata'],
-                           allow_missing=True)
+                           allow_missing=True,
+                           paths=toolchain_search_path)
 add_old_configure_assignment('LLVM_PROFDATA', llvm_profdata)
 def preprocess_option(compiler):
     # The uses of PREPROCESS_OPTION depend on the spacing for -o/-Fi.
     if compiler.type in ('gcc', 'clang'):
@@ -1952,17 +1955,18 @@ def as_info(target, c_compiler):
 # One would expect the assembler to be specified merely as a program.  But in
 # cases where the assembler is passed down into js/, it can be specified in
 # the same way as CC: a program + a list of argument flags.  We might as well
 # permit the same behavior in general, even though it seems somewhat unusual.
 # So we have to do the same sort of dance as we did above with
 # `provided_compiler`.
 provided_assembler = provided_program('AS')
 assembler = check_prog('_AS', input=provided_assembler.program,
-                       what='the assembler', progs=as_info.names)
+                       what='the assembler', progs=as_info.names,
+                       paths=toolchain_search_path)
 @depends(as_info, assembler, provided_assembler, c_compiler)
 def as_with_flags(as_info, assembler, provided_assembler, c_compiler):
     if provided_assembler:
         return provided_assembler.wrapper + \
             [provided_assembler.program] + \
--- a/build/moz.configure/windows.configure
+++ b/build/moz.configure/windows.configure
@@ -251,37 +251,33 @@ def valid_ucrt_sdk_dir(windows_sdk_dir, 
     return namespace(
+@depends(c_compiler, toolchain_search_path)
-def vc_path(c_compiler):
+def vc_path(c_compiler, toolchain_search_path):
     vc_path_env = os.environ.get('VC_PATH')
     if vc_path_env:
         return os.path.normpath(vc_path_env)
     if c_compiler.type not in ('msvc', 'clang-cl'):
-    # Normally, we'd start from c_compiler.compiler, but for now, it's not the
-    # ideal full path to the compiler. At least, we're guaranteed find_program
-    # will get us the one we found in toolchain.configure.
     vc_program = c_compiler.compiler
     # In clang-cl builds, we use the headers and libraries from an MSVC installation.
     if c_compiler.type == 'clang-cl':
-        vc_program = 'cl.exe'
+        vc_program = find_program('cl.exe', paths=toolchain_search_path)
-    cl = find_program(vc_program)
-    result = os.path.dirname(cl)
+    result = os.path.dirname(vc_program)
     while True:
         next, p = os.path.split(result)
         if next == result:
             die('Cannot determine the Visual C++ directory the compiler (%s) '
                 'is in' % cl)
         result = next
         if p.lower() == 'bin':
@@ -463,30 +459,16 @@ link = check_prog('LINKER', ('lld-link.e
 host_link = check_prog('HOST_LINKER', ('lld-link.exe', 'link.exe'),
 add_old_configure_assignment('LINKER', link)
-# Normally, we'd just have CC, etc. set to absolute paths, but the build system
-# doesn't currently handle properly the case where the paths contain spaces.
-# Additionally, there's the issue described in toolchain.configure, in
-# valid_compiler().
-def alter_path(sdk_bin_path):
-    path = os.pathsep.join(sdk_bin_path)
-    os.environ['PATH'] = path
-    return path
-set_config('PATH', alter_path)
 check_prog('MAKECAB', ('makecab.exe',))
 @depends(c_compiler, using_sccache)
 def need_showincludes_prefix(info, using_sccache):
     # sccache does its own -showIncludes prefix checking.
     # clang-cl uses a gcc-style dependency scheme, see toolchain.configure.
     if info.type == 'msvc' and not using_sccache:
--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -1083,17 +1083,18 @@ def midl_names(c_compiler, toolchain_pre
 def check_for_midl(target, compile_environment):
     if target.os != 'WINNT':
     if compile_environment:
         return True
-midl = check_prog('MIDL', midl_names, when=check_for_midl, allow_missing=True)
+midl = check_prog('MIDL', midl_names, when=check_for_midl, allow_missing=True,
+                  paths=sdk_bin_path)
 @depends(c_compiler, target, when=depends(midl, target)(lambda m, t: m and t.kernel == 'WINNT'))
 def midl_flags(c_compiler, target):
     if c_compiler and c_compiler.type in ('msvc', 'clang-cl'):
         env = {
             'x86': 'win32',
             'x86_64': 'x64',
@@ -1270,17 +1271,18 @@ option('--enable-layout-debugger',
 set_config('MOZ_LAYOUT_DEBUGGER', depends_if('--enable-layout-debugger')(lambda _: True))
 # Shader Compiler for Windows (and MinGW Cross Compile)
 # ==============================================================
 with only_when(compile_environment):
     fxc = check_prog('FXC', ('fxc.exe', 'fxc2.exe'), when=depends(target)
-                     (lambda t: t.kernel == 'WINNT'))
+                     (lambda t: t.kernel == 'WINNT'),
+                     paths=sdk_bin_path)
     wine = check_prog('WINE', ['wine'], when=depends(target, host)
                       (lambda t, h: t.kernel == 'WINNT' and h.kernel == 'Linux'))
 # VPX
 # ===
 with only_when(compile_environment):