Skip to content

Commit ecbe5e7

Browse files
authored
Use zipper rather than jdk/jar in the thrift rules (#286)
* use latest 0.6.0 in travis * use workers in travis * handle 0 input cases without the jar warning * Use zipper with an arg file instead of jdk/jar in thrift * fix incorrect args * use new lines on the arg files * Add a test with multiple thrifts * Append a newline at the end of the file
1 parent a5ed9ba commit ecbe5e7

File tree

4 files changed

+51
-31
lines changed

4 files changed

+51
-31
lines changed

test/src/main/scala/scala/test/twitter_scrooge/thrift/BUILD

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,9 @@ thrift_library(
99
],
1010
visibility = ["//visibility:public"],
1111
)
12+
13+
thrift_library(
14+
name = "thrift_many",
15+
srcs = ["ThriftMany1.thrift", "ThriftMany2.thrift"],
16+
visibility = ["//visibility:public"],
17+
)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
struct Many1 {
2+
1: i32 one
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
struct Many2 {
2+
1: i16 two
3+
}

thrift/thrift.bzl

Lines changed: 39 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def _thrift_library_impl(ctx):
2525
if len(src_paths) <= 0 and len(ctx.attr.external_jars) <= 0:
2626
fail("we require at least one thrift file in a target")
2727

28-
jarcmd = "{jar} cMf {out} -C {out}_tmp ."
28+
zipper_args = "\n".join(src_paths) + "\n"
2929
if len(prefixes) > 0:
3030
common_prefix = _common_prefix(src_paths)
3131
found_prefixes = [p for p in prefixes if common_prefix.find(p) >= 0]
@@ -43,34 +43,43 @@ def _thrift_library_impl(ctx):
4343
pos = common_prefix.find(prefix)
4444
endpos = pos + len(prefix)
4545
actual_prefix = common_prefix[0:endpos]
46-
jarcmd = "{{jar}} cMf {{out}} -C {{out}}_tmp/{pf} .".format(pf=actual_prefix)
47-
48-
_valid_thrift_deps(ctx.attr.deps)
49-
# We move the files and touch them so that the output file is a purely deterministic
50-
# product of the _content_ of the inputs
51-
cmd = """
52-
rm -rf {out}_tmp
53-
mkdir -p {out}_tmp
54-
{jar} cMf {out}_tmp/tmp.jar $@
55-
unzip -q -o {out}_tmp/tmp.jar -d {out}_tmp 2>/dev/null
56-
rm -rf {out}_tmp/tmp.jar
57-
find {out}_tmp -exec touch -t 198001010000 {{}} \;
58-
""" + jarcmd + """
59-
rm -rf {out}_tmp"""
60-
61-
cmd = cmd.format(out=ctx.outputs.libarchive.path,
62-
jar=ctx.executable._jar.path)
63-
64-
# We need _jdk to even run _jar. Depending on _jar is not enough with sandbox
65-
ctx.action(
66-
inputs = ctx.files.srcs +
67-
ctx.files._jdk +
68-
[ctx.executable._jar],
69-
outputs = [ctx.outputs.libarchive],
70-
command = cmd,
71-
progress_message = "making thrift archive %s" % ctx.label,
72-
arguments = src_paths,
73-
)
46+
zipper_args = "\n".join([ "%s=%s" % (src[endpos+1:], src) for src in src_paths ]) + "\n"
47+
48+
if len(src_paths) > 0:
49+
zipper_arg_path = ctx.actions.declare_file("%s_zipper_args" % ctx.outputs.libarchive.path)
50+
ctx.file_action(zipper_arg_path, zipper_args)
51+
_valid_thrift_deps(ctx.attr.deps)
52+
# We move the files and touch them so that the output file is a purely deterministic
53+
# product of the _content_ of the inputs
54+
cmd = """
55+
rm -f {out}
56+
{zipper} c {out} @{path}
57+
"""
58+
59+
cmd = cmd.format(out=ctx.outputs.libarchive.path,
60+
path=zipper_arg_path.path,
61+
zipper=ctx.executable._zipper.path)
62+
ctx.action(
63+
inputs = ctx.files.srcs + [ctx.executable._zipper, zipper_arg_path],
64+
outputs = [ctx.outputs.libarchive],
65+
command = cmd,
66+
progress_message = "making thrift archive %s (%s files)" % (ctx.label, len(src_paths)),
67+
)
68+
else:
69+
# we still have to create the output we declared
70+
ctx.action(
71+
inputs = [ctx.executable._zipper],
72+
outputs = [ctx.outputs.libarchive],
73+
command = """
74+
echo "empty" > {out}.contents
75+
rm -f {out}
76+
{zipper} c {out} {out}.contents
77+
rm {out}.contents
78+
""".format(out=ctx.outputs.libarchive.path,
79+
zipper=ctx.executable._zipper.path),
80+
progress_message = "making empty thrift archive %s" % ctx.label,
81+
)
82+
7483

7584
transitive_srcs = _collect_thrift_srcs(ctx.attr.deps)
7685
transitive_srcs += [ctx.outputs.libarchive]
@@ -135,8 +144,7 @@ thrift_library = rule(
135144
"absolute_prefixes": attr.string_list(),
136145
# This is a list of JARs which only contain Thrift files
137146
"external_jars": attr.label_list(),
138-
"_jar": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:jar"), allow_files=True),
139-
"_jdk": attr.label(default=Label("//tools/defaults:jdk"), allow_files=True),
147+
"_zipper": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/zip:zipper"), allow_files=True)
140148
},
141149
outputs={"libarchive": "lib%{name}.jar"},
142150
)

0 commit comments

Comments
 (0)