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

--HG--
extra : moz-landing-system : lando

build/moz.configure/toolchain.configure

@@ -747,14 +747,27 @@ def vc_compiler_path(host, target, vs_major_version, env, vs_release_name):
     return paths
 
 
-@depends(vc_compiler_path)
+@dependable
 @imports('os')
 @imports(_from='os', _import='environ')
-def toolchain_search_path(vc_compiler_path):
-    result = [environ.get('PATH')]
+def original_path():
+    return environ['PATH'].split(os.pathsep)
+
+
+@depends(vc_compiler_path, original_path)
+@imports('os')
+@imports(_from='os', _import='environ')
+def toolchain_search_path(vc_compiler_path, original_path):
+    result = list(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.
@@ -766,15 +779,26 @@ def toolchain_search_path(vc_compiler_path):
     bootstrap_cbindgen_path = os.path.join(mozbuild_state_dir, 'cbindgen')
     result.append(bootstrap_cbindgen_path)
 
-    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
 
 
+# As a workaround until bug 1516228 and bug 1516253 are fixed, set the PATH
+# variable for the build to contain the toolchain search path.
+@depends(toolchain_search_path)
+@imports('os')
+@imports(_from='os', _import='environ')
+def altered_path(toolchain_search_path):
+    path = environ['PATH'].split(os.pathsep)
+    altered_path = list(toolchain_search_path)
+    for p in path:
+        if p not in altered_path:
+            altered_path.append(p)
+    return os.pathsep.join(altered_path)
+
+
+set_config('PATH', altered_path)
+
+
 @template
 def default_c_compilers(host_or_target, other_c_compiler=None):
     '''Template defining the set of default C compilers for the host and
@@ -989,26 +1013,6 @@ def compiler(language, host_or_target, c_compiler=None, other_compiler=None,
         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,
                               host_or_target)
 
@@ -1481,7 +1485,8 @@ include('windows.configure', when=is_windows)
 # PGO
 # ==============================================================
 llvm_profdata = check_prog('LLVM_PROFDATA', ['llvm-profdata'],
-                           allow_missing=True)
+                           allow_missing=True,
+                           paths=toolchain_search_path)
 
 js_option('--enable-profile-generate',
           help='Build a PGO instrumented binary')
@@ -1990,7 +1995,8 @@ def as_info(target, c_compiler):
 # `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):

build/moz.configure/windows.configure

@@ -256,9 +256,9 @@ def valid_ucrt_sdk_dir(windows_sdk_dir, windows_sdk_dir_env, c_compiler):
     )
 
 
-@depends(c_compiler)
+@depends(c_compiler, toolchain_search_path)
 @imports('os')
-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)
@@ -266,17 +266,13 @@ def vc_path(c_compiler):
     if c_compiler.type not in ('msvc', 'clang-cl'):
         return
 
-    # 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:
@@ -468,20 +464,6 @@ 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().
-@depends(sdk_bin_path)
-@imports('os')
-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',))
 
 

toolkit/moz.configure

@@ -1093,7 +1093,8 @@ def check_for_midl(target, 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, vc_compiler_path,
@@ -1292,7 +1293,8 @@ set_config('MOZ_LAYOUT_DEBUGGER', depends_if('--enable-layout-debugger')(lambda
 
 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)
     add_old_configure_assignment('FXC', fxc)
     wine = check_prog('WINE', ['wine'], when=depends(target, host)
                       (lambda t, h: t.kernel == 'WINNT' and h.kernel == 'Linux'))