Bug 1515528 - Detect MSVC paths separately for host and target. r=chmanchester
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 11 Jan 2019 00:20:31 +0000
changeset 453420 f1ec67ff78066b5a20fab4e1900166b005b46d46
parent 453419 eb83a8375b871fcd39ba97d37509320a2e98a406
child 453421 c7d7c62b7b11c28bcc44d8a3424dcb419b9d7bce
push id35357
push usernerli@mozilla.com
push dateFri, 11 Jan 2019 21:54:07 +0000
treeherdermozilla-central@0ce024c91511 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschmanchester
bugs1515528
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 1515528 - Detect MSVC paths separately for host and target. r=chmanchester Because MSVC compilers only support one architecture, we need to search "cl" in different toolchain search paths for each of the host and target, especially when they are different. Likewise for the library paths for the linker. Ideally we'd pass -LIBPATH both for host and target, but that has implications for rust that I don't want to have to figure just now. Depends on D15263 Differential Revision: https://phabricator.services.mozilla.com/D15264
build/moz.configure/toolchain.configure
build/moz.configure/windows.configure
config/rules.mk
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -708,93 +708,116 @@ js_option('--with-visual-studio-version'
 
 
 @depends('--with-visual-studio-version')
 def vs_major_version(value):
     if value:
         return {'2017': 15}[value[0]]
 
 
-@depends(host, target, vs_major_version, check_build_environment, '--with-visual-studio-version')
-@imports(_from='__builtin__', _import='sorted')
-@imports(_from='operator', _import='itemgetter')
-@imports('platform')
-def vc_compiler_path(host, target, vs_major_version, env, vs_release_name):
-    if host.kernel != 'WINNT':
-        return
-    vc_target = {
-        'x86': 'x86',
-        'x86_64': 'x64',
-        'arm': 'arm',
-        'aarch64': 'arm64'
-    }.get(target.cpu)
-    if vc_target is None:
-        return
-
-    all_versions = sorted(get_vc_paths(env.topsrcdir), key=itemgetter(0))
-    if not all_versions:
-        return
-    if vs_major_version:
-        versions = [d for (v, d) in all_versions if v.major ==
-                    vs_major_version]
-        if not versions:
-            die('Visual Studio %s could not be found!' % vs_release_name)
-        data = versions[0]
-    else:
-        # Choose the newest version.
-        data = all_versions[-1][1]
-    paths = data.get(vc_target)
-    if not paths:
-        return
-    return paths
+@template
+def vc_compiler_path_for(host_or_target):
+    @depends(host, host_or_target, vs_major_version, check_build_environment,
+             '--with-visual-studio-version')
+    @imports(_from='__builtin__', _import='sorted')
+    @imports(_from='operator', _import='itemgetter')
+    @imports('platform')
+    def vc_compiler_path(host, target, vs_major_version, env, vs_release_name):
+        if host.kernel != 'WINNT':
+            return
+        vc_target = {
+            'x86': 'x86',
+            'x86_64': 'x64',
+            'arm': 'arm',
+            'aarch64': 'arm64'
+        }.get(target.cpu)
+        if vc_target is None:
+            return
+
+        all_versions = sorted(get_vc_paths(env.topsrcdir), key=itemgetter(0))
+        if not all_versions:
+            return
+        if vs_major_version:
+            versions = [d for (v, d) in all_versions if v.major ==
+                        vs_major_version]
+            if not versions:
+                die('Visual Studio %s could not be found!' % vs_release_name)
+            data = versions[0]
+        else:
+            # Choose the newest version.
+            data = all_versions[-1][1]
+        paths = data.get(vc_target)
+        if not paths:
+            return
+        return paths
+    return vc_compiler_path
+
+
+vc_compiler_path = vc_compiler_path_for(target)
+host_vc_compiler_path = vc_compiler_path_for(host)
 
 
 @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, original_path):
-    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:])
-
-        # 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)
-
-    return result
+@template
+def toolchain_search_path_for(host_or_target):
+    vc_path = {
+        host: host_vc_compiler_path,
+        target: vc_compiler_path,
+    }[host_or_target]
+
+    @depends(vc_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:
+            # 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)
+
+        return result
+    return toolchain_search_path
+
+
+toolchain_search_path = toolchain_search_path_for(target)
+host_toolchain_search_path = toolchain_search_path_for(host)
 
 
 # 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)
+@depends(toolchain_search_path, host_toolchain_search_path)
 @imports('os')
 @imports(_from='os', _import='environ')
-def altered_path(toolchain_search_path):
+def altered_path(toolchain_search_path, host_toolchain_search_path):
     path = environ['PATH'].split(os.pathsep)
     altered_path = list(toolchain_search_path)
+    for p in host_toolchain_search_path:
+        if p not in altered_path:
+            altered_path.append(p)
     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)
 
