Skip to content

Commit

Permalink
[1/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs
Browse files Browse the repository at this point in the history
Part 1: add v1.33.1 version to third_party/grpc
Note: partly switches to v1.33.1 too as not all bits are versioned and
      some of unversioned bits are used from other third_party targets

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed.  In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError.  This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

#12927
  • Loading branch information
scele authored and meteorcloudy committed Feb 25, 2021
1 parent c903cf9 commit 8f7bc2f
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 17 deletions.
18 changes: 9 additions & 9 deletions third_party/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ load("//tools/distributions:distribution_rules.bzl", "distrib_java_import", "dis

licenses(["notice"]) # Apache v2

exports_files(["grpc_1.32.0.patch"])
exports_files(["grpc_1.32.0.patch", "grpc_1.33.1.patch"])

package(default_visibility = ["//visibility:public"])

Expand All @@ -32,14 +32,14 @@ filegroup(
distrib_jar_filegroup(
name = "bootstrap-grpc-jars",
srcs = [
"grpc-api-1.32.2.jar",
"grpc-auth-1.32.2.jar",
"grpc-context-1.32.2.jar",
"grpc-core-1.32.2.jar",
"grpc-netty-1.32.2.jar",
"grpc-protobuf-1.32.2.jar",
"grpc-protobuf-lite-1.32.2.jar",
"grpc-stub-1.32.2.jar",
"grpc-api-1.33.1.jar",
"grpc-auth-1.33.1.jar",
"grpc-context-1.33.1.jar",
"grpc-core-1.33.1.jar",
"grpc-netty-1.33.1.jar",
"grpc-protobuf-1.33.1.jar",
"grpc-protobuf-lite-1.33.1.jar",
"grpc-stub-1.33.1.jar",
],
enable_distributions = ["debian"],
)
Expand Down
12 changes: 6 additions & 6 deletions third_party/grpc/README.bazel.md
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
# How to update the C++ sources of gRPC:

1. Update the gRPC definitions in WORKSPACE file, currently we use
https://github.com/grpc/grpc/archive/v1.32.2.tar.gz
https://github.com/grpc/grpc/archive/v1.33.1.tar.gz
2. Update the gRPC patch file if necessary, it mostly helps avoid unnecessary dependencies.
3. Update third_party/grpc/BUILD to redirect targets to @com_github_grpc_grpc if necessary.

# How to update the BUILD/bzl sources of gRPC:

1. `git clone http://github.com/grpc/grpc.git` in a convenient directory
2. `git checkout <tag>` (current is `v1.32.0`, commithash `414bb8322d`)
2. `git checkout <tag>` (current is `v1.33.1`, commithash `054ff69350`)
3. `mkdir -p third_party/grpc/bazel`
4. `cp <gRPC git tree>/bazel/{BUILD,cc_grpc_library.bzl,generate_cc.bzl,protobuf.bzl} third_party/grpc/bazel`
5. In the `third_party/grpc` directory, apply local patches:
`patch -p3 < bazel_1.32.0.patch`
`patch -p3 < bazel_1.33.1.patch`

# How to update the Java plugin:

1. Checkout tag `v1.32.2` from https://github.com/grpc/grpc-java
1. Checkout tag `v1.33.1` from https://github.com/grpc/grpc-java
2. `cp -R <grpc-java git tree>/compiler/src/java_plugin third_party/grpc/compiler/src`

# How to update the Java code:

Download the necessary jars at version `1.32.2` from maven central.
Download the necessary jars at version `1.33.1` from maven central.

# Submitting the change needs 3 pull requests

1. Update third_party/grpc to include files from new version
2. Switch WORKSPACE, scripts/bootstrap/compile.sh and any other references to new version
2. Switch distdir_deps.bzl, scripts/bootstrap/compile.sh and any other references to new version
3. Remove older version from third_party/grpc
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ class LogHelper {
}
};

// Abort the program after logging the mesage if the given condition is not
// Abort the program after logging the message if the given condition is not
// true. Otherwise, do nothing.
#define GRPC_CODEGEN_CHECK(x) !(x) && LogHelper(&std::cerr).get_os() \
<< "CHECK FAILED: " << __FILE__ << ":" \
<< __LINE__ << ": "

// Abort the program after logging the mesage.
// Abort the program after logging the message.
#define GRPC_CODEGEN_FAIL GRPC_CODEGEN_CHECK(false)

namespace java_grpc_generator {
Expand Down
Binary file added third_party/grpc/grpc-api-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-auth-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-context-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-core-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-netty-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-protobuf-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-protobuf-lite-1.33.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-stub-1.33.1.jar
Binary file not shown.
122 changes: 122 additions & 0 deletions third_party/grpc/grpc_1.33.1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
commit bb0d04663c7dc6c0096f8717cb4ec26330a5ae40
Author: Yun Peng <pcloudy@google.com>
Date: Wed Jun 3 15:35:31 2020 +0200

Patch grpc v1.26.0 for Bazel build

- Avoid loading dependencies that're not needed for the gRPC C++
libraries
- Add bazel mirror URL for upb and cares
- Redirect zlib to @//third_party/zlib
- Add darwin_arm64 and darwin_arm64e config settings to allow
building for Apple Silicon

diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl
index 7bb6b8bdb9..7644107b70 100644
--- a/bazel/grpc_build_system.bzl
+++ b/bazel/grpc_build_system.bzl
@@ -25,7 +25,7 @@

load("//bazel:cc_grpc_library.bzl", "cc_grpc_library")
load("@upb//bazel:upb_proto_library.bzl", "upb_proto_library")
-load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test")
+# load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test")

# The set of pollers to test against if a test exercises polling
POLLERS = ["epollex", "epoll1", "poll"]
@@ -181,13 +181,13 @@ def ios_cc_test(
testonly = 1,
)
ios_test_deps = [ios_test_adapter, ":" + test_lib_ios]
- ios_unit_test(
- name = name + "_on_ios",
- size = kwargs.get("size"),
- tags = ios_tags,
- minimum_os_version = "9.0",
- deps = ios_test_deps,
- )
+ # ios_unit_test(
+ # name = name + "_on_ios",
+ # size = kwargs.get("size"),
+ # tags = ios_tags,
+ # minimum_os_version = "9.0",
+ # deps = ios_test_deps,
+ # )

def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data = [], uses_polling = True, language = "C++", size = "medium", timeout = None, tags = [], exec_compatible_with = [], exec_properties = {}, shard_count = None, flaky = None):
copts = if_mac(["-DGRPC_CFSTREAM"])
diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl
index 09fcad95a2..9b737e5deb 100644
--- a/bazel/grpc_deps.bzl
+++ b/bazel/grpc_deps.bzl
@@ -33,7 +33,7 @@ def grpc_deps():

native.bind(
name = "madler_zlib",
- actual = "@zlib//:zlib",
+ actual = "@//third_party/zlib",
)

native.bind(
diff --git a/bazel/grpc_extra_deps.bzl b/bazel/grpc_extra_deps.bzl
index 4c1dfad2e8..f63c54ddef 100644
--- a/bazel/grpc_extra_deps.bzl
+++ b/bazel/grpc_extra_deps.bzl
@@ -1,11 +1,6 @@
"""Loads the dependencies necessary for the external repositories defined in grpc_deps.bzl."""

-load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
load("@upb//bazel:workspace_deps.bzl", "upb_deps")
-load("@envoy_api//bazel:repositories.bzl", "api_dependencies")
-load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
-load("@build_bazel_rules_apple//apple:repositories.bzl", "apple_rules_dependencies")
-load("@build_bazel_apple_support//lib:repositories.bzl", "apple_support_dependencies")

def grpc_extra_deps():
"""Loads the extra dependencies.
@@ -26,15 +21,5 @@ def grpc_extra_deps():
grpc_extra_deps()
```
"""
- protobuf_deps()
-
upb_deps()

- api_dependencies()
-
- go_rules_dependencies()
- go_register_toolchains()
-
- apple_rules_dependencies()
-
- apple_support_dependencies()
diff --git a/third_party/cares/cares.BUILD b/third_party/cares/cares.BUILD
index c047f0c515..7c24fbc617 100644
--- a/third_party/cares/cares.BUILD
+++ b/third_party/cares/cares.BUILD
@@ -10,6 +10,16 @@ config_setting(
values = {"cpu": "darwin_x86_64"},
)

+config_setting(
+ name = "darwin_arm64",
+ values = {"cpu": "darwin_arm64"},
+)
+
+config_setting(
+ name = "darwin_arm64e",
+ values = {"cpu": "darwin_arm64e"},
+)
+
config_setting(
name = "windows",
values = {"cpu": "x64_windows"},
@@ -99,6 +109,8 @@ copy_file(
":watchos_arm64_32": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
":darwin": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
":darwin_x86_64": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
+ ":darwin_arm64": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
+ ":darwin_arm64e": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
":windows": "@com_github_grpc_grpc//third_party/cares:config_windows/ares_config.h",
":android": "@com_github_grpc_grpc//third_party/cares:config_android/ares_config.h",
"//conditions:default": "@com_github_grpc_grpc//third_party/cares:config_linux/ares_config.h",

0 comments on commit 8f7bc2f

Please sign in to comment.