Merged PR 18232: Update VS CMake builds and scripts

This PR updates Windows build using Visual Studio CMake compilation with Ninja. It does not affect standard VS compilation or Windows builds on Azure/GitHub CI. List of changes: - Fixed syntax in the script installing dependencies via vcpkg. - Removed installation of Protobuf (already included as a submodule) and Boost 1.72 (the previous solution no longer works with new vcpkg). - Disabled compilation of marian-server in the default setting due to Boost issues. - Disabled compilation of NCCL in the default setting due to an error (see comment in the code). - Updated vs/README.
marian-nmt · Mar 19, 2021 · 326b940 · 326b940
1 parent 272096c
commit 326b940
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 198 deletions.
diff --git a/CMakeSettings.json b/CMakeSettings.json
@@ -17,10 +17,7 @@
       // Boost and OpenSSL are required if you compile with COMPILE_SERVER=TRUE
       "BOOST_INCLUDEDIR": "",
       "BOOST_LIBRARYDIR": "",
-      "OPENSSL_ROOT_DIR": "",
-
-      // Protobuf is required if you compile with USE_SENTENCEPIECE=TRUE
-      "Protobuf_SRC_ROOT_FOLDER": ""
+      "OPENSSL_ROOT_DIR": ""
     }
   ],
   "configurations": [
@@ -44,12 +41,13 @@
         { "name": "COMPILE_CUDA:BOOL", "value": "TRUE" },
         { "name": "COMPILE_CPU:BOOL", "value": "TRUE" },
         { "name": "COMPILE_EXAMPLES:BOOL", "value": "FALSE" },
-        { "name": "COMPILE_SERVER:BOOL", "value": "TRUE" },
+        { "name": "COMPILE_SERVER:BOOL", "value": "FALSE" },
         { "name": "COMPILE_TESTS:BOOL", "value": "FALSE" },
 
         { "name": "USE_CUDNN:BOOL", "value": "FALSE" },
         { "name": "USE_FBGEMM:BOOL", "value": "TRUE" },
         { "name": "USE_MPI:BOOL", "value": "FALSE" },
+        { "name": "USE_NCCL:BOOL", "value": "FALSE" },
         { "name": "USE_SENTENCEPIECE:BOOL", "value": "TRUE" },
         { "name": "USE_STATIC_LIBS:BOOL", "value": "TRUE" }
       ]
@@ -74,12 +72,13 @@
         { "name": "COMPILE_CUDA:BOOL", "value": "TRUE" },
         { "name": "COMPILE_CPU:BOOL", "value": "TRUE" },
         { "name": "COMPILE_EXAMPLES:BOOL", "value": "FALSE" },
-        { "name": "COMPILE_SERVER:BOOL", "value": "TRUE" },
+        { "name": "COMPILE_SERVER:BOOL", "value": "FALSE" },
         { "name": "COMPILE_TESTS:BOOL", "value": "TRUE" },
 
         { "name": "USE_CUDNN:BOOL", "value": "FALSE" },
         { "name": "USE_FBGEMM:BOOL", "value": "TRUE" },
         { "name": "USE_MPI:BOOL", "value": "FALSE" },
+        { "name": "USE_NCCL:BOOL", "value": "FALSE" },
         { "name": "USE_SENTENCEPIECE:BOOL", "value": "TRUE" },
         { "name": "USE_STATIC_LIBS:BOOL", "value": "TRUE" }
       ]

diff --git a/src/3rd_party/CMakeLists.txt b/src/3rd_party/CMakeLists.txt
@@ -183,6 +183,10 @@ if(CUDA_FOUND)
       BINARY_DIR ${CMAKE_CURRENT_SOURCE_DIR}/nccl
       CONFIGURE_COMMAND ""
       BUILD_COMMAND
+        # Note: $(MAKE) here causes CMake cache generation via Ninja failing (Windows build with
+        # CMake only) because Ninja complains about unescaped $ signs. This happens only if
+        # COMPILE_CUDA=on and because of that, USE_NCCL=off is used in CMakeSettings.json for now.
+        # @TODO: find proper escaping that works for MSVC builds.
         "\$(MAKE)" -f ${CMAKE_CURRENT_SOURCE_DIR}/nccl/Makefile src.build
         BUILDDIR=${CMAKE_BINARY_DIR}/local CUDA_HOME=${CUDA_TOOLKIT_ROOT_DIR}
         CUDA8_GENCODE=${GENCODE} CXX=${CMAKE_CXX_COMPILER} CXX_FLAGS=${NCCL_FLAGS}

