Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bazel rule closure_grpc_web_library #219

Merged
merged 1 commit into from
Aug 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions WORKSPACE
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
workspace(name = "com_github_grpc_grpc_web")

# TODO(yannic): Update to use official repository.
# See https://github.com/bazelbuild/rules_closure/pull/278
http_archive(
name = "io_bazel_rules_closure",
sha256 = "a80acb69c63d5f6437b099c111480a4493bad4592015af2127a2f49fb7512d8d",
strip_prefix = "rules_closure-0.7.0",
sha256 = "248a7a98eb3962d9f0013e543ea79c5063a83bac7af349ebf412d844e6ab3035",
strip_prefix = "rules_closure-53f2cab21fa6c608f32f114387d88ffd7868c5fc",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this particular commit? Something new available after 0.7.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit includes bazelbuild/rules_closure#278, which is required for this because closure_proto_aspect, which we need to generate the normal proto js files, is not exposed by rules_closure (Everything which starts with _ is private in bazel).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can only merge this PR after bazelbuild/rules_closure#278 got merged right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically you can merge this, you'll just be dependent on a commit that's not in rules_closure's master branch (until it gets merged, that is).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's exactly like @oferb said, there is no technical reason to block this until the other PR is merged.

urls = [
"https://mirror.bazel.build/github.com/bazelbuild/rules_closure/archive/0.7.0.tar.gz",
"https://github.com/bazelbuild/rules_closure/archive/0.7.0.tar.gz",
"https://github.com/Yannic/rules_closure/archive/53f2cab21fa6c608f32f114387d88ffd7868c5fc.zip",
],
)

Expand Down
Empty file added bazel/BUILD.bazel
Empty file.
195 changes: 195 additions & 0 deletions bazel/closure_grpc_web_library.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
# This rule was inspired by rules_closure`s implementation of
# |closure_proto_library|, licensed under Apache 2.
# https://github.com/bazelbuild/rules_closure/blob/3555e5ba61fdcc17157dd833eaf7d19b313b1bca/closure/protobuf/closure_proto_library.bzl

load(
"@io_bazel_rules_closure//closure/compiler:closure_js_library.bzl",
"closure_js_library_impl",
)
load(
"@io_bazel_rules_closure//closure/private:defs.bzl",
"CLOSURE_WORKER_ATTR",
"CLOSURE_LIBRARY_BASE_ATTR",
"unfurl",
)
load(
"@io_bazel_rules_closure//closure/protobuf:closure_proto_library.bzl",
"closure_proto_aspect",
)

# This was borrowed from Rules Go, licensed under Apache 2.
# https://github.com/bazelbuild/rules_go/blob/67f44035d84a352cffb9465159e199066ecb814c/proto/compiler.bzl#L72
def _proto_path(proto):
path = proto.path
root = proto.root.path
ws = proto.owner.workspace_root
if path.startswith(root):
path = path[len(root):]
if path.startswith("/"):
path = path[1:]
if path.startswith(ws):
path = path[len(ws):]
if path.startswith("/"):
path = path[1:]
return path

def _proto_include_path(proto):
path = proto.path[:-len(_proto_path(proto))]
if not path:
return "."
if path.endswith("/"):
path = path[:-1]
return path

def _proto_include_paths(protos):
return depset([_proto_include_path(proto) for proto in protos])

def _generate_closure_grpc_web_src_progress_message(name):
# TODO(yannic): Add a better message?
return "Generating GRPC Web %s" % name

def _generate_closure_grpc_web_srcs(
actions, protoc, protoc_gen_grpc_web, import_style, mode,
sources, transitive_sources):
all_sources = [src for src in sources] + [src for src in transitive_sources]
proto_include_paths = [
"-I%s" % p for p in _proto_include_paths(
[f for f in all_sources])
]

grpc_web_out_common_options = ",".join([
"import_style={}".format(import_style),
"mode={}".format(mode),
])

files = []
for src in sources:
name = "{}.grpc.js".format(
".".join(src.path.split("/")[-1].split(".")[:-1]))
js = actions.declare_file(name)
files.append(js)

args = proto_include_paths + [
"--plugin=protoc-gen-grpc-web={}".format(protoc_gen_grpc_web.path),
"--grpc-web_out={options},out={out_file}:{path}".format(
options = grpc_web_out_common_options,
out_file = name,
path = js.path[:js.path.rfind("/")],
),
src.path
]

actions.run(
inputs = [protoc_gen_grpc_web] + all_sources,
outputs = [js],
executable = protoc,
arguments = args,
progress_message =
_generate_closure_grpc_web_src_progress_message(name),
)

return files

_error_multiple_deps = "".join([
"'deps' attribute must contain exactly one label ",
"(we didn't name it 'dep' for consistency). ",
"We may revisit this restriction later.",
])

def _closure_grpc_web_library_impl(ctx):
if len(ctx.attr.deps) > 1:
# TODO(yannic): Revisit this restriction.
fail(_error_multiple_deps, "deps");

