Skip to content

Commit 921f43e

Browse files
committed
- Remove duplicate attribute semantics of 'paths' and 'imports': 'imports' (string_list) is
the mechanism to pass '--proto_path' or '-I' args to protoc. - BUILD file cleanup.
1 parent faa7a83 commit 921f43e

File tree

16 files changed

+35
-116
lines changed

16 files changed

+35
-116
lines changed

bzl/base/class.bzl

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,42 +54,11 @@ def build_generated_filenames(lang, self):
5454
self["outs"] += [srcfile.rsplit('.', 1)[0] + ext]
5555

5656

57-
def build_paths(lang, self):
57+
def build_imports(lang, self):
5858
"""Build the list of imports"""
5959
ctx = self["ctx"]
6060
if ctx:
61-
self["paths"] = self.get("paths", []) + ctx.attr.paths
62-
63-
def build_imports(lang, self):
64-
"""Copy any named proto files in the imports to the genfiles area to make visible to protoc. Not sure this is necessary anymore."""
65-
ctx = self["ctx"]
66-
if not ctx:
67-
fail("Bazel context is required for build_imports")
68-
#print("ctx.attr.imports: %s" % type(ctx.attr.imports))
69-
for target in ctx.attr.imports:
70-
for srcfile in target.files:
71-
#print("srcfile: %s" % srcfile.path)
72-
dstfile = ctx.new_file(srcfile.path)
73-
#dstfile = ctx.new_file(srcfile.path)
74-
# By declaring that the proto action (happening later)
75-
# depends on these files, we ensure that the
76-
# CpImportToPackageGenfile action is run. Without this
77-
# linkage, bazel assumes the action isn't required.
78-
self["requires"] += [dstfile]
79-
if ctx.attr.verbose > 1:
80-
print("Copying import %s --> %s" % (srcfile.path, dstfile.path))
81-
ctx.action(mnemonic = "CpImportToPackageGenfiles",
82-
inputs = [srcfile],
83-
outputs = [dstfile],
84-
arguments = [srcfile.path, dstfile.path],
85-
command = "cp $1 $2")
86-
# These probably need to be compiled also. Nope: if you
87-
# try this you get "inconsistent package names" from
88-
# https://github.com/golang/protobuf/blob/1687f003bf0280bd6182a5a1c7241856b269a6c0/protoc-gen-go/generator/generator.go#L750
89-
#self["srcs"] += [dstfile]
90-
91-
#self["go_plugin_options"] = self.get("go_plugin_options", []) + ["M" + srcfile.path + "="]
92-
#self["paths"] += [self["outdir"]]
61+
self["imports"] = self.get("imports", []) + ctx.attr.imports
9362

9463
def build_plugin_out(name, key, lang, self):
9564
#print("build_plugin_out(%s, %s)" % (name, key))
@@ -164,7 +133,7 @@ def build_protobuf_invocation(lang, self):
164133
def build_protoc_command(lang, self):
165134
"""Build a command list required for genrule execution"""
166135
self["cmd"] += ["$(location %s)" % self["protoc"]]
167-
self["cmd"] += ["--proto_path=" + i for i in self["paths"]]
136+
self["cmd"] += ["--proto_path=" + i for i in self["imports"]]
168137
self["cmd"] += self["args"]
169138
self["cmd"] += ["$(location " + proto + ")" for proto in self["protos"]]
170139

