From 75cc994e0ab86ebd27222ce71a842d8aa6b924d6 Mon Sep 17 00:00:00 2001 From: mehrdadn Date: Sun, 1 Dec 2019 15:05:50 -0800 Subject: [PATCH] Update various build options relating to Windows (#6315) * Update .bazelrc for Windows compatibility * Block inclusion of (legacy) WinSock.h to avoid errors * Suppress warnings for Windows code * Include boost::asio in includes so that it is passed as -isystem to avoid warnings * Link with -lpthread only on non-Windows * Undefine BOOST_FALLTHROUGH, which is unnecessary and causes macro redefinition warnings * Define RAY_STATIC and ARROW_STATIC to compile for Windows * Add WinSock import library for Arrow --- .bazelrc | 11 ++++--- BUILD.bazel | 30 +++++++++++++++++-- bazel/BUILD.plasma | 20 ++++++++++++- bazel/ray_deps_setup.bzl | 4 +++ src/ray/object_manager/object_manager.h | 4 +-- src/ray/rpc/worker/core_worker_client.h | 2 +- src/ray/util/visibility.h | 4 ++- ...t-exception-no_warn_typeid_evaluated.patch | 11 +++++++ ...les_boost-undefine-boost_fallthrough.patch | 9 ++++++ 9 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 thirdparty/patches/boost-exception-no_warn_typeid_evaluated.patch create mode 100644 thirdparty/patches/rules_boost-undefine-boost_fallthrough.patch diff --git a/.bazelrc b/.bazelrc index 7f5db6745..1dcb8f1f2 100644 --- a/.bazelrc +++ b/.bazelrc @@ -9,17 +9,20 @@ build --action_env=PYTHON3_BIN_PATH build --action_env=USE_CLANG_CL=1 # Enable build:windows, build:linux, build:macos, build:freebsd build --enable_platform_specific_config +# TODO(mehrdadn): Revert the "-\\.(asm|S)$" exclusion when this Bazel bug +# for compiling assembly files is fixed on Windows: +# https://github.com/bazelbuild/bazel/issues/8924 # Warnings should be errors -build --per_file_copt=-.*/arrow/util/logging.cc@-Werror +build --per_file_copt="-\\.(asm|S)$,-.*/arrow/util/logging\\.cc@-Werror" # Ignore warnings for protobuf generated files and external projects. -build --per_file_copt='\\.pb\\.cc$@-w' -build --per_file_copt='external/.*@-w' +build --per_file_copt="-\\.(asm|S)$,\\.pb\\.cc$@-w" +build --per_file_copt="-\\.(asm|S)$,external/.*@-w" # Ignore minor warnings for host tools, which we generally can't control build --host_copt="-Wno-builtin-macro-redefined" build --host_copt="-Wno-inconsistent-missing-override" build --host_copt="-Wno-microsoft-unqualified-friend" # This workaround is needed due to https://github.com/bazelbuild/bazel/issues/4341 -build --per_file_copt="external/com_github_grpc_grpc/.*@-DGRPC_BAZEL_BUILD" +build --per_file_copt="-\\.(asm|S)$,external/com_github_grpc_grpc/.*@-DGRPC_BAZEL_BUILD" build --http_timeout_scaling=5.0 # This workaround is due to an incompatibility of # bazel_common/tools/maven/pom_file.bzl with Bazel 1.0 diff --git a/BUILD.bazel b/BUILD.bazel index 6ba838936..15cc7c2a0 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -8,10 +8,15 @@ load("@com_github_grpc_grpc//bazel:cython_library.bzl", "pyx_library") load("@rules_proto_grpc//python:defs.bzl", "python_grpc_compile") load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library") +# TODO(mehrdadn): (How to) support dynamic linking? +PROPAGATED_WINDOWS_DEFINES = ["RAY_STATIC"] + COPTS = ["-DRAY_USE_GLOG"] + select({ "@bazel_tools//src/conditions:windows": [ + "-DWIN32_LEAN_AND_MEAN=", # Block the inclusion of WinSock.h, which is obsolete and causes errors "-Wno-builtin-macro-redefined", # To get rid of warnings caused by deterministic build macros (e.g. #define __DATE__ "redacted") - ], + "-Wno-microsoft-unqualified-friend", # This shouldn't normally be enabled, but otherwise we get: google/protobuf/map_field.h: warning: unqualified friend declaration referring to type outside of the nearest enclosing namespace is a Microsoft extension; add a nested name specifier (for: friend class DynamicMessage) + ] + ["-D" + define for define in PROPAGATED_WINDOWS_DEFINES], "//conditions:default": [ ], }) @@ -211,6 +216,9 @@ cc_library( ], ), copts = COPTS, + includes = [ + "@boost//:asio", + ], deps = [ ":common_cc_proto", ":gcs_cc_proto", @@ -270,7 +278,13 @@ cc_library( includes = [ "src", ], - linkopts = ["-pthread"], + linkopts = select({ + "@bazel_tools//src/conditions:windows": [ + ], + "//conditions:default": [ + "-lpthread", + ], + }), deps = [ ":ray_util", "@com_github_jupp0r_prometheus_cpp//pull", @@ -302,7 +316,17 @@ cc_library( "src/ray/raylet/*.h", ]), copts = COPTS, - linkopts = ["-pthread"], + defines = select({ + "@bazel_tools//src/conditions:windows": PROPAGATED_WINDOWS_DEFINES, + "//conditions:default": [], + }), + linkopts = select({ + "@bazel_tools//src/conditions:windows": [ + ], + "//conditions:default": [ + "-lpthread", + ], + }), deps = [ ":common_cc_proto", ":gcs", diff --git a/bazel/BUILD.plasma b/bazel/BUILD.plasma index f030b5dc7..3f4a0e421 100644 --- a/bazel/BUILD.plasma +++ b/bazel/BUILD.plasma @@ -1,13 +1,24 @@ load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library") +# TODO(mehrdadn): (How to) support dynamic linking? +PROPAGATED_WINDOWS_DEFINES = ["ARROW_STATIC"] + COPTS = [] + select({ "@bazel_tools//src/conditions:windows": [ - ], + ] + ["-D" + define for define in PROPAGATED_WINDOWS_DEFINES], "//conditions:default": [ "-DARROW_USE_GLOG", ], }) +LINKOPTS = [] + select({ + "@bazel_tools//src/conditions:windows": [ + "-DefaultLib:" + "ws2_32.lib", + ], + "//conditions:default": [ + ], +}) + cc_library( name = "arrow", srcs = [ @@ -60,6 +71,7 @@ cc_library( "cpp/src/arrow/vendored/xxhash/xxhash.h", ], copts = COPTS, + linkopts = LINKOPTS, strip_include_prefix = "cpp/src", deps = [ "@boost//:filesystem", @@ -92,6 +104,11 @@ cc_library( "cpp/src/plasma/protocol.h", ], copts = COPTS, + defines = select({ + "@bazel_tools//src/conditions:windows": PROPAGATED_WINDOWS_DEFINES, + "//conditions:default": [], + }), + linkopts = LINKOPTS, strip_include_prefix = "cpp/src", visibility = ["//visibility:public"], deps = [ @@ -173,6 +190,7 @@ cc_library( "cpp/src/plasma/thirdparty/dlmalloc.c", ], copts = COPTS, + linkopts = LINKOPTS, strip_include_prefix = "cpp/src", deps = [ ":plasma_client", diff --git a/bazel/ray_deps_setup.bzl b/bazel/ray_deps_setup.bzl index 61e580ebd..38e661f0b 100644 --- a/bazel/ray_deps_setup.bzl +++ b/bazel/ray_deps_setup.bzl @@ -111,6 +111,7 @@ def ray_deps_setup(): strip_prefix = "boost_1_68_0", url = "https://dl.bintray.com/boostorg/release/1.68.0/source/boost_1_68_0.tar.gz", patches = [ + "//thirdparty/patches:boost-exception-no_warn_typeid_evaluated.patch", # Backport Clang-Cl patch on Boost 1.69 to Boost <= 1.68: # https://lists.boost.org/Archives/boost/2018/09/243420.php "//thirdparty/patches:boost-type_traits-trivial_move.patch", @@ -123,6 +124,9 @@ def ray_deps_setup(): commit = "df908358c605a7d5b8bbacde07afbaede5ac12cf", remote = "https://github.com/nelhage/rules_boost", sha256 = "3775c5ab217e0c9cc380f56e243a4d75fe6fee8eaee1447899eaa04c5d582cf1", + patches = [ + "//thirdparty/patches:rules_boost-undefine-boost_fallthrough.patch", + ], ) github_repository( diff --git a/src/ray/object_manager/object_manager.h b/src/ray/object_manager/object_manager.h index 12a80082e..3a5404010 100644 --- a/src/ray/object_manager/object_manager.h +++ b/src/ray/object_manager/object_manager.h @@ -189,7 +189,7 @@ class ObjectManager : public ObjectManagerInterface, /// /// \param object_id The object's object id. /// \return Status of whether the pull request successfully initiated. - ray::Status Pull(const ObjectID &object_id); + ray::Status Pull(const ObjectID &object_id) override; /// Try to Pull an object from one of its expected client locations. If there /// are more client locations to try after this attempt, then this method @@ -207,7 +207,7 @@ class ObjectManager : public ObjectManagerInterface, /// /// \param object_id The ObjectID. /// \return Void. - void CancelPull(const ObjectID &object_id); + void CancelPull(const ObjectID &object_id) override; /// Callback definition for wait. using WaitCallback = std::function &found, diff --git a/src/ray/rpc/worker/core_worker_client.h b/src/ray/rpc/worker/core_worker_client.h index cf1a0e9d0..ab70a6998 100644 --- a/src/ray/rpc/worker/core_worker_client.h +++ b/src/ray/rpc/worker/core_worker_client.h @@ -157,7 +157,7 @@ class CoreWorkerClient : public std::enable_shared_from_this, virtual ray::Status GetObjectStatus( const GetObjectStatusRequest &request, - const ClientCallback &callback) { + const ClientCallback &callback) override { auto call = client_call_manager_.CreateCall( *stub_, &CoreWorkerService::Stub::PrepareAsyncGetObjectStatus, request, callback); diff --git a/src/ray/util/visibility.h b/src/ray/util/visibility.h index fa04ac3b2..2f286f6c1 100644 --- a/src/ray/util/visibility.h +++ b/src/ray/util/visibility.h @@ -8,7 +8,9 @@ #pragma GCC diagnostic ignored "-Wattributes" #endif -#ifdef RAY_EXPORTING +#ifdef RAY_STATIC +#define RAY_EXPORT +#elif defined(RAY_EXPORTING) #define RAY_EXPORT __declspec(dllexport) #else #define RAY_EXPORT __declspec(dllimport) diff --git a/thirdparty/patches/boost-exception-no_warn_typeid_evaluated.patch b/thirdparty/patches/boost-exception-no_warn_typeid_evaluated.patch new file mode 100644 index 000000000..f126c7f44 --- /dev/null +++ b/thirdparty/patches/boost-exception-no_warn_typeid_evaluated.patch @@ -0,0 +1,11 @@ +diff --git boost/exception/info.hpp boost/exception/info.hpp +index f7ac50e..9a18650 100644 +--- boost/exception/info.hpp ++++ boost/exception/info.hpp +@@ -84,3 +84,4 @@ boost + #ifndef BOOST_NO_RTTI +- BOOST_ASSERT( *BOOST_EXCEPTION_DYNAMIC_TYPEID(*p).type_==*ti.type_ ); ++ error_info_base &target = *p; (void)target; // Avoid -Wpotentially-evaluated-expression ++ BOOST_ASSERT( *BOOST_EXCEPTION_DYNAMIC_TYPEID(target).type_==*ti.type_ ); + #endif +-- diff --git a/thirdparty/patches/rules_boost-undefine-boost_fallthrough.patch b/thirdparty/patches/rules_boost-undefine-boost_fallthrough.patch new file mode 100644 index 000000000..6aa1d989e --- /dev/null +++ b/thirdparty/patches/rules_boost-undefine-boost_fallthrough.patch @@ -0,0 +1,9 @@ +diff --git BUILD.boost BUILD.boost +index 104c726..108e778 100644 +--- BUILD.boost ++++ BUILD.boost +@@ -1356,3 +1356,2 @@ boost_library( + defines = [ +- "BOOST_FALLTHROUGH", + ], +--