diff --git a/vs/CheckDeps.bat → vs/CheckOrInstallDeps.bat b/vs/CheckDeps.bat → vs/CheckOrInstallDeps.bat
@@ -1,5 +1,5 @@
 ::
-:: Usage: CheckDeps.bat
+:: Usage: CheckOrInstallDeps.bat
 ::
 :: This script is used to verify that all the dependencies required to build Marian are available.
 :: The Cuda SDK and the Intel MKL must be installed beforehand by the user.
@@ -29,7 +29,6 @@ set ROOT=%~dp0
 ::set BOOST_INCLUDEDIR=
 ::set BOOST_LIBRARYDIR=
 ::set OPENSSL_ROOT_DIR=
-::set Protobuf_SRC_ROOT_FOLDER=
 
 
 :: If all the variables are empty and vcpkg is found in a known path, the script will download and
@@ -45,7 +44,6 @@ set ROOT=%~dp0
 
 if "%BOOST_INCLUDEDIR%" == "" goto :needVcPkg
 if "%OPENSSL_ROOT_DIR%" == "" goto :needVcPkg
-if "%Protobuf_SRC_ROOT_FOLDER%"=="" goto :needVcPkg
 
 goto :checkDeps
 
@@ -62,39 +60,22 @@ if "%VCPKG_ROOT%" == "" for /f "delims=" %%p in ('where vcpkg 2^>nul') do set VC
 if "%VCPKG_ROOT%" == "" set VCPKG_ROOT=%ROOT%deps\vcpkg
 
 if not exist %VCPKG_ROOT% (
-
     echo --- Cloning vcpkg...
     git clone https://github.com/Microsoft/vcpkg.git %VCPKG_ROOT%
-
-    set USE_BOOST_172=1
     set BOOTSTRAP_VCPKG=1
-
 ) else (
-
     pushd %VCPKG_ROOT%
 
     echo --- Updating vcpkg...
     for /f "delims=" %%p in ('git pull') do (
         if not "%%p" == "Already up to date." (
-            set USE_BOOST_172=1
             set BOOTSTRAP_VCPKG=1
         )
     )
 
     popd
 )
 