@@ -836,16 +859,20 @@ def default_c_compilers(host_or_target, 
                     # If the target C compiler is GCC, and it can't be used with
                     # -m32/-m64 for the host, it's probably toolchain-prefixed,
                     # so we prioritize a raw 'gcc' instead.
                     prioritized = info.type
             elif info.type == 'clang' and android_clang_compiler:
                 # Android NDK clangs do not function as host compiler, so
                 # prioritize a raw 'clang' instead.
                 prioritized = info.type
+            elif info.type == 'msvc' and target.cpu != host_or_target.cpu:
+                # MSVC compilers only support one architecture, so we'll
+                # want a cl in another (detected) path.
+                prioritized = 'cl'
 
             types = [prioritized] + [t for t in types if t != info.type]
 
         gcc = ('gcc',)
         if toolchain_prefix and host_or_target is target:
             gcc = tuple('%sgcc' % p for p in toolchain_prefix) + gcc
 
         result = []
@@ -979,23 +1006,28 @@ def compiler(language, host_or_target, c
     what = 'the %s %s compiler' % (host_or_target_str, language)
 
     option(env=var, nargs=1, help='Path to %s' % what)
 
     # Handle the compiler given by the user through one of the CC/CXX/HOST_CC/
     # HOST_CXX variables.
     provided_compiler = provided_program(var)
 
+    search_path = {
+        host: host_toolchain_search_path,
+        target: toolchain_search_path,
+    }[host_or_target]
+
     # Normally, we'd use `var` instead of `_var`, but the interaction with
     # old-configure complicates things, and for now, we a) can't take the plain
     # result from check_prog as CC/CXX/HOST_CC/HOST_CXX and b) have to let
     # old-configure AC_SUBST it (because it's autoconf doing it, not us)
     compiler = check_prog('_%s' % var, what=what, progs=default_compilers,
                           input=provided_compiler.program,
-                          paths=toolchain_search_path)
+                          paths=search_path)
 
     @depends(compiler, provided_compiler, compiler_wrapper, host_or_target, macos_sdk)
     @checking('whether %s can be used' % what, lambda x: bool(x))
     @imports(_from='mozbuild.shellutil', _import='quote')
     def valid_compiler(compiler, provided_compiler, compiler_wrapper,
                        host_or_target, macos_sdk):
         wrapper = list(compiler_wrapper or ())
         if provided_compiler:
--- a/build/moz.configure/windows.configure
+++ b/build/moz.configure/windows.configure
@@ -338,76 +338,101 @@ def include_path(vc_path, windows_sdk_di
     includes = os.pathsep.join(includes)
     os.environ['INCLUDE'] = includes
     return includes
 
 
 set_config('INCLUDE', include_path)
 
 
-@depends(target, c_compiler, vc_path, valid_windows_sdk_dir, valid_ucrt_sdk_dir, dia_sdk_dir)
-@imports('os')
-def lib_path(target, c_compiler, vc_path, windows_sdk_dir, ucrt_sdk_dir, dia_sdk_dir):
-    if not vc_path:
-        return
-    sdk_target = {
-        'x86': 'x86',
-        'x86_64': 'x64',
-        'arm': 'arm',
-        'aarch64': 'arm64',
-    }.get(target.cpu)
+@template
+def lib_path_for(host_or_target):
+    compiler = {
+        host: host_c_compiler,
+        target: c_compiler,
+    }[host_or_target]
+
+    @depends(host_or_target, dependable(host_or_target is host), compiler, vc_path,
+             valid_windows_sdk_dir, valid_ucrt_sdk_dir, dia_sdk_dir)
+    @imports('os')
+    def lib_path(target, is_host, c_compiler, vc_path, windows_sdk_dir, ucrt_sdk_dir, dia_sdk_dir):
+        if not vc_path:
+            return
+        sdk_target = {
+            'x86': 'x86',
+            'x86_64': 'x64',
+            'arm': 'arm',
+            'aarch64': 'arm64',
+        }.get(target.cpu)
 
-    old_target = {
-        'x86': '',
-        'x86_64': 'amd64',
-        'arm': 'arm',
-        'aarch64': 'arm64'
-    }.get(target.cpu)
-    if old_target is None:
-        return
-    # As old_target can be '', and os.path.join will happily use the empty
-    # string, leading to a string ending with a backslash, that Make will
-    # interpret as a "string continues on next line" indicator, use variable
-    # args.
-    old_target = (old_target,) if old_target else ()
-    if c_compiler.version < '19.10':
-        # MSVC2015
-        vc_target = old_target
-    else:
-        # MSVC2017 switched to use the same target naming as the sdk.
-        vc_target = (sdk_target,)
+        old_target = {
+            'x86': '',
+            'x86_64': 'amd64',
+            'arm': 'arm',
+            'aarch64': 'arm64'
+        }.get(target.cpu)
+        if old_target is None:
+            return
+        # As old_target can be '', and os.path.join will happily use the empty
+        # string, leading to a string ending with a backslash, that Make will
+        # interpret as a "string continues on next line" indicator, use variable
+        # args.
+        old_target = (old_target,) if old_target else ()
+        if c_compiler.version < '19.10':
+            # MSVC2015
+            vc_target = old_target
+        else:
+            # MSVC2017 switched to use the same target naming as the sdk.
+            vc_target = (sdk_target,)
 
-    atlmfc_dir = os.path.join(vc_path, 'atlmfc', 'lib', *vc_target)
-    if not os.path.isdir(atlmfc_dir):
-        die('Cannot find the ATL/MFC libraries in the Visual C++ directory '
-            '(%s). Please install them.' % vc_path)
+        atlmfc_dir = os.path.join(vc_path, 'atlmfc', 'lib', *vc_target)
+        if not os.path.isdir(atlmfc_dir):
+            die('Cannot find the ATL/MFC libraries in the Visual C++ directory '
+                '(%s). Please install them.' % vc_path)
 
-    libs = []
-    lib_env = os.environ.get('LIB')
-    if lib_env:
-        libs.append(lib_env)
-    libs.extend((
-        os.path.join(vc_path, 'lib', *vc_target),
-        atlmfc_dir,
-        os.path.join(windows_sdk_dir.lib, 'um', sdk_target),
-        os.path.join(ucrt_sdk_dir.lib, 'ucrt', sdk_target),
-    ))
-    if dia_sdk_dir:
-        # For some reason the DIA SDK still uses the old-style targets
-        # even in a newer MSVC.
-        libs.append(os.path.join(dia_sdk_dir, 'lib', *old_target))
+        libs = []
+        lib_env = os.environ.get('LIB')
+        if lib_env and not is_host:
+            libs.extend(lib_env.split(os.pathsep))
+        libs.extend((
+            os.path.join(vc_path, 'lib', *vc_target),
+            atlmfc_dir,
+            os.path.join(windows_sdk_dir.lib, 'um', sdk_target),
+            os.path.join(ucrt_sdk_dir.lib, 'ucrt', sdk_target),
+        ))
+        if dia_sdk_dir:
+            # For some reason the DIA SDK still uses the old-style targets
+            # even in a newer MSVC.
+            libs.append(os.path.join(dia_sdk_dir, 'lib', *old_target))
+        return libs
+
+    return lib_path
+
+
+@depends(lib_path_for(target))
+@imports('os')
+def lib_path(libs):
     # Set in the environment for old-configure
     libs = os.pathsep.join(libs)
     os.environ['LIB'] = libs
     return libs
 
 
 set_config('LIB', lib_path)
 
 
+@depends(lib_path_for(host))
+@imports(_from='mozbuild.shellutil', _import='quote')
+def host_linker_libpaths(libs):
+    return ['-LIBPATH:%s' % quote(l) for l in libs]
+
+
+set_config('HOST_LINKER_LIBPATHS', host_linker_libpaths)
+
+
 option(env='MT', nargs=1, help='Path to the Microsoft Manifest Tool')
 
 
 @depends(valid_windows_sdk_dir, valid_ucrt_sdk_dir)
 @imports(_from='os', _import='environ')
 @imports('platform')
 def sdk_bin_path(valid_windows_sdk_dir, valid_ucrt_sdk_dir):
     if not valid_windows_sdk_dir:
@@ -470,17 +495,17 @@ def linker_progs_for(host_or_target):
             return ('lld-link', 'link')
     return linker_progs
 
 
 link = check_prog('LINKER', linker_progs_for(target),
                   paths=toolchain_search_path)
 
 host_link = check_prog('HOST_LINKER', linker_progs_for(host),
-                       paths=toolchain_search_path)
+                       paths=host_toolchain_search_path)
 
 add_old_configure_assignment('LINKER', link)
 
 
 check_prog('MAKECAB', ('makecab.exe',))
 
 
 @depends(c_compiler, using_sccache)
--- a/config/rules.mk
+++ b/config/rules.mk
@@ -577,17 +577,17 @@ ifdef ENABLE_STRIP
 endif
 ifdef MOZ_POST_PROGRAM_COMMAND
 	$(MOZ_POST_PROGRAM_COMMAND) $@
 endif
 
 $(HOST_PROGRAM): $(HOST_PROGOBJS) $(HOST_LIBS) $(HOST_EXTRA_DEPS) $(GLOBAL_DEPS) $(call mkdir_deps,$(DEPTH)/dist/host/bin)
 	$(REPORT_BUILD)
 ifeq (_WINNT,$(GNU_CC)_$(HOST_OS_ARCH))
-	$(HOST_LINKER) -NOLOGO -OUT:$@ -PDB:$(HOST_PDBFILE) $($(notdir $@)_OBJS) $(WIN32_EXE_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
+	$(HOST_LINKER) -NOLOGO -OUT:$@ -PDB:$(HOST_PDBFILE) $($(notdir $@)_OBJS) $(WIN32_EXE_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LINKER_LIBPATHS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
 ifdef MSMANIFEST_TOOL
 	@if test -f $@.manifest; then \
 		if test -f '$(srcdir)/$(notdir $@).manifest'; then \
 			echo 'Embedding manifest from $(srcdir_rel)/$(notdir $@).manifest and $@.manifest'; \
 			$(MT) -NOLOGO -MANIFEST '$(srcdir_rel)/$(notdir $@).manifest' $@.manifest -OUTPUTRESOURCE:$@\;1; \
 		else \
 			echo 'Embedding manifest from $@.manifest'; \
 			$(MT) -NOLOGO -MANIFEST $@.manifest -OUTPUTRESOURCE:$@\;1; \
@@ -638,17 +638,17 @@ ifdef ENABLE_STRIP
 endif
 ifdef MOZ_POST_PROGRAM_COMMAND
 	$(MOZ_POST_PROGRAM_COMMAND) $@
 endif
 
 $(HOST_SIMPLE_PROGRAMS): host_%$(HOST_BIN_SUFFIX): $(HOST_LIBS) $(HOST_EXTRA_DEPS) $(GLOBAL_DEPS)
 	$(REPORT_BUILD)
 ifeq (WINNT_,$(HOST_OS_ARCH)_$(GNU_CC))
-	$(HOST_LINKER) -NOLOGO -OUT:$@ -PDB:$(HOST_PDBFILE) $($(notdir $@)_OBJS) $(WIN32_EXE_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
+	$(HOST_LINKER) -NOLOGO -OUT:$@ -PDB:$(HOST_PDBFILE) $($(notdir $@)_OBJS) $(WIN32_EXE_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LINKER_LIBPATHS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
 else
 ifneq (,$(HOST_CPPSRCS)$(USE_HOST_CXX))
 	$(HOST_CXX) $(HOST_OUTOPTION)$@ $(HOST_CXX_LDFLAGS) $($(notdir $@)_OBJS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
 else
 	$(HOST_CC) $(HOST_OUTOPTION)$@ $(HOST_C_LDFLAGS) $($(notdir $@)_OBJS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
 endif
 endif
 ifndef CROSS_COMPILE
@@ -670,17 +670,17 @@ ifeq ($(OS_ARCH),WINNT)
 # See bug 795204.
 $(IMPORT_LIBRARY): $(SHARED_LIBRARY) ;
 endif
 
 $(HOST_SHARED_LIBRARY): Makefile
 	$(REPORT_BUILD)
 	$(RM) $@
 ifdef _MSC_VER
-	$(HOST_LINKER) -NOLOGO -DLL -OUT:$@ $($(notdir $@)_OBJS) $(HOST_CXX_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
+	$(HOST_LINKER) -NOLOGO -DLL -OUT:$@ $($(notdir $@)_OBJS) $(HOST_CXX_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LINKER_LIBPATHS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
 else
 	$(HOST_CXX) $(HOST_OUTOPTION)$@ $($(notdir $@)_OBJS) $(HOST_CXX_LDFLAGS) $(HOST_LDFLAGS) $(HOST_LIBS) $(HOST_EXTRA_LIBS)
 endif
 
 # On Darwin (Mac OS X), dwarf2 debugging uses debug info left in .o files,
 # so instead of deleting .o files after repacking them into a dylib, we make
 # symlinks back to the originals. The symlinks are a no-op for stabs debugging,
 # so no need to conditionalize on OS version or debugging format.