Bug 1515579 - Use absolute paths for compilers, etc. r=ted
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 10 Jan 2019 23:37:46 +0000
changeset 453365 fa8e39feef4389d1ff83fe952c76ffba7c2217d6
parent 453364 5b112f7e8d9d259f51c7329236e634036ee8a4c9
child 453366 65326bd78f839f71f6c38d1b8a8e8b0cf10746e1
push id35352
push userdvarga@mozilla.com
push dateFri, 11 Jan 2019 04:12:48 +0000
treeherdermozilla-central@65326bd78f83 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs1515579, 1259382, 1290040
milestone66.0a1
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
build/moz.configure/toolchain.configure
build/moz.configure/windows.configure
toolkit/moz.configure
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -742,44 +742,68 @@ 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
     return paths
 
 
-@depends(vc_compiler_path)
+@dependable
+@imports('os')
+@imports(_from='os', _import='environ')
+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):
-    result = [environ.get('PATH')]
+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.
     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')
     result.append(bootstrap_clang_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
     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`.
     '''
@@ -984,36 +1008,16 @@ def compiler(language, host_or_target, c
             wrapper.extend(provided_wrapper)
             flags = provided_compiler.flags
         else:
             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,
                               host_or_target)
 
         # 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.
@@ -1476,17 +1480,18 @@ def is_windows(target, host):
     return host.kernel == 'WINNT' and target.kernel == 'WINNT'
 
 
 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')
 
 imply_option('MOZ_PGO',
              depends_if('--enable-profile-generate')(lambda _: True))
 
 set_config('MOZ_PROFILE_GENERATE',
@@ -1985,17 +1990,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] + \
             provided_assembler.flags
 
--- 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(
         path=sdk.path,
         include=sdk.include,
         lib=sdk.lib,
         version=version,
     )
 
 
-@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)
 
     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:
             die('Cannot determine the Visual C++ directory the compiler (%s) '
                 'is in' % cl)
         result = next
         if p.lower() == 'bin':
             break
@@ -463,30 +459,16 @@ link = check_prog('LINKER', ('lld-link.e
                   paths=toolchain_search_path)
 
 host_link = check_prog('HOST_LINKER', ('lld-link.exe', 'link.exe'),
                        paths=toolchain_search_path)
 
 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',))
 
 
 @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
@@ -1088,17 +1088,18 @@ def midl_names(c_compiler, toolchain_pre
 def check_for_midl(target, compile_environment):
     if target.os != 'WINNT':
         return
 
     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, vc_compiler_path,
          when=depends(midl, target)(lambda m, t: m and t.kernel == 'WINNT'))
 @imports(_from='mozbuild.shellutil', _import='quote')
 def midl_flags(c_compiler, target, vc_compiler_path):
     if c_compiler and c_compiler.type in ('msvc', 'clang-cl'):
         env = {
@@ -1287,17 +1288,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)
     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'))
 
 
 # VPX
 # ===