-:: Checkout to the last version of Boost that is supported by the 3rd party library
-:: SimpleWebSocketServer. Vcpkg does not allow installing a specific version of library yet, but
-:: the feature has a PR and should be available soon.
-:: For more details, see https://github.com/microsoft/vcpkg/issues/1681
-if "%USE_BOOST_172%"=="1" (
-    pushd %VCPKG_ROOT%
-    echo --- Checkout to Boost version 1.72...
-    git checkout 597038559647776ee39d02dcf159da05d9342f1d --pathspec-from-file=../../pathspec-boost-1.72.txt
-    popd
-)
-
 if "%BOOTSTRAP_VCPKG%"=="1" (
     pushd %VCPKG_ROOT%
     call bootstrap-vcpkg.bat
@@ -124,8 +105,7 @@ echo.
 echo ... CUDA
 if "%CUDA_PATH%"=="" (
     echo The CUDA_PATH environment variable is not defined: this will compile only the CPU version.
-)
-else (
+) else (
     echo Found Cuda SDK in %CUDA_PATH%
 )
 
@@ -211,17 +191,6 @@ if "%OPENSSL_ROOT_DIR%"=="" (
     set OPENSSL_ROOT_DIR=%VCPKG_INSTALL%
 )
 
-:: -------------------------
-:: Protobuf_SRC_ROOT_FOLDER can be set to an existing Protobuf installation.
-:: If not, we use vcpkg to install the library
-::
-echo.
-echo ... Protobuf
-if "%Protobuf_SRC_ROOT_FOLDER%"=="" (
-    %VCPKG% install protobuf
-    set Protobuf_SRC_ROOT_FOLDER=%VCPKG_INSTALL%
-)
-
 set CMAKE_PREFIX_PATH=%VCPKG_INSTALL%
 
 echo.
@@ -233,7 +202,6 @@ echo        CMAKE_PREFIX_PATH ^| %CMAKE_PREFIX_PATH%
 echo                CUDA_PATH ^| %CUDA_PATH%
 echo                  MKLROOT ^| %MKLROOT%
 echo         OPENSSL_ROOT_DIR ^| %OPENSSL_ROOT_DIR%
-echo Protobuf_SRC_ROOT_FOLDER ^| %Protobuf_SRC_ROOT_FOLDER%
 echo               VCPKG_ROOT ^| %VCPKG_ROOT%
 echo --------------------------------------------------
 echo.

diff --git a/vs/CreateVSProjects.bat b/vs/CreateVSProjects.bat
@@ -25,7 +25,7 @@ if "%BUILD_ROOT%"=="" set BUILD_ROOT=%ROOT%build
 
 set GENERATOR_TARGET="Visual Studio 15 2017 Win64"
 
-call CheckDeps.bat
+call CheckOrInstallDeps.bat
 if errorlevel 1 exit /b 1
 
 
@@ -79,4 +79,4 @@ cmake %CMAKE_OPT% %MARIAN_ROOT%
 
 popd
 
-exit /b 0
+exit /b 0
diff --git a/vs/README.md b/vs/README.md
@@ -8,23 +8,22 @@ installed. If only CUDA is installed but not MKL, a GPU-only version will be bui
 installed and not CUDA, only the CPU version will be built. So if you are interested in only one
 functionality, you can omit one of them. Install both for full functionality.
 
-   - [Cuda 10](https://developer.nvidia.com/cuda-downloads?target_os=Windows&target_arch=x86_64&target_version=10&target_type=exelocal),
+   - [CUDA](https://developer.nvidia.com/cuda-downloads?target_os=Windows&target_arch=x86_64&target_version=10&target_type=exelocal),
      Base installer, CUDA 10.0+ is recommended, there might be issues with CUDA 9.2, see below
    - [Intel MKL](https://software.intel.com/en-us/mkl)
 
 ---
-## Check dependencies : `CheckDeps.bat`
+## Check dependencies : `CheckOrInstallDeps.bat`
 
 In addition to the 2 previous prerequisites, Marian may need the following libraries that you may
 already have on your system:
 
     - Boost (1.58-1.72), optional for marian-server (`COMPILE_SERVER=TRUE` in CMake)
     - OpenSSL, optional for marian-server
-    - Protobuf, optional for compiling with SentencePiece (`USE_SENTENCEPIECE=TRUE` in CMake),
-      recommended
 
-The script `CheckDeps.bat` can be used to verify that all dependencies are found on your system. If
-not, it will use the `vcpkg` library manager to download and manage your dependencies for CMake.
+The script `CheckOrInstallDeps.bat` can be used to verify that all dependencies are found on your
+system. If not, it will use the `vcpkg` library manager to download and manage your dependencies for
+CMake.
 
 If you already have a working `vcpkg` installation, this script can use it.
 If vcpkg is in your `PATH` environment variable, the script will find it and use it automatically.
@@ -67,9 +66,8 @@ configuration is done in 3 different files:
      Use this file to configure the environment variables and the parameters passed to CMake to
      generate the project.
 
-   - `.vs\launch.vs.json`: this is a user specific file and it is not commited in the Git repo.
-     Use this file to configure the debugging targets.
-     For example:
+   - `.vs\launch.vs.json`: this is an optional user specific file and it is not commited in the Git
+     repo. Use this file to configure the debugging targets. For example:
 
          {
              "version": "0.2.1",
@@ -152,11 +150,10 @@ The last alternative is to use the script `BuildRelease.bat` that will:
 
 2. __It does not compile with Boost 1.73 or newer__
 
-    At the moment (version 1.9.26) SimpleWebSocketServer, a 3rd party library that Marian uses for
-    marian-server, does not support Boost newer than 1.72. Since vcpkg does not allow installing a
-    specific library versions, you need to revert `ports/boost*` directories to install older Boost.
-    See `CheckDeps.bat` for an example.
+    It may happen that SimpleWebSocketServer, a 3rd party library that Marian uses for
+    marian-server, does not support the version of Boost available in vcpkg. In such case install a
+    supported version of Boost; if you use vcpkg, an option is to checkout to #5970385, which has
+    Boost 1.72.
 
     Note that Boost is required only if you compile with marian-server, for compilation using CMake,
     it is if you set `COMPILE_SERVER` to `TRUE` in CMakeSettings.json.
-
diff --git a/vs/pathspec-boost-1.72.txt b/vs/pathspec-boost-1.72.txt