Bump to grpcio 0.5.1. (pantsbuild#9470)

### Problem pantsbuild#9395 occurs within our `grpcio` dependency, which is quite stale. Although pantsbuild#9395 is more likely to be related to changes in our executor (pantsbuild#9071) or to our transitive rust dependencies (pantsbuild#9122), getting on a more recent version of the `grpcio` crate _might_ resolve the issue, or make it easier to report an issue. ### Solution Bump to `0.5.1` with one patch (tikv/grpc-rs#457) pulled forward from our previous fork. [ci skip-jvm-tests] # No JVM changes made.
foursquare · Apr 5, 2020 · b1475af · b1475af
1 parent eb43627
commit b1475af
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 70 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -2000,7 +2000,7 @@ jobs:
     if: commit_message !~ /\[ci skip-rust-tests\]/
     name: Rust tests - OSX
     os: osx
-    osx_image: xcode8.3
+    osx_image: xcode8
     script:
     - ./build-support/bin/ci.py --rust-tests
     - ./build-support/bin/release.sh -f

diff --git a/build-support/bin/generate_travis_yml.py b/build-support/bin/generate_travis_yml.py
@@ -678,10 +678,13 @@ def rust_tests_osx() -> Dict:
         **_RUST_TESTS_BASE,
         "name": "Rust tests - OSX",
         "os": "osx",
-        # We need to use xcode8.3 because newer versions of OSX won't let new kexts be installed
-        # without travis taking some action, and we need the osxfuse kext.
-        # See https://github.com/travis-ci/travis-ci/issues/10017
-        "osx_image": "xcode8.3",
+        # We need to use xcode8 because:
+        # 1) versions of OSX newer than xcode8.3 won't let new kexts be installed without travis
+        #    taking some action, and we need the osxfuse kext.
+        #      See https://github.com/travis-ci/travis-ci/issues/10017
+        # 2) xcode 8.3 fails to compile grpc-sys:
+        #      See https://gist.github.com/stuhood/856a9b09bbaa86141f36c9925c14fae7
+        "osx_image": "xcode8",
         "before_install": [
             './build-support/bin/install_python_for_ci.sh "${PYENV_PY36_VERSION}"',
             # We don't use the standard travis "addons" section here because it will either silently

diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock
diff --git a/src/rust/engine/fs/store/Cargo.toml b/src/rust/engine/fs/store/Cargo.toml
@@ -14,7 +14,8 @@ dirs = "1"
 fs = { path = ".." }
 futures01 = { package = "futures", version = "0.1" }
 futures = { version = "0.3", features = ["compat"] }
-grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "b582ef3dc4e8c7289093c8febff8dadf0997b532", default_features = false, features = ["protobuf-codec", "secure"] }
+# TODO: This is 0.5.1 + https://github.com/tikv/grpc-rs/pull/457.
+grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "d16fcfb4afbdd4c791e7c7e64e9748319ed8ad70", default_features = false, features = ["protobuf-codec", "secure"] }
 hashing = { path = "../../hashing" }
 indexmap = "1.0.2"
 itertools = "0.7.2"

