Fix windows conda build (onnx#2452)

* fix build issue for windows conda env * Plus few typo fixes * add option to turn of use of static runtimes + fix appveyor ci * Fix Appveyor build for Miniconda35-x64 * address review comments * updates to build instructions
aipul · Nov 15, 2019 · 748d81b · 748d81b
1 parent a32db1c
commit 748d81b
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 17 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -16,6 +16,7 @@ cmake_policy(SET CMP0063 NEW)
 # Project
 project(onnx C CXX)
 option(ONNX_BUILD_BENCHMARKS "Build ONNX micro-benchmarks" OFF)
+option(ONNX_USE_PROTOBUF_SHARED_LIBS "Build ONNX using protobuf shared library. Sets PROTOBUF_USE_DLLS CMAKE Flag " OFF)
 
 option(BUILD_ONNX_PYTHON "Build Python binaries" OFF)
 option(ONNX_GEN_PB_TYPE_STUBS "Generate protobuf python type stubs" ON)
@@ -44,6 +45,17 @@ if(NOT DEFINED ONNX_VERIFY_PROTO3)
   option(ONNX_VERIFY_PROTO3 "Generate code by proto3" ${PROTO3_ENABLED})
 endif()
 
+ if (ONNX_USE_PROTOBUF_SHARED_LIBS)
+  if(MSVC AND ONNX_USE_MSVC_STATIC_RUNTIME)
+    add_definitions(-DPROTOBUF_USE_DLLS)
+  else()
+    message(
+      FATAL_ERROR 
+        "Building ONNX with Protobuf as shared lib is only available for MSVC compilers and that too when USE_MSVC_STATIC_RUNTIME environment variable is set to 1. Please read the build instructions for more details."
+    )
+  endif()
+endif()
+
 # Set C++11 as standard for the whole project
 if(NOT MSVC)
   set(CMAKE_CXX_STANDARD 11)
@@ -445,7 +457,14 @@ if(BUILD_ONNX_PYTHON)
                                            # unsigned from include\google\protob
                                            # uf\wire_format_lite.h
                                    ${EXTRA_FLAGS})
-    target_compile_options(onnx_cpp2py_export PRIVATE /MT)
+    if(ONNX_USE_PROTOBUF_SHARED_LIBS)
+      target_compile_options(onnx_cpp2py_export 
+                             PRIVATE /wd4251 # 'identifier' : class 'type1' needs to 
+                                             # have dll-interface to be used by 
+                                             # clients of class 'type2'
+                            )
+    endif()
+    add_msvc_runtime_flag(onnx_cpp2py_export)
     add_onnx_global_defines(onnx_cpp2py_export)
   endif()
 endif()
@@ -503,6 +522,18 @@ if(MSVC)
                                  /wd4146 # unary minus operator applied to
                                          # unsigned type, result still unsigned
                                  ${EXTRA_FLAGS})