dep = ctx.attr.deps[0]

srcs = _generate_closure_grpc_web_srcs(
actions = ctx.actions,
protoc = ctx.executable._protoc,
protoc_gen_grpc_web = ctx.executable._protoc_gen_grpc_web,
import_style = ctx.attr.import_style,
mode = ctx.attr.mode,
sources = dep.proto.direct_sources,
transitive_sources = dep.proto.transitive_imports,
)

deps = unfurl(ctx.attr.deps, provider = "closure_js_library")
deps += [
ctx.attr._grpc_web_abstractclientbase,
ctx.attr._grpc_web_clientreadablestream,
ctx.attr._grpc_web_error,
ctx.attr._grpc_web_grpcwebclientbase
]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why only these 4 classes? Why not Status and StatusCode etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to add the directly goog.required classes to deps, Status, StatusCode, etc are included as transitive deps by those 4 classes.
https://github.com/grpc/grpc-web/blob/master/javascript/net/grpc/web/grpc_generator.cc#L409
(Only mode GrpcWeb seems to be open source).


suppress = [
"misplacedTypeAnnotation",
]

library = closure_js_library_impl(
actions = ctx.actions,
label = ctx.label,
workspace_name = ctx.workspace_name,

srcs = srcs,
deps = deps,
testonly = ctx.attr.testonly,
suppress = suppress,
lenient = False,

closure_library_base = ctx.files._closure_library_base,
_ClosureWorker = ctx.executable._ClosureWorker,
)
return struct(
exports = library.exports,
closure_js_library = library.closure_js_library,
# The usual suspects are exported as runfiles, in addition to raw source.
runfiles = ctx.runfiles(files = srcs),
)

closure_grpc_web_library = rule(
implementation = _closure_grpc_web_library_impl,
attrs = {
"deps": attr.label_list(
mandatory = True,
providers = ["proto", "closure_js_library"],
# The files generated by this aspect are required dependencies.
aspects = [closure_proto_aspect],
),
"import_style": attr.string(
default = "closure",
values = ["closure"],
),
"mode": attr.string(
default = "grpcwebtext",
values = ["grpcwebtext", "grpcweb"],
),

# Required for closure_js_library_impl
"_ClosureWorker": CLOSURE_WORKER_ATTR,
"_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,

# internal only
"_protoc": attr.label(
default = Label("@com_google_protobuf//:protoc"),
executable = True,
cfg = "host",
),
"_protoc_gen_grpc_web": attr.label(
default = Label("//javascript/net/grpc/web:protoc-gen-grpc-web"),
executable = True,
cfg = "host",
),
"_grpc_web_abstractclientbase": attr.label(
default = Label("//javascript/net/grpc/web:abstractclientbase"),
),
"_grpc_web_clientreadablestream": attr.label(
default = Label("//javascript/net/grpc/web:clientreadablestream"),
),
"_grpc_web_error": attr.label(
default = Label("//javascript/net/grpc/web:error"),
),
"_grpc_web_grpcwebclientbase": attr.label(
default = Label("//javascript/net/grpc/web:grpcwebclientbase"),
),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, why just these 4 classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

},
)
1 change: 1 addition & 0 deletions javascript/net/grpc/web/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ closure_js_library(
"@io_bazel_rules_closure//closure/library/net:eventtype",
"@io_bazel_rules_closure//closure/library/net:xhrio",
"@io_bazel_rules_closure//closure/library/net:xmlhttp",
"@io_bazel_rules_closure//closure/library/string",
],
suppress = [
"reportUnknownTypes",
Expand Down
2 changes: 1 addition & 1 deletion javascript/net/grpc/web/grpc_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ class GrpcCodeGenerator : public CodeGenerator {
string mode;
string import_style_str;
ImportStyle import_style;

for (size_t i = 0; i < options.size(); ++i) {
if (options[i].first == "out") {
file_name = options[i].second;
Expand Down
15 changes: 15 additions & 0 deletions net/grpc/gateway/examples/echo/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
load("//bazel:closure_grpc_web_library.bzl", "closure_grpc_web_library")

proto_library(
name = "echo_proto",
srcs = [
"echo.proto",
],
)

closure_grpc_web_library(
name = "echo",
deps = [
":echo_proto",
],
)
2 changes: 1 addition & 1 deletion net/grpc/gateway/examples/echo/echoapp.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ echoapp.EchoApp.prototype.load = function() {
if (e.keyCode == 13) self.send(); // enter key
return false;
});

$("#msg").focus();
});
};
Expand Down
4 changes: 3 additions & 1 deletion scripts/kokoro.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ docker build -t grpc-web:ubuntu_16_04 \

docker-compose build

bazel test javascript/net/grpc/web/...
bazel test \
//javascript/net/grpc/web/... \
//net/grpc/gateway/examples/...

cd packages/grpc-web && \
npm install && \
Expand Down