Compatibility issues (#6071)

* Pass -f - to tar to force stdin on Windows

* Quote paths that may contain spaces (causes issues on Windows)

* Copy over Windows code from Arrow for glog signal handle uninstall

* Add missing COPTS to build rules since we'll need them for Windows compatibility

* Begin adding COPTS for Windows compatibility

* Disable glog on Arrow until we change WIN32 to _WIN32 there

* Missing header files that cause problems on Windows

* WORD typedef conflicts with Windows; remove it

* uint -> unsigned int wherever we're dealing with milliseconds (signed version is already int)

* uint -> unsigned int for enums

* uint -> size_t, wherever we're dealing with sizes or indices into arrays

* Work around Boost 1.68 bug in detecting clang-cl (revert this after upgrading)

* Missing #include <unistd.h>

* Add check for signal handler uninstallation failure

* Linting issue

BUILD.bazel

@@ -8,7 +8,13 @@ load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")
 load("@//bazel:ray.bzl", "flatbuffer_py_library")
 load("@//bazel:cython_library.bzl", "pyx_library")
 
-COPTS = ["-DRAY_USE_GLOG"]
+COPTS = ["-DRAY_USE_GLOG"] + select({
+    "@bazel_tools//src/conditions:windows": [
+        "-Wno-builtin-macro-redefined",  # To get rid of warnings caused by deterministic build macros (e.g. #define __DATE__ "redacted")
+    ],
+    "//conditions:default": [
+    ],
+})
 
 # === Begin of protobuf definitions ===
 
@@ -536,6 +542,7 @@ cc_test(
 cc_test(
     name = "stats_test",
     srcs = ["src/ray/stats/stats_test.cc"],
+    copts = COPTS,
     deps = [
         ":stats_lib",
         "@com_google_googletest//:gtest_main",
@@ -620,6 +627,7 @@ cc_library(
     hdrs = [
         "src/ray/thirdparty/sha256.h",
     ],
+    copts = COPTS,
     includes = ["src/ray/thirdparty"],
 )
 
@@ -642,6 +650,7 @@ cc_library(
         "src/ray/thirdparty/ae/ae_kqueue.c",
         "src/ray/thirdparty/ae/ae_epoll.c",
     ]),
+    copts = COPTS,
     includes = [
         "src/ray/thirdparty/ae",
         "src/ray/thirdparty/hiredis",
@@ -755,6 +764,7 @@ pyx_library(
         "python/ray/includes/*.pxd",
         "python/ray/includes/*.pxi",
     ]),
+    copts = COPTS,
     deps = [
         "//:core_worker_lib",
         "//:raylet_lib",
@@ -839,7 +849,7 @@ genrule(
     ],
     cmd = """
         set -x &&
-        curl -sL \"https://github.com/antirez/redis/archive/5.0.3.tar.gz\" | tar xz --strip-components=1 -C . &&
+        curl -sL \"https://github.com/antirez/redis/archive/5.0.3.tar.gz\" | tar -xzf - --strip-components=1 -C . &&
         make &&
         mv ./src/redis-server $(location redis-server) &&
         chmod +x $(location redis-server) &&
@@ -892,26 +902,26 @@ genrule(
     cmd = """
         set -x &&
         WORK_DIR=$$(pwd) &&
-        cp -f $(location python/ray/_raylet.so) $$WORK_DIR/python/ray &&
-        mkdir -p $$WORK_DIR/python/ray/core/src/ray/thirdparty/redis/src/ &&
-        cp -f $(location //:redis-server) $$WORK_DIR/python/ray/core/src/ray/thirdparty/redis/src/ &&
-        cp -f $(location //:redis-cli) $$WORK_DIR/python/ray/core/src/ray/thirdparty/redis/src/ &&
-        mkdir -p $$WORK_DIR/python/ray/core/src/ray/gcs/redis_module/ &&
-        cp -f $(locations //:libray_redis_module.so) $$WORK_DIR/python/ray/core/src/ray/gcs/redis_module/ &&
-        cp -f $(location //:raylet_monitor) $$WORK_DIR/python/ray/core/src/ray/raylet/ &&
-        cp -f $(location @plasma//:plasma_store_server) $$WORK_DIR/python/ray/core/src/plasma/ &&
-        cp -f $(location //:raylet) $$WORK_DIR/python/ray/core/src/ray/raylet/ &&
-        mkdir -p $$WORK_DIR/python/ray/core/generated/ray/protocol/ &&
+        cp -f $(location python/ray/_raylet.so) "$$WORK_DIR/python/ray" &&
+        mkdir -p "$$WORK_DIR/python/ray/core/src/ray/thirdparty/redis/src/" &&
+        cp -f $(location //:redis-server) "$$WORK_DIR/python/ray/core/src/ray/thirdparty/redis/src/" &&
+        cp -f $(location //:redis-cli) "$$WORK_DIR/python/ray/core/src/ray/thirdparty/redis/src/" &&
+        mkdir -p "$$WORK_DIR/python/ray/core/src/ray/gcs/redis_module/" &&
+        cp -f $(locations //:libray_redis_module.so) "$$WORK_DIR/python/ray/core/src/ray/gcs/redis_module/" &&
+        cp -f $(location //:raylet_monitor) "$$WORK_DIR/python/ray/core/src/ray/raylet/" &&
+        cp -f $(location @plasma//:plasma_store_server) "$$WORK_DIR/python/ray/core/src/plasma/" &&
+        cp -f $(location //:raylet) "$$WORK_DIR/python/ray/core/src/ray/raylet/" &&
+        mkdir -p "$$WORK_DIR/python/ray/core/generated/ray/protocol/" &&
         for f in $(locations //:all_py_proto); do
-            cp -f $$f $$WORK_DIR/python/ray/core/generated/;
+            cp -f "$$f" "$$WORK_DIR/python/ray/core/generated/";
         done &&
         # NOTE(hchen): Protobuf doesn't allow specifying Python package name. So we use this `sed`
         # command to change the import path in the generated file.
-        sed -i -E 's/from src.ray.protobuf/from ./' $$WORK_DIR/python/ray/core/generated/gcs_pb2.py &&
-        sed -i -E 's/from src.ray.protobuf/from ./' $$WORK_DIR/python/ray/core/generated/common_pb2.py &&
-        sed -i -E 's/from src.ray.protobuf/from ./' $$WORK_DIR/python/ray/core/generated/node_manager_pb2.py &&
-        sed -i -E 's/from src.ray.protobuf/from ./' $$WORK_DIR/python/ray/core/generated/node_manager_pb2_grpc.py &&
-        echo $$WORK_DIR > $@
+        sed -i -E 's/from src.ray.protobuf/from ./' "$$WORK_DIR/python/ray/core/generated/gcs_pb2.py" &&
+        sed -i -E 's/from src.ray.protobuf/from ./' "$$WORK_DIR/python/ray/core/generated/common_pb2.py" &&
+        sed -i -E 's/from src.ray.protobuf/from ./' "$$WORK_DIR/python/ray/core/generated/node_manager_pb2.py" &&
+        sed -i -E 's/from src.ray.protobuf/from ./' "$$WORK_DIR/python/ray/core/generated/node_manager_pb2_grpc.py" &&
+        echo "$$WORK_DIR" > $@
     """,
     local = 1,
 )

bazel/BUILD.plasma

@@ -1,6 +1,12 @@
 load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")
 
-COPTS = ["-DARROW_USE_GLOG"]
+COPTS = [] + select({
+    "@bazel_tools//src/conditions:windows": [
+    ],
+    "//conditions:default": [
+        "-DARROW_USE_GLOG",
+    ],
+})
 
 cc_library(
     name = "arrow",
@@ -20,6 +26,7 @@ cc_library(
         "cpp/src/arrow/buffer.h",
         "cpp/src/arrow/io/concurrency.h",
         "cpp/src/arrow/io/interfaces.h",
+        "cpp/src/arrow/io/mman.h",
         "cpp/src/arrow/io/util_internal.h",
         "cpp/src/arrow/memory_pool.h",
         "cpp/src/arrow/result.h",
@@ -41,6 +48,7 @@ cc_library(
         "cpp/src/arrow/util/thread_pool.h",
         "cpp/src/arrow/util/type_traits.h",
         "cpp/src/arrow/util/ubsan.h",
+        "cpp/src/arrow/util/utf8.h",
         "cpp/src/arrow/util/variant.h",
         "cpp/src/arrow/util/visibility.h",
         "cpp/src/arrow/util/windows_compatibility.h",

bazel/cython_library.bzl

@@ -8,7 +8,7 @@
 # been written at cython/cython and tensorflow/tensorflow. We branch from
 # Tensorflow's version as it is more actively maintained and works for gRPC
 # Python's needs.
-def pyx_library(name, deps=[], py_deps=[], srcs=[], **kwargs):
+def pyx_library(name, deps=[], py_deps=[], srcs=[], copts=[], **kwargs):
     """Compiles a group of .pyx / .pxd / .py files.
     First runs Cython to create .cpp files for each input .pyx or .py + .pxd
     pair. Then builds a shared object for each, passing "deps" to each cc_binary
@@ -46,7 +46,7 @@ def pyx_library(name, deps=[], py_deps=[], srcs=[], **kwargs):
             # Optionally use PYTHON_BIN_PATH on Linux platforms so that python 3
             # works. Windows has issues with cython_binary so skip PYTHON_BIN_PATH.
             cmd=
-            "PYTHONHASHSEED=0 $${PYTHON_BIN_PATH} $(location @cython//:cython_binary) --cplus $(SRCS) --output-file $(OUTS)",
+            'PYTHONHASHSEED=0 "$${PYTHON_BIN_PATH}" "$(location @cython//:cython_binary)" --cplus $(SRCS) --output-file $(OUTS)',
             tools=["@cython//:cython_binary"] + pxd_srcs,
         )
 
@@ -57,6 +57,7 @@ def pyx_library(name, deps=[], py_deps=[], srcs=[], **kwargs):
         native.cc_binary(
             name=shared_object_name,
             srcs=[stem + ".cpp"],
+            copts=copts,
             deps=deps + ["@local_config_python//:python_headers"],
             linkshared=1,
         )

src/ray/common/task/scheduling_resources.cc

@@ -92,7 +92,7 @@ ResourceSet::ResourceSet(const std::unordered_map<std::string, double> &resource
 ResourceSet::ResourceSet(const std::vector<std::string> &resource_labels,
                          const std::vector<double> resource_capacity) {
   RAY_CHECK(resource_labels.size() == resource_capacity.size());
-  for (uint i = 0; i < resource_labels.size(); i++) {
+  for (size_t i = 0; i < resource_labels.size(); i++) {
     RAY_CHECK(resource_capacity[i] > 0);
     resource_capacity_[resource_labels[i]] =
         FractionalResourceQuantity(resource_capacity[i]);

src/ray/gcs/callback.h

@@ -1,6 +1,11 @@
 #ifndef RAY_GCS_CALLBACK_H
 #define RAY_GCS_CALLBACK_H
 
+#if defined(__clang__) && defined(_MSC_VER)
+// TODO(mehrdadn): Remove this Windows (clang-cl) workaround once we upgrade to
+// Boost > 1.68: https://lists.boost.org/Archives/boost/2018/09/243420.php
+#include <boost/type_traits.hpp>
+#endif
 #include <boost/optional/optional.hpp>
 #include <vector>
 #include "ray/common/status.h"

src/ray/gcs/redis_gcs_client.cc

@@ -3,6 +3,8 @@
 #include "ray/common/ray_config.h"
 #include "ray/gcs/redis_context.h"
 
+#include <unistd.h>
+
 static void GetRedisShards(redisContext *context, std::vector<std::string> &addresses,
                            std::vector<int> &ports) {
   // Get the total number of Redis shards in the system.

src/ray/object_manager/object_buffer_pool.h

@@ -157,7 +157,7 @@ class ObjectBufferPool {
   };
 
   /// The state of a chunk associated with a create operation.
-  enum class CreateChunkState : uint { AVAILABLE = 0, REFERENCED, SEALED };
+  enum class CreateChunkState : unsigned int { AVAILABLE = 0, REFERENCED, SEALED };
 
   /// Holds the state of a create buffer.
   struct CreateBufferState {

src/ray/object_manager/object_manager.cc

@@ -270,7 +270,7 @@ void ObjectManager::HandlePushTaskTimeout(const ObjectID &object_id,
                    << " after waiting for " << config_.push_timeout_ms << " ms.";
   auto iter = unfulfilled_push_requests_.find(object_id);
   RAY_CHECK(iter != unfulfilled_push_requests_.end());
-  uint num_erased = iter->second.erase(client_id);
+  size_t num_erased = iter->second.erase(client_id);
   RAY_CHECK(num_erased == 1);
   if (iter->second.size() == 0) {
     unfulfilled_push_requests_.erase(iter);

src/ray/object_manager/object_manager.h

@@ -37,7 +37,7 @@ struct ObjectManagerConfig {
   int object_manager_port;
   /// The time in milliseconds to wait before retrying a pull
   /// that fails due to client id lookup.
-  uint pull_timeout_ms;
+  unsigned int pull_timeout_ms;
   /// Object chunk size, in bytes
   uint64_t object_chunk_size;
   /// The store socket name.

src/ray/object_manager/test/object_manager_stress_test.cc

@@ -95,7 +95,7 @@ class TestObjectManagerBase : public ::testing::Test {
     store_id_1 = StartStore(UniqueID::FromRandom().Hex());
     store_id_2 = StartStore(UniqueID::FromRandom().Hex());
 
-    uint pull_timeout_ms = 1000;
+    unsigned int pull_timeout_ms = 1000;
     uint64_t object_chunk_size = static_cast<uint64_t>(std::pow(10, 3));
     int push_timeout_ms = 10000;
 
@@ -193,7 +193,7 @@ class StressTestObjectManager : public TestObjectManagerBase {
   };
 
   int async_loop_index = -1;
-  uint num_expected_objects;
+  size_t num_expected_objects;
 
   std::vector<TransferPattern> async_loop_patterns = {
       TransferPattern::PUSH_A_B,
@@ -254,7 +254,7 @@ class StressTestObjectManager : public TestObjectManagerBase {
 
   void TransferTestNext() {
     async_loop_index += 1;
-    if ((uint)async_loop_index < async_loop_patterns.size()) {
+    if ((size_t)async_loop_index < async_loop_patterns.size()) {
       TransferPattern pattern = async_loop_patterns[async_loop_index];
       TransferTestExecute(100, 3 * std::pow(10, 3) - 1, pattern);
     } else {
@@ -305,12 +305,12 @@ class StressTestObjectManager : public TestObjectManagerBase {
                   << static_cast<int>(async_loop_patterns[async_loop_index]) << " "
                   << v1.size() << " " << elapsed;
     ASSERT_TRUE(v1.size() == v2.size());
-    for (uint i = 0; i < v1.size(); ++i) {
+    for (size_t i = 0; i < v1.size(); ++i) {
       ASSERT_TRUE(std::find(v1.begin(), v1.end(), v2[i]) != v1.end());
     }
 
     // Compare objects and their hashes.
-    for (uint i = 0; i < v1.size(); ++i) {
+    for (size_t i = 0; i < v1.size(); ++i) {
       ObjectID object_id_2 = v2[i];
       ObjectID object_id_1 =
           v1[std::distance(v1.begin(), std::find(v1.begin(), v1.end(), v2[i]))];
@@ -333,9 +333,9 @@ class StressTestObjectManager : public TestObjectManagerBase {
     if (transfer_pattern == TransferPattern::BIDIRECTIONAL_PULL ||
         transfer_pattern == TransferPattern::BIDIRECTIONAL_PUSH ||
         transfer_pattern == TransferPattern::BIDIRECTIONAL_PULL_VARIABLE_DATA_SIZE) {
-      num_expected_objects = (uint)2 * num_trials;
+      num_expected_objects = (size_t)2 * num_trials;
     } else {
-      num_expected_objects = (uint)num_trials;
+      num_expected_objects = (size_t)num_trials;
     }
 
     start_time = current_time_ms();

src/ray/object_manager/test/object_manager_test.cc

@@ -88,7 +88,7 @@ class TestObjectManagerBase : public ::testing::Test {
     store_id_1 = StartStore(UniqueID::FromRandom().Hex());
     store_id_2 = StartStore(UniqueID::FromRandom().Hex());
 
-    uint pull_timeout_ms = 1;
+    unsigned int pull_timeout_ms = 1;
     push_timeout_ms = 1000;
 
     // start first server
@@ -175,7 +175,7 @@ class TestObjectManagerBase : public ::testing::Test {
   std::string store_id_1;
   std::string store_id_2;
 
-  uint push_timeout_ms;
+  unsigned int push_timeout_ms;
 
   uint64_t object_chunk_size = static_cast<uint64_t>(std::pow(10, 3));
 };
@@ -227,7 +227,7 @@ class TestObjectManager : public TestObjectManagerBase {
         });
     RAY_CHECK_OK(status);
 
-    uint data_size = 1000000;
+    size_t data_size = 1000000;
 
     // dummy_id is not local. The push function will timeout.
     ObjectID dummy_id = ObjectID::FromRandom();
@@ -251,8 +251,8 @@ class TestObjectManager : public TestObjectManagerBase {
   }
 
   void NotificationTestCompleteIfSatisfied() {
-    uint num_expected_objects1 = 1;
-    uint num_expected_objects2 = 2;
+    size_t num_expected_objects1 = 1;
+    size_t num_expected_objects2 = 2;
     if (v1.size() == num_expected_objects1 && v2.size() == num_expected_objects2) {
       SubscribeObjectThenWait();
     }
@@ -371,9 +371,9 @@ class TestObjectManager : public TestObjectManagerBase {
           RAY_LOG(DEBUG) << "remaining " << remaining.size();
 
           // Ensure object order is preserved for all invocations.
-          uint j = 0;
-          uint k = 0;
-          for (uint i = 0; i < object_ids.size(); ++i) {
+          size_t j = 0;
+          size_t k = 0;
+          for (size_t i = 0; i < object_ids.size(); ++i) {
             ObjectID oid = object_ids[i];
             // Make sure the object is in either the found vector or the remaining vector.
             if (j < found.size() && found[j] == oid) {

src/ray/raylet/lineage_cache.h

@@ -2,6 +2,11 @@
 #define RAY_RAYLET_LINEAGE_CACHE_H
 
 #include <gtest/gtest_prod.h>
+#if defined(__clang__) && defined(_MSC_VER)
+// TODO(mehrdadn): Remove this Windows (clang-cl) workaround once we upgrade to
+// Boost > 1.68: https://lists.boost.org/Archives/boost/2018/09/243420.php
+#include <boost/type_traits.hpp>
+#endif
 #include <boost/optional.hpp>
 
 #include "ray/common/id.h"

src/ray/raylet/object_manager_integration_test.cc

@@ -126,7 +126,7 @@ class TestObjectManagerBase : public ::testing::Test {
 
 class TestObjectManagerIntegration : public TestObjectManagerBase {
  public:
-  uint num_expected_objects;
+  size_t num_expected_objects;
 
   int num_connected_clients = 0;
 
@@ -178,7 +178,7 @@ class TestObjectManagerIntegration : public TestObjectManagerBase {
   void TestPush(int64_t data_size) {
     ray::Status status = ray::Status::OK();
 
-    num_expected_objects = (uint)1;
+    num_expected_objects = (size_t)1;
     ObjectID oid1 = WriteDataToClient(client1, data_size);
     server1->object_manager_.Push(oid1, client_id_2);
   }

src/ray/raylet/raylet_client.cc

@@ -267,12 +267,12 @@ ray::Status RayletClient::Wait(const std::vector<ObjectID> &object_ids, int num_
   // Parse the flatbuffer object.
   auto reply_message = flatbuffers::GetRoot<ray::protocol::WaitReply>(reply.get());
   auto found = reply_message->found();
-  for (uint i = 0; i < found->size(); i++) {
+  for (size_t i = 0; i < found->size(); i++) {
     ObjectID object_id = ObjectID::FromBinary(found->Get(i)->str());
     result->first.push_back(object_id);
   }
   auto remaining = reply_message->remaining();
-  for (uint i = 0; i < remaining->size(); i++) {
+  for (size_t i = 0; i < remaining->size(); i++) {
     ObjectID object_id = ObjectID::FromBinary(remaining->Get(i)->str());
     result->second.push_back(object_id);
   }

src/ray/raylet/worker.h

@@ -11,6 +11,8 @@
 #include "ray/rpc/worker/direct_actor_client.h"
 #include "ray/rpc/worker/worker_client.h"
 
+#include <unistd.h>  // pid_t
+
 namespace ray {
 
 namespace raylet {

src/ray/thirdparty/sha256.c

@@ -29,7 +29,7 @@
 #define SIG1(x) (ROTRIGHT(x,17) ^ ROTRIGHT(x,19) ^ ((x) >> 10))
 
 /**************************** VARIABLES *****************************/
-static const WORD k[64] = {
+static const unsigned int k[64] = {
 	0x428a2f98,0x71374491,0xb5c0fbcf,0xe9b5dba5,0x3956c25b,0x59f111f1,0x923f82a4,0xab1c5ed5,
 	0xd807aa98,0x12835b01,0x243185be,0x550c7dc3,0x72be5d74,0x80deb1fe,0x9bdc06a7,0xc19bf174,
 	0xe49b69c1,0xefbe4786,0x0fc19dc6,0x240ca1cc,0x2de92c6f,0x4a7484aa,0x5cb0a9dc,0x76f988da,
@@ -43,7 +43,7 @@ static const WORD k[64] = {
 /*********************** FUNCTION DEFINITIONS ***********************/
 void sha256_transform(SHA256_CTX *ctx, const BYTE data[])
 {
-	WORD a, b, c, d, e, f, g, h, i, j, t1, t2, m[64];
+	unsigned int a, b, c, d, e, f, g, h, i, j, t1, t2, m[64];
 
 	for (i = 0, j = 0; i < 16; ++i, j += 4)
 		m[i] = (data[j] << 24) | (data[j + 1] << 16) | (data[j + 2] << 8) | (data[j + 3]);
@@ -98,7 +98,7 @@ void sha256_init(SHA256_CTX *ctx)
 
 void sha256_update(SHA256_CTX *ctx, const BYTE data[], size_t len)
 {
-	WORD i;
+	unsigned int i;
 
 	for (i = 0; i < len; ++i) {
 		ctx->data[ctx->datalen] = data[i];
@@ -113,7 +113,7 @@ void sha256_update(SHA256_CTX *ctx, const BYTE data[], size_t len)
 
 void sha256_final(SHA256_CTX *ctx, BYTE hash[])
 {
-	WORD i;
+	unsigned int i;
 
 	i = ctx->datalen;
 

src/ray/thirdparty/sha256.h

@@ -17,13 +17,12 @@
 
 /**************************** DATA TYPES ****************************/
 typedef unsigned char BYTE;             // 8-bit byte
-typedef unsigned int  WORD;             // 32-bit word, change to "long" for 16-bit machines
 
 typedef struct {
 	BYTE data[64];
-	WORD datalen;
+	unsigned int datalen;
 	unsigned long long bitlen;
-	WORD state[8];
+	unsigned int state[8];
 } SHA256_CTX;
 
 /*********************** FUNCTION DECLARATIONS **********************/

src/ray/util/logging.cc

@@ -173,15 +173,21 @@ void RayLog::UninstallSignalAction() {
   RAY_LOG(DEBUG) << "Uninstall signal handlers.";
   // This signal list comes from glog's signalhandler.cc.
   // https://github.com/google/glog/blob/master/src/signalhandler.cc#L58-L70
-  static std::vector<int> installed_signals({SIGSEGV, SIGILL, SIGFPE, SIGABRT, SIGTERM});
+  std::vector<int> installed_signals({SIGSEGV, SIGILL, SIGFPE, SIGABRT, SIGTERM});
+#ifdef _WIN32  // Do NOT use WIN32 (without the underscore); we want _WIN32 here
+  for (int signal_num : installed_signals) {
+    RAY_CHECK(signal(signal_num, SIG_DFL) != SIG_ERR);
+  }
+#else
   struct sigaction sig_action;
   memset(&sig_action, 0, sizeof(sig_action));
   sigemptyset(&sig_action.sa_mask);
   sig_action.sa_handler = SIG_DFL;
   for (int signal_num : installed_signals) {
-    sigaction(signal_num, &sig_action, NULL);
+    RAY_CHECK(sigaction(signal_num, &sig_action, NULL) == 0);
   }
 #endif
+#endif
 }
 
 void RayLog::ShutDownRayLog() {