+  if(ONNX_USE_PROTOBUF_SHARED_LIBS)
+      target_compile_options(onnx_proto 
+                             PRIVATE /wd4251 # 'identifier' : class 'type1' needs to 
+                                             # have dll-interface to be used by 
+                                             # clients of class 'type2'
+                            )
+      target_compile_options(onnx 
+                             PRIVATE /wd4251 # 'identifier' : class 'type1' needs to 
+                                             # have dll-interface to be used by
+                                             # clients of class 'type2'
+                            )
+  endif()
   add_msvc_runtime_flag(onnx_proto)
   add_msvc_runtime_flag(onnx)
   set(onnx_static_library_flags

diff --git a/README.md b/README.md
@@ -60,6 +60,7 @@ conda install -c conda-forge onnx
 
 ## Source
 
+### Linux and MacOS
 You will need an install of protobuf and numpy to build ONNX.  One easy
 way to get these dependencies is via
 [Anaconda](https://www.anaconda.com/download/):
@@ -91,6 +92,65 @@ sudo apt-get install protobuf-compiler libprotoc-dev
 pip install onnx
 ```
 
+### Windows
+When building on Windows it is highly recommended that you also build protobuf locally as a static library. The version distributed with conda-forge is a DLL and this is a conflict as ONNX expects it to be a static lib.
+
+#### Instructions to build protobuf and ONNX on windows
+Step 1 : Build protobuf locally
+```
+git clone https://github.com/protocolbuffers/protobuf.git
+cd protobuf
+git checkout 3.9.x
+cd cmake
+# Explicitly set -Dprotobuf_MSVC_STATIC_RUNTIME=OFF to make sure protobuf does not statically link to runtime library
+cmake -G "Visual Studio 15 2017 Win64" -Dprotobuf_MSVC_STATIC_RUNTIME=OFF -Dprotobuf_BUILD_TESTS=OFF -Dprotobuf_BUILD_EXAMPLES=OFF -DCMAKE_INSTALL_PREFIX=<protobuf_install_dir>
+msbuild protobuf.sln /m /p:Configuration=Release
+msbuild INSTALL.vcxproj /p:Configuration=Release
+```
+
+Step 2: Build ONNX
+```
+# Get ONNX
+git clone https://github.com/onnx/onnx.git
+cd onnx
+git submodule update --init --recursive
+
+# Set environment variables to find protobuf and turn off static linking of ONNX to runtime library.
+# Even better option is to add it to user\system PATH so this step can be performed only once.
+# For more details check https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2017
+set PATH=<protobuf_install_dir>\bin;%PATH%
+set USE_MSVC_STATIC_RUNTIME=0
+
+# Optional : Set environment variable `ONNX_ML=1` for onnx-ml
+
+# Build ONNX
+python setup.py install
+```
+
+If you do not want to build protobuf and instead want to use protobuf from conda forge then follow these instructions. 
+However please note : This method is just added as a convenience for users and there is very limited support from ONNX team when using this method.
+
+#### Instructions to build ONNX on windows in anaconda environment
+
+```
+# Use conda-forge protobuf
+conda install -c conda-forge protobuf=3.9.2 numpy
+
+# Get ONNX
+git clone https://github.com/onnx/onnx.git
+cd onnx
+git submodule update --init --recursive
+
+# Set environment variable for ONNX to use protobuf shared lib
+set CMAKE_ARGS="-DONNX_USE_PROTOBUF_SHARED_LIBS=ON"
+
+# Build ONNX
+# Optional : Set environment variable `ONNX_ML=1` for onnx-ml
+
+python setup.py install
+```
+
+## Verify Installation
 After installation, run
 
 ```

diff --git a/appveyor.yml b/appveyor.yml
@@ -37,22 +37,20 @@ install:
 before_build:
 - cmd: SET PATH=%CONDA_PREFIX%;%CONDA_PREFIX%\Scripts;%PATH%
 - cmd: SET _prefix=%CONDA_PREFIX:~0,14%
+- cmd: SET _arch=%CONDA_PREFIX:~15,18%
 - >
-  IF "%_prefix%"=="C:\Miniconda37"
+  IF "%_arch%"=="x64"
   (
-  conda install -y -c conda-forge libprotobuf=3.5.2 numpy &&
-  set "PATH=%PATH%;%CONDA_PREFIX%\Library\bin"
+  conda install -y -c conda-forge libprotobuf=3.5.2=he51fdeb_1 numpy
   )
   ELSE
   (
-  IF "%CONDA_PREFIX%"=="C:\Miniconda36-x64"
-  (
-  conda install -y -c conda-forge libprotobuf=3.5.2 numpy=1.17.0
+  conda install -y -c conda-forge libprotobuf=3.5.2=vc14_0 numpy
   )
-  ELSE
+
+  IF "%_prefix%"=="C:\Miniconda37"
   (
-  conda install -y -c conda-forge libprotobuf=3.5.2 numpy
-  )
+  set "PATH=%PATH%;%CONDA_PREFIX%\Library\bin"
   )
 - cmd: pip install --quiet pytest nbval numpy
 

diff --git a/cmake/Utils.cmake b/cmake/Utils.cmake
@@ -1,12 +1,10 @@
 #
-# Add MSVC RunTime Flag, this part is necessary for tests in CI
+# Add MSVC RunTime Flag
 function(add_msvc_runtime_flag lib)
   if(${ONNX_USE_MSVC_STATIC_RUNTIME})
-    if(${CMAKE_BUILD_TYPE} MATCHES "Debug")
-      target_compile_options(${lib} PRIVATE /MTd)
-    else()
-      target_compile_options(${lib} PRIVATE /MT)
-    endif()
+    target_compile_options(${lib} PRIVATE $<$<NOT:$<CONFIG:Debug>>:/MT> $<$<CONFIG:Debug>:/MTd>)    
+  else()
+    target_compile_options(${lib} PRIVATE $<$<NOT:$<CONFIG:Debug>>:/MD> $<$<CONFIG:Debug>:/MDd>)
   endif()
 endfunction()
 

diff --git a/setup.py b/setup.py
@@ -41,6 +41,9 @@
 # Global variables for controlling the build variant
 ################################################################################
 
+# Default value is set to TRUE\1 to keep the settings same as the current ones.
+# However going forward the recomemded way to is to set this to False\0
+USE_MSVC_STATIC_RUNTIME = bool(os.getenv('USE_MSVC_STATIC_RUNTIME', '1') == '1')
 ONNX_ML = not bool(os.getenv('ONNX_ML') == '0')
 ONNX_VERIFY_PROTO3 = bool(os.getenv('ONNX_VERIFY_PROTO3') == '1')
 ONNX_NAMESPACE = os.getenv('ONNX_NAMESPACE', 'onnx')
@@ -173,8 +176,9 @@ def run(self):
                     # passing python version to window in order to
                     # find python in cmake
                     '-DPY_VERSION={}'.format('{0}.{1}'.format(*sys.version_info[:2])),
-                    '-DONNX_USE_MSVC_STATIC_RUNTIME=ON',
                 ])
+                if USE_MSVC_STATIC_RUNTIME:
+                    cmake_args.append('-DONNX_USE_MSVC_STATIC_RUNTIME=ON')
                 if 8 * struct.calcsize("P") == 64:
                     # Temp fix for CI
                     # TODO: need a better way to determine generator