Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/…

…dbg (#13133) Commit Message: Clean up build flags for clang + msvc opt/fastbuild/dbg Additional Description: - Move what were '--config=msvc-cl' settings out of .bazelrc into the default cflags for Windows compilation. This enables bazel build ... and bazel test ... to simply work out of the box. - Cleans up .bazelrc lists of windows vs. clang-cl settings, dividing the list into desired toggles vs. bazel or envoy bug workarounds - Fix errors in curl library creation in fastbuild on Windows - Introduce toggles and defaults for alwayslink argument to library objects. Defaults remain the same for now. - Introduce, but do not toggle clang-cl flags (this will arrive in a later PR when we resolve the opt build errors missing intrinsic base class destructors in the mock classes) Risk Level: low Testing: local, msvc and clang-cl on windows Docs Changes: n/a Release Notes: n/a Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]>
envoyproxy · Oct 11, 2020 · 3c86d8d · 3c86d8d
1 parent 8a1d444
commit 3c86d8d
Show file tree

Hide file tree

Showing 18 changed files with 121 additions and 26 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -19,7 +19,8 @@ build --host_javabase=@bazel_tools//tools/jdk:remote_jdk11
 build --javabase=@bazel_tools//tools/jdk:remote_jdk11
 build --enable_platform_specific_config
 
-# Enable position independent code, this option is not supported on Windows and default on on macOS.
+# Enable position independent code (this is the default on macOS and Windows)
+# (Workaround for https://github.com/bazelbuild/rules_foreign_cc/issues/421)
 build:linux --copt=-fPIC
 build:linux --cxxopt=-std=c++17
 build:linux --conlyopt=-fexceptions
@@ -287,27 +288,27 @@ build:windows --define signal_trace=disabled
 build:windows --define hot_restart=disabled
 build:windows --define tcmalloc=disabled
 build:windows --define manual_stamp=manual_stamp
+build:windows --cxxopt="/std:c++17"
 
-# Should not be required after upstream fix to bazel,
-# and already a no-op to linux/macos builds
-# see issue https://github.com/bazelbuild/rules_foreign_cc/issues/301
+# TODO(wrowe,sunjayBhatia): Resolve bugs upstream in curl and rules_foreign_cc
+# See issue https://github.com/bazelbuild/rules_foreign_cc/issues/301
 build:windows --copt="-DCARES_STATICLIB"
 build:windows --copt="-DNGHTTP2_STATICLIB"
 build:windows --copt="-DCURL_STATICLIB"
-build:windows --cxxopt="/std:c++17"
 
-# Required to work around build defects on Windows MSVC cl
-# Unguarded gcc pragmas in quiche are not recognized by MSVC
-build:msvc-cl --copt="/wd4068"
-# Allows 'nodiscard' function return values to be discarded
-build:msvc-cl --copt="/wd4834"
-# Allows inline functions to be undefined
-build:msvc-cl --copt="/wd4506"
-build:msvc-cl --copt="-D_SILENCE_EXPERIMENTAL_FILESYSTEM_DEPRECATION_WARNING"
+# Override any clang preference if building msvc-cl
+# Drop the determinism feature (-DDATE etc are a no-op in msvc-cl)
+build:msvc-cl --action_env=USE_CLANG_CL=""
+build:msvc-cl --define clang_cl=0
+build:msvc-cl --features=-determinism
+
+# Windows build behaviors when using clang-cl
+build:clang-cl --action_env=USE_CLANG_CL=1
+build:clang-cl --define clang_cl=1
 
 # Required to work around Windows clang-cl build defects
 # Ignore conflicting definitions of _WIN32_WINNT
-# Overriding __TIME__ etc is problematic (and is actually an invalid no-op)
+# Override determinism flags (DATE etc) is valid on clang-cl compiler
 build:clang-cl --copt="-Wno-macro-redefined"
 build:clang-cl --copt="-Wno-builtin-macro-redefined"
 build:clang-cl --action_env=USE_CLANG_CL=1

diff --git a/bazel/BUILD b/bazel/BUILD
@@ -84,6 +84,33 @@ config_setting(
     },
 )
 