@@ -219,7 +188,6 @@ CLASS = struct(
219188
build_generated_filenames = build_generated_filenames,
220189
build_imports = build_imports,
221190
build_inputs = build_inputs,
222-
build_paths = build_paths,
223191
build_tools = build_tools,
224192
build_protobuf_invocation = build_protobuf_invocation,
225193
build_protobuf_out = build_protobuf_out,

bzl/cpp/rules.bzl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ def cc_proto_library(
1212
grpc_plugin_options = [],
1313
imports = [],
1414
lang = CPP,
15-
paths = [],
1615
protobuf_plugin_options = [],
1716
protobuf_plugin = None,
1817
proto_compile = cc_proto_compile,
@@ -35,7 +34,6 @@ def cc_proto_library(
3534
args["gen_protobuf_" + lang.name + "_plugin"] = protobuf_plugin
3635
args["gen_" + lang.name + "_plugin_options"] = protobuf_plugin_options
3736
args["gen_grpc_" + lang.name + "_plugin"] = grpc_plugin
38-
args["paths"] = paths
3937
args["protoc"] = protoc
4038
args["protos"] = srcs
4139
args["verbose"] = verbose

bzl/go/class.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def _build_grpc_out(lang, self):
1515

1616

1717
def _build_imports(lang, self):
18-
"""@Override: Copy any named proto files in the imports to the genfiles area to make visible to protoc. Not sure this is necessary anymore."""
18+
"""@Override: for all transitive packages source file names, provide import mapping."""
1919
invokesuper("build_imports", lang, self)
2020

2121
ctx = self["ctx"]

bzl/go/rules.bzl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ def go_proto_library(
1212
grpc_plugin_options = [],
1313
imports = [],
1414
lang = GO,
15-
paths = [],
1615
protobuf_plugin_options = [],
1716
protobuf_plugin = None,
1817
proto_compile = go_proto_compile,
@@ -35,7 +34,6 @@ def go_proto_library(
3534
args["gen_protobuf_" + lang.name + "_plugin"] = protobuf_plugin
3635
args["gen_" + lang.name + "_plugin_options"] = protobuf_plugin_options
3736
args["gen_grpc_" + lang.name + "_plugin"] = grpc_plugin
38-
args["paths"] = paths
3937
args["protoc"] = protoc
4038
args["protos"] = srcs
4139
args["verbose"] = verbose

bzl/java/rules.bzl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ def java_proto_library(
1111
grpc_plugin_options = [],
1212
imports = [],
1313
lang = JAVA,
14-
paths = [],
1514
protobuf_plugin_options = [],
1615
protobuf_plugin = None,
1716
proto_compile = java_proto_compile,
@@ -34,7 +33,6 @@ def java_proto_library(
3433
args["gen_protobuf_" + lang.name + "_plugin"] = protobuf_plugin
3534
args["gen_" + lang.name + "_plugin_options"] = protobuf_plugin_options
3635
args["gen_grpc_" + lang.name + "_plugin"] = grpc_plugin
37-
args["paths"] = paths
3836
args["protoc"] = protoc
3937
args["protos"] = srcs
4038
args["verbose"] = verbose

bzl/protoc.bzl

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ def protoc_genrule(name,
5555
"gendir": outdir,
5656
"outdir": outdir,
5757
"protos": protos,
58-
"paths": paths,
5958
"protoc": protoc,
6059
"protobuf_plugin": protobuf_plugin,
6160
"protobuf_plugin_options": protobuf_plugin_options,
@@ -81,7 +80,7 @@ def protoc_genrule(name,
8180

8281
invoke("build_generated_filenames", lang, self)
8382
invoke("build_tools", lang, self)
84-
invoke("build_paths", lang, self)
83+
invoke("build_imports", lang, self)
8584
invoke("build_protobuf_invocation", lang, self)
8685
invoke("build_protobuf_out", lang, self)
8786
if with_grpc:
@@ -119,7 +118,7 @@ def _execute_rule(self):
119118
#self["args"] += ["--descriptor_set_out=%s" % (descriptor_set_file.path)]
120119

121120
arglist = list(set(self["args"]))
122-
pathlist = ["--proto_path=" + i for i in set(self["paths"])]
121+
pathlist = ["--proto_path=" + i for i in set(self["imports"])]
123122

124123
arguments = arglist + pathlist + srcfiles
125124
inputs = list(set(self["requires"]))
@@ -171,9 +170,9 @@ def _build_source_files(ctx, self):
171170
self["srcs"] += [srcfile]
172171
self["srcfilenames"] += [srcfile.short_path]
173172

174-
# This is the key to enable imports: protoc can see the entire
175-
# source tree from the workspace root.
176-
self["paths"] += ["."]
173+
# This is the key for protoc to see the entire source tree from the
174+
# workspace root.
175+
self["imports"] += ["."]
177176

178177
def _protoc_rule_impl(ctx):
179178

@@ -189,7 +188,6 @@ def _protoc_rule_impl(ctx):
189188
"gendir": gendir,
190189
"outdir": outdir,
191190
"imports": [],
192-
"paths": [],
193191
"args": [],
194192
"srcs": [],
195193
"srcfilenames": [],
@@ -200,7 +198,6 @@ def _protoc_rule_impl(ctx):
200198
"with_grpc": getattr(ctx.attr, "with_grpc", False),
201199
#"descriptor_set_file": descriptor_set_file,
202200
"transitive_imports": [],
203-
"transitive_paths": [],
204201
"transitive_packages": {},
205202
"transitive_requires": [],
206203
"transitive_srcs": [],
@@ -209,7 +206,6 @@ def _protoc_rule_impl(ctx):
209206
# Propogate proto deps:
210207
for dep in ctx.attr.deps:
211208
self["transitive_imports"] += dep.proto.transitive_imports
212-
self["transitive_paths"] += dep.proto.transitive_paths
213209
self["transitive_packages"] += dep.proto.transitive_packages
214210
self["transitive_requires"] += dep.proto.transitive_requires
215211
self["transitive_srcs"] += dep.proto.transitive_srcs
@@ -230,7 +226,6 @@ def _protoc_rule_impl(ctx):
230226

231227
invoke("build_generated_files", lang, self)
232228
invoke("build_imports", lang, self)
233-
invoke("build_paths", lang, self)
234229
invoke("build_protobuf_invocation", lang, self)
235230
invoke("build_protobuf_out", lang, self)
236231
if self["with_grpc"]:
@@ -254,14 +249,12 @@ def _protoc_rule_impl(ctx):
254249
proto=struct(
255250

256251
imports = self["imports"],
257-
paths = self["paths"],
258252
packages = self["packages"],
259253
srcs = set(self["srcs"]),
260254
requires = self["requires"],
261255

262256
transitive_requires = self["requires"] + self["transitive_requires"],
263257
transitive_imports = self["imports"] + self["transitive_imports"],
264-
transitive_paths = self["paths"] + self["transitive_paths"],
265258
transitive_packages = self["packages"] + self["transitive_packages"],
266259
transitive_srcs = self["srcs"] + self["transitive_srcs"],
267260
),
@@ -289,18 +282,8 @@ def implement(spec):
289282

290283
attrs["deps"] = attr.label_list(providers = ["proto"])
291284

292-
# Options to be passed to protoc as --proto_path. Differs from
293-
# imports in that these are raw strings rather than labels.
294-
attrs["paths"] = attr.string_list()
295-
296-
# Protos that should be made available for proto imports. These are
297-
# not added as options but rather copied over to the sandbox where
298-
# protoc is run, making them available for import. TODO: is this
299-
# really needed? Test it by using the descriptor protos from
300-
# google/protobuf.
301-
attrs["imports"] = attr.label_list(
302-
allow_files = FileType([".proto"]),
303-
)
285+
# Options to be passed to protoc as --proto_path.
286+
attrs["imports"] = attr.string_list()
304287

305288
# The list of files the rule generates. How is this actually being
306289
# used?

examples/helloworld/cpp/BUILD

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
package(default_visibility = ["//visibility:public"])
22

3-
load("//bzl:cpp/rules.bzl", "cc_proto_library", "cc_proto_compile")
4-
load(
5-
"//bzl:cpp/class.bzl",
6-
CPP = "CLASS",
7-
)
3+
load("//bzl:cpp/class.bzl", CPP = "CLASS")
84

95
cc_test(
106
name = "test",
Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,10 @@
11
package(default_visibility = ["//visibility:public"])
22

33
load("@io_bazel_rules_go//go:def.bzl", "go_binary")
4-
load(
5-
"//bzl:go/class.bzl",
6-
GO = "CLASS",
7-
)
8-
load("//bzl:go/rules.bzl", "go_proto_compile", "go_proto_library")
4+
load("//bzl:go/class.bzl", GO = "CLASS")
95

106
go_binary(
117
name = "client",
128
srcs = ["main.go"],
139
deps = ["//examples/helloworld/proto:go"] + GO.grpc.compile_deps,
1410
)
15-
16-
# # if you depend on this rule, the proto sources get copied to the
17-
# # namespace of the calling BUILD file (examples/helloworld/go/client),
18-
# # affecting the go "import" naming.
19-
# go_proto_library(
20-
# name = 'protolib',
21-
# protos = [
22-
# '//examples/helloworld/proto:srcs',
23-
# ],
24-
# with_grpc = True,
25-
# verbose = 0,
26-
# )

examples/helloworld/go/greeter_test/BUILD

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package(default_visibility = ["//visibility:public"])
22

33
load("@io_bazel_rules_go//go:def.bzl", "go_test")
4-
load(
5-
"//bzl:go/class.bzl",
6-
GO = "CLASS",
7-
)
4+
load("//bzl:go/class.bzl", GO = "CLASS")
85

96
go_test(
107
name = "greeter_test",

examples/helloworld/go/server/BUILD

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package(default_visibility = ["//visibility:public"])
22

33
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
4-
load(
5-
"//bzl:go/class.bzl",
6-
GO = "CLASS",
7-
)
4+
load("//bzl:go/class.bzl", GO = "CLASS")
85

96
go_binary(
107
name = "server",

0 commit comments

Comments
 (0)