diff --git a/src/rust/engine/fs/store/src/remote.rs b/src/rust/engine/fs/store/src/remote.rs
@@ -277,7 +277,7 @@ impl ByteStore {
               .map(|(_client, bytes)| Some(bytes.freeze()))
               .or_else(|e| match e {
                 grpcio::Error::RpcFailure(grpcio::RpcStatus {
-                  status: grpcio::RpcStatusCode::NotFound,
+                  status: grpcio::RpcStatusCode::NOT_FOUND,
                   ..
                 }) => Ok(None),
                 _ => Err(format!(

diff --git a/src/rust/engine/process_execution/Cargo.toml b/src/rust/engine/process_execution/Cargo.toml
@@ -17,7 +17,8 @@ digest = "0.8"
 fs = { path = "../fs" }
 futures01 = { package = "futures", version = "0.1" }
 futures = { version = "0.3", features = ["compat"] }
-grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "b582ef3dc4e8c7289093c8febff8dadf0997b532", default_features = false, features = ["protobuf-codec", "secure"] }
+# TODO: This is 0.5.1 + https://github.com/tikv/grpc-rs/pull/457.
+grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "d16fcfb4afbdd4c791e7c7e64e9748319ed8ad70", default_features = false, features = ["protobuf-codec", "secure"] }
 hashing = { path = "../hashing" }
 libc = "0.2.39"
 log = "0.4"

diff --git a/src/rust/engine/process_execution/bazel_protos/Cargo.toml b/src/rust/engine/process_execution/bazel_protos/Cargo.toml
@@ -8,7 +8,8 @@ publish = false
 [dependencies]
 bytes = "0.4.5"
 futures = "^0.1.16"
-grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "b582ef3dc4e8c7289093c8febff8dadf0997b532", default_features = false, features = ["protobuf-codec", "secure"] }
+# TODO: This is 0.5.1 + https://github.com/tikv/grpc-rs/pull/457.
+grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "d16fcfb4afbdd4c791e7c7e64e9748319ed8ad70", default_features = false, features = ["protobuf-codec", "secure"] }
 hashing = { path = "../../hashing" }
 prost = "0.4"
 prost-derive = "0.4"

diff --git a/src/rust/engine/process_execution/src/remote.rs b/src/rust/engine/process_execution/src/remote.rs
@@ -659,7 +659,7 @@ impl CommandRunner {
         }
 
         let status = execute_response.take_status();
-        if grpcio::RpcStatusCode::from(status.get_code()) == grpcio::RpcStatusCode::Ok {
+        if grpcio::RpcStatusCode::from(status.get_code()) == grpcio::RpcStatusCode::OK {
           let mut execution_attempts = std::mem::replace(&mut attempts.attempts, vec![]);
           execution_attempts.push(attempts.current_attempt);
           return populate_fallible_execution_result(
@@ -678,8 +678,8 @@ impl CommandRunner {
     };
 
     match grpcio::RpcStatusCode::from(status.get_code()) {
-      grpcio::RpcStatusCode::Ok => unreachable!(),
-      grpcio::RpcStatusCode::FailedPrecondition => {
+      grpcio::RpcStatusCode::OK => unreachable!(),
+      grpcio::RpcStatusCode::FAILED_PRECONDITION => {
         if status.get_details().len() != 1 {
           return future::err(ExecutionError::Fatal(format!(
             "Received multiple details in FailedPrecondition ExecuteResponse's status field: {:?}",
@@ -750,11 +750,11 @@ impl CommandRunner {
         future::err(ExecutionError::MissingDigests(missing_digests)).to_boxed()
       }
       code => match code {
-        grpcio::RpcStatusCode::Aborted
-        | grpcio::RpcStatusCode::Internal
-        | grpcio::RpcStatusCode::ResourceExhausted
-        | grpcio::RpcStatusCode::Unavailable
-        | grpcio::RpcStatusCode::Unknown => {
+        grpcio::RpcStatusCode::ABORTED
+        | grpcio::RpcStatusCode::INTERNAL
+        | grpcio::RpcStatusCode::RESOURCE_EXHAUSTED
+        | grpcio::RpcStatusCode::UNAVAILABLE
+        | grpcio::RpcStatusCode::UNKNOWN => {
           future::err(ExecutionError::Retryable(status.get_message().to_owned())).to_boxed()
         }
         _ => future::err(ExecutionError::Fatal(format!(
@@ -1184,7 +1184,7 @@ fn rpcerror_recover_cancelled(
 ) -> Result<bazel_protos::operations::Operation, grpcio::Error> {
   // If the error represented cancellation, return an Operation for the given Operation name.
   match &err {
-    &grpcio::Error::RpcFailure(ref rs) if rs.status == grpcio::RpcStatusCode::Cancelled => {
+    &grpcio::Error::RpcFailure(ref rs) if rs.status == grpcio::RpcStatusCode::CANCELLED => {
       let mut next_operation = bazel_protos::operations::Operation::new();
       next_operation.set_name(operation_name);
       return Ok(next_operation);

diff --git a/src/rust/engine/process_execution/src/remote_tests.rs b/src/rust/engine/process_execution/src/remote_tests.rs
@@ -588,7 +588,7 @@ async fn server_rejecting_execute_request_gives_error() {
   let error = run_command_remote(mock_server.address(), execute_request)
     .await
     .expect_err("Want Err");
-  assert_that(&error).contains("InvalidArgument");
+  assert_that(&error).contains("INVALID_ARGUMENT");
   assert_that(&error).contains("Did not expect this request");
 }
 
@@ -1591,7 +1591,7 @@ async fn execute_missing_file_uploads_if_known_status() {
     let op_name = "cat".to_owned();
 
     let status = grpcio::RpcStatus {
-      status: grpcio::RpcStatusCode::FailedPrecondition,
+      status: grpcio::RpcStatusCode::FAILED_PRECONDITION,
       details: None,
       status_proto_bytes: Some(
         make_precondition_failure_status(vec![missing_preconditionfailure_violation(
@@ -1925,14 +1925,14 @@ async fn extract_execute_response_other_status() {
     let mut response = bazel_protos::remote_execution::ExecuteResponse::new();
     response.set_status({
       let mut status = bazel_protos::status::Status::new();
-      status.set_code(grpcio::RpcStatusCode::PermissionDenied as i32);
+      status.set_code(grpcio::RpcStatusCode::PERMISSION_DENIED.into());
       status
     });
     response
   }));
 
   match extract_execute_response(operation, Platform::Linux).await {
-    Err(ExecutionError::Fatal(err)) => assert_contains(&err, "PermissionDenied"),
+    Err(ExecutionError::Fatal(err)) => assert_contains(&err, "PERMISSION_DENIED"),
     other => assert!(false, "Want fatal error, got {:?}", other),
   };
 }
@@ -2396,7 +2396,7 @@ fn make_incomplete_operation(operation_name: &str) -> MockOperation {
 
 fn make_retryable_operation_failure() -> MockOperation {
   let mut status = bazel_protos::status::Status::new();
-  status.set_code(grpcio::RpcStatusCode::Aborted as i32);
+  status.set_code(grpcio::RpcStatusCode::ABORTED.into());
   status.set_message(String::from("the bot running the task appears to be lost"));
 
   let mut operation = bazel_protos::operations::Operation::new();
@@ -2533,7 +2533,7 @@ fn make_precondition_failure_status(
   violations: Vec<bazel_protos::error_details::PreconditionFailure_Violation>,
 ) -> bazel_protos::status::Status {
   let mut status = bazel_protos::status::Status::new();
-  status.set_code(grpcio::RpcStatusCode::FailedPrecondition as i32);
+  status.set_code(grpcio::RpcStatusCode::FAILED_PRECONDITION.into());
   status.mut_details().push(make_any_proto(&{
     let mut precondition_failure = bazel_protos::error_details::PreconditionFailure::new();
     for violation in violations.into_iter() {

diff --git a/src/rust/engine/testutil/mock/Cargo.toml b/src/rust/engine/testutil/mock/Cargo.toml
@@ -9,7 +9,8 @@ publish = false
 bazel_protos = { path = "../../process_execution/bazel_protos" }
 bytes = "0.4.5"
 futures01 = { package = "futures", version = "0.1" }
-grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "b582ef3dc4e8c7289093c8febff8dadf0997b532", default_features = false, features = ["protobuf-codec", "secure"] }
+# TODO: This is 0.5.1 + https://github.com/tikv/grpc-rs/pull/457.
+grpcio = { git = "https://github.com/pantsbuild/grpc-rs.git", rev = "d16fcfb4afbdd4c791e7c7e64e9748319ed8ad70", default_features = false, features = ["protobuf-codec", "secure"] }
 hashing = { path = "../../hashing" }
 parking_lot = "0.6"
 protobuf = { version = "2.0.6", features = ["with-bytes"] }

diff --git a/src/rust/engine/testutil/mock/src/cas.rs b/src/rust/engine/testutil/mock/src/cas.rs
@@ -177,7 +177,7 @@ impl StubCAS {
   /// The address on which this server is listening over insecure HTTP transport.
   ///
   pub fn address(&self) -> String {
-    let bind_addr = self.server_transport.bind_addrs().first().unwrap();
+    let bind_addr = self.server_transport.bind_addrs().next().unwrap();
     format!("{}:{}", bind_addr.0, bind_addr.1)
   }
 
@@ -209,14 +209,18 @@ macro_rules! check_auth {
       if authorization_headers.len() != 1
         || authorization_headers[0] != required_auth_header.as_bytes()
       {
-        $sink.fail(grpcio::RpcStatus::new(
-          grpcio::RpcStatusCode::Unauthenticated,
-          Some(format!(
-            "Bad Authorization header; want {:?} got {:?}",
-            required_auth_header.as_bytes(),
-            authorization_headers
-          )),
-        ));
+        $ctx.spawn(
+          $sink
+            .fail(grpcio::RpcStatus::new(
+              grpcio::RpcStatusCode::UNAUTHENTICATED,
+              Some(format!(
+                "Bad Authorization header; want {:?} got {:?}",
+                required_auth_header.as_bytes(),
+                authorization_headers
+              )),
+            ))
+            .then(|_| Ok(())),
+        );
         return;
       }
     }
@@ -238,7 +242,7 @@ impl StubCASResponder {
       || parts.get(1) != Some(&"blobs")
     {
       return Err(grpcio::RpcStatus::new(
-        grpcio::RpcStatusCode::InvalidArgument,
+        grpcio::RpcStatusCode::INVALID_ARGUMENT,
         Some(format!(
           "Bad resource name format {} - want {}/blobs/some-sha256/size",
           req.get_resource_name(),
@@ -249,13 +253,13 @@ impl StubCASResponder {
     let digest = parts[2];
     let fingerprint = Fingerprint::from_hex_string(digest).map_err(|e| {
       grpcio::RpcStatus::new(
-        grpcio::RpcStatusCode::InvalidArgument,
+        grpcio::RpcStatusCode::INVALID_ARGUMENT,
         Some(format!("Bad digest {}: {}", digest, e)),
       )
     })?;
     if self.always_errors {
       return Err(grpcio::RpcStatus::new(
-        grpcio::RpcStatusCode::Internal,
+        grpcio::RpcStatusCode::INTERNAL,
         Some("StubCAS is configured to always fail".to_owned()),
       ));
     }
@@ -273,7 +277,7 @@ impl StubCASResponder {
           .collect(),
       ),
       None => Err(grpcio::RpcStatus::new(
-        grpcio::RpcStatusCode::NotFound,
+        grpcio::RpcStatusCode::NOT_FOUND,
         Some(format!("Did not find digest {}", fingerprint)),
       )),
     }
@@ -319,7 +323,7 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
         ),
       ),
       Err(err) => {
-        sink.fail(err);
+        ctx.spawn(sink.fail(err).then(|_| Ok(())));
       }
     }
   }
@@ -350,7 +354,7 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
               Some(ref resource_name) => {
                 if resource_name != req.get_resource_name() {
                   return Err(grpcio::Error::RpcFailure(grpcio::RpcStatus::new(
-                    grpcio::RpcStatusCode::InvalidArgument,
+                    grpcio::RpcStatusCode::INVALID_ARGUMENT,
                     Some(format!(
                       "All resource names in stream must be the same. Got {} but earlier saw {}",
                       req.get_resource_name(),
@@ -362,7 +366,7 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
             }
             if req.get_write_offset() != want_next_offset {
               return Err(grpcio::Error::RpcFailure(grpcio::RpcStatus::new(
-                grpcio::RpcStatusCode::InvalidArgument,
+                grpcio::RpcStatusCode::INVALID_ARGUMENT,
                 Some(format!(
                   "Missing chunk. Expected next offset {}, got next offset: {}",
                   want_next_offset,
@@ -378,12 +382,12 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
         })
         .map_err(move |err: grpcio::Error| match err {
           grpcio::Error::RpcFailure(status) => status,
-          e => grpcio::RpcStatus::new(grpcio::RpcStatusCode::Unknown, Some(format!("{:?}", e))),
+          e => grpcio::RpcStatus::new(grpcio::RpcStatusCode::UNKNOWN, Some(format!("{:?}", e))),
         })
         .and_then(
           move |(maybe_resource_name, bytes)| match maybe_resource_name {
             None => Err(grpcio::RpcStatus::new(
-              grpcio::RpcStatusCode::InvalidArgument,
+              grpcio::RpcStatusCode::INVALID_ARGUMENT,
               Some("Stream saw no messages".to_owned()),
             )),
             Some(resource_name) => {
@@ -394,15 +398,15 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
                 || parts.get(3) != Some(&"blobs")
               {
                 return Err(grpcio::RpcStatus::new(
-                  grpcio::RpcStatusCode::InvalidArgument,
+                  grpcio::RpcStatusCode::INVALID_ARGUMENT,
                   Some(format!("Bad resource name: {}", resource_name)),
                 ));
               }
               let fingerprint = match Fingerprint::from_hex_string(parts[4]) {
                 Ok(f) => f,
                 Err(err) => {
                   return Err(grpcio::RpcStatus::new(
-                    grpcio::RpcStatusCode::InvalidArgument,
+                    grpcio::RpcStatusCode::INVALID_ARGUMENT,
                     Some(format!(
                       "Bad fingerprint in resource name: {}: {}",
                       parts[4], err
@@ -414,14 +418,14 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
                 Ok(s) => s,
                 Err(err) => {
                   return Err(grpcio::RpcStatus::new(
-                    grpcio::RpcStatusCode::InvalidArgument,
+                    grpcio::RpcStatusCode::INVALID_ARGUMENT,
                     Some(format!("Bad size in resource name: {}: {}", parts[5], err)),
                   ));
                 }
               };
               if size != bytes.len() {
                 return Err(grpcio::RpcStatus::new(
-                  grpcio::RpcStatusCode::InvalidArgument,
+                  grpcio::RpcStatusCode::INVALID_ARGUMENT,
                   Some(format!(
                     "Size was incorrect: resource name said size={} but got {}",
                     size,
@@ -432,7 +436,7 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
 
               if always_errors {
                 return Err(grpcio::RpcStatus::new(
-                  grpcio::RpcStatusCode::Internal,
+                  grpcio::RpcStatusCode::INTERNAL,
                   Some("StubCAS is configured to always fail".to_owned()),
                 ));
               }
@@ -463,7 +467,7 @@ impl bazel_protos::bytestream_grpc::ByteStream for StubCASResponder {
     sink: grpcio::UnarySink<bazel_protos::bytestream::QueryWriteStatusResponse>,
   ) {
     sink.fail(grpcio::RpcStatus::new(
-      grpcio::RpcStatusCode::Unimplemented,
+      grpcio::RpcStatusCode::UNIMPLEMENTED,
       None,
     ));
   }
@@ -480,14 +484,14 @@ impl bazel_protos::remote_execution_grpc::ContentAddressableStorage for StubCASR
 
     if self.always_errors {
       sink.fail(grpcio::RpcStatus::new(
-        grpcio::RpcStatusCode::Internal,
+        grpcio::RpcStatusCode::INTERNAL,
         Some("StubCAS is configured to always fail".to_owned()),
       ));
       return;
     }
     if req.instance_name != self.instance_name() {
       sink.fail(grpcio::RpcStatus::new(
-        grpcio::RpcStatusCode::NotFound,
+        grpcio::RpcStatusCode::NOT_FOUND,
         Some(format!(
           "Wrong instance_name; want {:?} got {:?}",
           self.instance_name(),
@@ -515,7 +519,7 @@ impl bazel_protos::remote_execution_grpc::ContentAddressableStorage for StubCASR
     sink: grpcio::UnarySink<bazel_protos::remote_execution::BatchUpdateBlobsResponse>,
   ) {
     sink.fail(grpcio::RpcStatus::new(
-      grpcio::RpcStatusCode::Unimplemented,
+      grpcio::RpcStatusCode::UNIMPLEMENTED,
       None,
     ));
   }