+config_setting(
+    name = "clang_cl_opt_build",
+    values = {
+        "cpu": "x64_windows",
+        "define": "clang_cl=1",
+        "compilation_mode": "opt",
+    },
+)
+
+config_setting(
+    name = "clang_cl_dbg_build",
+    values = {
+        "cpu": "x64_windows",
+        "define": "clang_cl=1",
+        "compilation_mode": "dbg",
+    },
+)
+
+config_setting(
+    name = "clang_cl_fastbuild_build",
+    values = {
+        "cpu": "x64_windows",
+        "define": "clang_cl=1",
+        "compilation_mode": "fastbuild",
+    },
+)
+
 config_setting(
     name = "opt_build",
     values = {"compilation_mode": "opt"},

diff --git a/bazel/envoy_binary.bzl b/bazel/envoy_binary.bzl
@@ -60,8 +60,13 @@ def _envoy_linkopts():
             "-pagezero_size 10000",
             "-image_base 100000000",
         ],
+        "@envoy//bazel:clang_cl_opt_build": [
+            "-DEFAULTLIB:ws2_32.lib",
+            "-DEFAULTLIB:iphlpapi.lib",
+            "-DEBUG:FULL",
+            "-WX",
+        ],
         "@envoy//bazel:windows_x86_64": [
-            "-DEFAULTLIB:advapi32.lib",
             "-DEFAULTLIB:ws2_32.lib",
             "-DEFAULTLIB:iphlpapi.lib",
             "-WX",

diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl
@@ -33,11 +33,16 @@ def envoy_copts(repository, test = False):
         "-DNOIME",
         "-DNOCRYPT",
         # Ignore unguarded gcc pragmas in quiche (unrecognized by MSVC)
-        # TODO(wrowe): Drop this change when fixed in bazel/external/quiche.genrule_cmd
+        # TODO(wrowe,sunjayBhatia): Drop this change when fixed in bazel/external/quiche.genrule_cmd
         "-wd4068",
-        # this is to silence the incorrect MSVC compiler warning when trying to convert between
-        # std::optional data types while conversions between primitive types are producing no error
+        # Silence incorrect MSVC compiler warnings when converting between std::optional
+        # data types (while conversions between primitive types are producing no error)
         "-wd4244",
+        # Allow inline functions to be undefined
+        "-wd4506",
+        # Allow 'nodiscard' function return values to be discarded
+        # TODO(wrowe,sunjayBhatia): Drop this option when all causes are fixed
+        "-wd4834",
     ]
 
     return select({
@@ -51,6 +56,9 @@ def envoy_copts(repository, test = False):
                repository + "//bazel:windows_opt_build": [],
                repository + "//bazel:windows_fastbuild_build": [],
                repository + "//bazel:windows_dbg_build": [],
+               repository + "//bazel:clang_cl_opt_build": [] if test else ["-Z7", "-fstandalone-debug"],
+               repository + "//bazel:clang_cl_fastbuild_build": ["-fno-standalone-debug"],
+               repository + "//bazel:clang_cl_dbg_build": ["-fstandalone-debug"],
            }) + select({
                repository + "//bazel:clang_build": ["-fno-limit-debug-info", "-Wgnu-conditional-omitted-operand", "-Wc++2a-extensions", "-Wrange-loop-analysis"],
                repository + "//bazel:gcc_build": ["-Wno-maybe-uninitialized"],

diff --git a/bazel/envoy_test.bzl b/bazel/envoy_test.bzl
@@ -29,6 +29,7 @@ def _envoy_cc_test_infrastructure_library(
         tags = [],
         include_prefix = None,
         copts = [],
+        alwayslink = 1,
         **kargs):
     # Add implicit tcmalloc external dependency(if available) in order to enable CPU and heap profiling in tests.
     deps += tcmalloc_external_deps(repository)
@@ -44,7 +45,7 @@ def _envoy_cc_test_infrastructure_library(
         ],
         tags = tags,
         include_prefix = include_prefix,
-        alwayslink = 1,
+        alwayslink = alwayslink,
         linkstatic = envoy_linkstatic(),
         **kargs
     )
@@ -58,7 +59,6 @@ def _envoy_test_linkopts():
             "-image_base 100000000",
         ],
         "@envoy//bazel:windows_x86_64": [
-            "-DEFAULTLIB:advapi32.lib",
             "-DEFAULTLIB:ws2_32.lib",
             "-DEFAULTLIB:iphlpapi.lib",
             "-WX",
@@ -205,6 +205,7 @@ def envoy_cc_test_library(
         tags = [],
         include_prefix = None,
         copts = [],
+        alwayslink = 1,
         **kargs):
     deps = deps + [
         repository + "//test/test_common:printers_includes",
@@ -222,6 +223,7 @@ def envoy_cc_test_library(
         include_prefix,
         copts,
         visibility = ["//visibility:public"],
+        alwayslink = alwayslink,
         **kargs
     )
 

diff --git a/bazel/foreign_cc/BUILD b/bazel/foreign_cc/BUILD
@@ -138,11 +138,6 @@ envoy_cmake_external(
         "ZLIB_LIBRARY": "$EXT_BUILD_DEPS/zlib",
         "ZLIB_INCLUDE_DIR": "$EXT_BUILD_DEPS/zlib/include",
         "CMAKE_CXX_COMPILER_FORCED": "on",
-        "CMAKE_C_FLAGS_BAZEL": "-fPIC",
-        # Note we use Bazel's flags (not _RELEASE/_DEBUG CMake flags), but this toggle
-        # also works around a bug in CMP0091 logic which re-injected a badly placed -M flag.
-        # See https://github.com/bazelbuild/rules_foreign_cc/issues/426
-        "CURL_STATIC_CRT": "on",
     },
     defines = ["CURL_STATICLIB"],
     generate_crosstool_file = True,
@@ -351,7 +346,6 @@ envoy_cmake_external(
 envoy_cmake_external(
     name = "zlib",
     cache_entries = {
-        "BUILD_SHARED_LIBS": "off",
         "CMAKE_CXX_COMPILER_FORCED": "on",
         "CMAKE_C_COMPILER_FORCED": "on",
         "SKIP_BUILD_EXAMPLES": "on",

diff --git a/bazel/foreign_cc/curl.patch b/bazel/foreign_cc/curl.patch
@@ -0,0 +1,29 @@
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index ec1cfa782..0c5a72f00 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -42,0 +42,5 @@
++# revert CMake bug triggered by curl's defined max CMake policy version, see https://gitlab.kitware.com/cmake/cmake/-/issues/21288
++if(POLICY CMP0091)
++  cmake_policy(SET CMP0091 OLD)
++endif()
++
+@@ -249,3 +254,6 @@
+-  set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
+-  set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} /MT")
+-  set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} /MTd")
++  foreach(build_suffix "" _DEBUG _RELEASE _MINSIZEREL _RELWITHDEBINFO)
++    set(flags_var CMAKE_C_FLAGS${build_suffix})
++    if("${${flags_var}}" MATCHES "/MD")
++      string(REGEX REPLACE "/MD" "/MT" ${flags_var} "${${flags_var}}")
++    endif()
++  endforeach()
+diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
+index 911c9096d..ba6af1bf1 100644
+--- a/lib/CMakeLists.txt
++++ b/lib/CMakeLists.txt
+@@ -91,4 +91,0 @@ add_library(
+-if(MSVC AND NOT BUILD_SHARED_LIBS)
+-  set_target_properties(${LIB_NAME} PROPERTIES STATIC_LIBRARY_FLAGS ${CMAKE_EXE_LINKER_FLAGS})
+-endif()
+-
diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl
@@ -739,6 +739,16 @@ def _com_github_curl():
         build_file_content = BUILD_ALL_CONTENT + """
 cc_library(name = "curl", visibility = ["//visibility:public"], deps = ["@envoy//bazel/foreign_cc:curl"])
 """,
+        # Patch curl 7.72.0 due to CMake's problematic implementation of policy `CMP0091`
+        # introduced in CMake 3.15 and then deprecated in CMake 3.18. Curl forcing the CMake
+        # ruleset to 3.16 breaks the Envoy windows fastbuild target.
+        # Also cure a fatal assumption creating a static library using LLVM `lld-link.exe`
+        # adding dynamic link flags, which breaks the Envoy clang-cl library archive step.
+        # Upstream patch submitted: https://github.com/curl/curl/pull/6050
+        # TODO(https://github.com/envoyproxy/envoy/issues/11816): This patch is obsoleted
+        # by elimination of the curl dependency.
+        patches = ["@envoy//bazel/foreign_cc:curl.patch"],
+        patch_args = ["-p1"],
         **location
     )
     native.bind(

diff --git a/test/exe/BUILD b/test/exe/BUILD
@@ -63,6 +63,8 @@ envoy_cc_test(
     name = "main_common_test",
     srcs = ["main_common_test.cc"],
     data = ["//test/config/integration:google_com_proxy_port_0"],
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl fastbuild TIMEOUT flake
+    tags = ["flaky_on_windows"],
     deps = [
         "//source/common/api:api_lib",
         "//source/exe:main_common_lib",

diff --git a/test/extensions/filters/http/adaptive_concurrency/BUILD b/test/extensions/filters/http/adaptive_concurrency/BUILD
@@ -34,6 +34,8 @@ envoy_extension_cc_test(
         "adaptive_concurrency_filter_integration_test.h",
     ],
     extension_name = "envoy.filters.http.adaptive_concurrency",
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl build test failure
+    tags = ["fails_on_windows"],
     deps = [
         "//source/extensions/filters/http/adaptive_concurrency:config",
         "//source/extensions/filters/http/fault:config",

diff --git a/test/extensions/filters/http/common/compressor/BUILD b/test/extensions/filters/http/common/compressor/BUILD
@@ -49,4 +49,6 @@ envoy_cc_benchmark_binary(
 envoy_benchmark_test(
     name = "compressor_filter_speed_test_benchmark_test",
     benchmark_binary = "compressor_filter_speed_test",
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl build test failure
+    tags = ["fails_on_windows"],
 )
diff --git a/test/extensions/filters/http/grpc_json_transcoder/BUILD b/test/extensions/filters/http/grpc_json_transcoder/BUILD
@@ -60,6 +60,8 @@ envoy_extension_cc_test(
         "//test/proto:bookstore_proto_descriptor",
     ],
     extension_name = "envoy.filters.http.grpc_json_transcoder",
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl build test failure
+    tags = ["fails_on_windows"],
     deps = [
         "//source/common/grpc:codec_lib",
         "//source/common/http:header_map_lib",

diff --git a/test/extensions/filters/http/jwt_authn/BUILD b/test/extensions/filters/http/jwt_authn/BUILD
@@ -149,6 +149,8 @@ envoy_extension_cc_test(
     name = "group_verifier_test",
     srcs = ["group_verifier_test.cc"],
     extension_name = "envoy.filters.http.jwt_authn",
+    # TODO(envoyproxy/windows-dev): Diagnose msvc-cl fastbuild test failure
+    tags = ["fails_on_windows"],
     deps = [
         ":mock_lib",
         ":test_common_lib",

diff --git a/test/extensions/filters/http/lua/BUILD b/test/extensions/filters/http/lua/BUILD
@@ -15,6 +15,8 @@ envoy_extension_cc_test(
     name = "lua_filter_test",
     srcs = ["lua_filter_test.cc"],
     extension_name = "envoy.filters.http.lua",
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl build test failure
+    tags = ["fails_on_windows"],
     deps = [
         "//source/common/stream_info:stream_info_lib",
         "//source/extensions/filters/http/lua:lua_filter_lib",

diff --git a/test/extensions/filters/network/sni_dynamic_forward_proxy/BUILD b/test/extensions/filters/network/sni_dynamic_forward_proxy/BUILD
@@ -33,6 +33,8 @@ envoy_extension_cc_test(
         "//test/config/integration/certs",
     ],
     extension_name = "envoy.filters.network.sni_dynamic_forward_proxy",
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl build test failure
+    tags = ["fails_on_windows"],
     deps = [
         "//source/extensions/clusters/dynamic_forward_proxy:cluster",
         "//source/extensions/filters/listener/tls_inspector:config",

diff --git a/test/extensions/quic_listeners/quiche/integration/BUILD b/test/extensions/quic_listeners/quiche/integration/BUILD
@@ -13,6 +13,7 @@ envoy_cc_test(
     size = "medium",
     srcs = ["quic_http_integration_test.cc"],
     data = ["//test/config/integration/certs"],
+    # TODO(envoyproxy/windows-dev): diagnose msvc-cl build test failure
     tags = [
         "fails_on_windows",
         "nofips",

diff --git a/test/extensions/stats_sinks/hystrix/BUILD b/test/extensions/stats_sinks/hystrix/BUILD
@@ -31,6 +31,8 @@ envoy_extension_cc_test(
     name = "hystrix_test",
     srcs = ["hystrix_test.cc"],
     extension_name = "envoy.stat_sinks.hystrix",
+    # TODO(envoyproxy/windows-dev): Diagnose msvc-cl fastbuild test failure
+    tags = ["fails_on_windows"],
     deps = [
         "//source/common/json:json_loader_lib",
         "//source/common/stats:stats_lib",

diff --git a/test/server/BUILD b/test/server/BUILD
@@ -358,6 +358,8 @@ envoy_cc_test(
         ":server_test_data",
         ":static_validation_test_data",
     ],
+    # TODO(envoyproxy/windows-dev): diagnose clang-cl build test timeout
+    tags = ["fails_on_windows"],
     deps = [
         "//source/common/version:version_lib",
         "//source/extensions/access_loggers/file:config",