From 33e3c9ac679d95f28b2486ceea14188caa191d5c Mon Sep 17 00:00:00 2001 From: Huy Do Date: Wed, 11 Jan 2023 22:28:08 +0000 Subject: [PATCH] Not explicitly set the manifest filename in Windows (#91988) I'm at a loss to explain why this happens, but not setting the manifest file explicitly in the linker fixes it. ### Testing locally * With `/MANIFESTFILE:bin\torch_python.dll.manifest` ``` C:\PROGRA~2\MICROS~2\2019\BUILDT~1\VC\Tools\MSVC\1428~1.293\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\torch_python.rsp /out:bin\torch_python.dll /implib:lib\torch_python.lib /pdb:bin\torch_python.pdb /dll /version:0.0 /machine:x64 /ignore:4049 /ignore:4217 /ignore:4099 /INCREMENTAL:NO /NODEFAULTLIB:LIBCMT.LIB -WHOLEARCHIVE:C:/actions-runner/_work/pytorch/pytorch/build/lib/onnx.lib /MANIFEST /MANIFESTFILE:bin\torch_python.dll.manifest LINK : fatal error LNK1000: Internal error during CImplib::EmitImportThunk ``` * Work fine without the flag ``` C:\PROGRA~2\MICROS~2\2019\BUILDT~1\VC\Tools\MSVC\1428~1.293\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\torch_python.rsp /out:bin\torch_python.dll /implib:lib\torch_python.lib /pdb:bin\torch_python.pdb /dll /version:0.0 /machine:x64 /ignore:4049 /ignore:4217 /ignore:4099 /INCREMENTAL:NO /NODEFAULTLIB:LIBCMT.LIB -WHOLEARCHIVE:C:/actions-runner/_work/pytorch/pytorch/build/lib/onnx.lib /MANIFEST ``` In both case, the `/MANIFEST` flag is set, so the manifest file is there. In the latter case, the filename comes by appending `.manifest` suffix to `bin\torch_python.dll`. Thus, it's still correctly be `bin\torch_python.dll.manifest`. Weird. ``` C:\actions-runner\_work\pytorch\pytorch>ls -la build/bin/torch_* -rwxr-xr-x 1 runneruser 197121 246796288 Jan 11 04:30 build/bin/torch_cpu.dll -rw-r--r-- 1 runneruser 197121 381 Jan 11 04:26 build/bin/torch_cpu.dll.manifest -rwxr-xr-x 1 runneruser 197121 9728 Jan 11 03:55 build/bin/torch_global_deps.dll -rw-r--r-- 1 runneruser 197121 381 Jan 11 03:55 build/bin/torch_global_deps.dll.manifest -rwxr-xr-x 1 runneruser 197121 11746816 Jan 11 04:31 build/bin/torch_python.dll -rw-r--r-- 1 runneruser 197121 381 Jan 11 04:30 build/bin/torch_python.dll.manifest ``` Pull Request resolved: https://github.com/pytorch/pytorch/pull/91988 Approved by: https://github.com/malfet, https://github.com/Blackhex, https://github.com/ZainRizvi --- .github/workflows/pull.yml | 2 -- CMakeLists.txt | 10 ++++++++++ c10/util/env.h | 7 +++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull.yml b/.github/workflows/pull.yml index 83c13cc61c7..581886750bf 100644 --- a/.github/workflows/pull.yml +++ b/.github/workflows/pull.yml @@ -230,7 +230,6 @@ jobs: test-matrix: ${{ needs.linux-bionic-py3_7-clang8-xla-build.outputs.test-matrix }} win-vs2019-cpu-py3-build: - if: false name: win-vs2019-cpu-py3 uses: ./.github/workflows/_win-build.yml with: @@ -244,7 +243,6 @@ jobs: ]} win-vs2019-cpu-py3-test: - if: false name: win-vs2019-cpu-py3 uses: ./.github/workflows/_win-test.yml needs: win-vs2019-cpu-py3-build diff --git a/CMakeLists.txt b/CMakeLists.txt index 53aff333bfd..0ef284063af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -529,6 +529,16 @@ if(MSVC) string(APPEND ${flag_var} " /ignore:4049 /ignore:4217 /ignore:4099") endforeach(flag_var) + foreach(flag_var + CMAKE_SHARED_LINKER_FLAGS) + # https://github.com/pytorch/pytorch/issues/91933: Don't set the manifest filename + # explicitly helps fix the linker error when linking torch_python.dll. The manifest + # file would still be there in the correct format torch_python.dll.manifest + if(${flag_var} MATCHES "/MANIFESTFILE:.*\\.manifest") + string(REGEX REPLACE "/MANIFESTFILE:.*\\.manifest" "" ${flag_var} "${${flag_var}}") + endif() + endforeach(flag_var) + # Try harder string(APPEND CMAKE_CUDA_FLAGS " -Xcompiler /w -w") diff --git a/c10/util/env.h b/c10/util/env.h index ec489034825..e354b28d9c8 100644 --- a/c10/util/env.h +++ b/c10/util/env.h @@ -14,7 +14,14 @@ namespace utils { // NB: // Issues a warning if the value of the environment variable is not 0 or 1. inline optional check_env(const char* name) { +#ifdef _MSC_VER +#pragma warning(push) +#pragma warning(disable : 4996) +#endif auto envar = std::getenv(name); +#ifdef _MSC_VER +#pragma warning(pop) +#endif if (envar) { if (strcmp(envar, "0") == 0) { return false;