thrift_library fails silently if unzip is not installed

[nollbit]

Hello!

I couldn't get the following to work on my Ubuntu box even though it worked flawlessly on OS X. The generated jar files would turn up empty. Looking at the commands executed (using -s) I noticed the unzip command. On a hunch I checked unzip, and it turns out it wasn't installed. Installing it fixed the issue.

unzip fails silently (since it has it's stderr redirected, and it's not the last command executed to in the end the exit code looks OK from Bazel's perspective), so the directory ultimately used to build the jar in the end is empty. Thus the jar turns out empty.

https://github.com/bazelbuild/rules_scala/blob/master/thrift/thrift.bzl#L55

I don't have a PR since I'm not really sure where the fix should be applied:

1. Bazel switching to bash -ce instead of bash -e?
2. thrift.bzl adding && at the end of each command?

thrift_library(
  name = "thrift_test_src",
  srcs = glob(["src/test/thrift/**/*.thrift"]),
  deps = [
  ],
  visibility = ["//visibility:public"],
)

~/my-repo$ bazel build :thrift_test_src -s --spawn_strategy standalone
INFO: Found 1 target...
>>>>> # //thing:thrift_test_src [action 'making thrift archive //thing:thrift_test_src']
(cd /home/user/.cache/bazel/_bazel_user/478b92944ef4677a88e99594ea89c7ae/execroot/my-repo && \
  exec env - \
  /bin/bash -c '
rm -rf bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp
mkdir -p bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp
external/local_jdk/bin/jar cMf bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp/tmp.jar $@
unzip -q -o bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp/tmp.jar -d bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp 2>/dev/null
rm -rf bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp/tmp.jar
find bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp -exec touch -t 198001010000 {} \;
external/local_jdk/bin/jar cMf bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar -C bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp .
rm -rf bazel-out/local-fastbuild/bin/thing/libthrift_test_src.jar_tmp' '' thing/src/test/thrift/microservice.thrift thing/src/test/thrift/second.thrift)
Target //thing:thrift_test_src up-to-date:
  bazel-bin/thing/libthrift_test_src.jar
INFO: Elapsed time: 2.746s, Critical Path: 0.13s

$ jar tf ~/my-repo/bazel-bin/thing/libthrift_test_src.jar
..empty...

[johnynek]

Thanks for reporting this.

In fact, bash based scripts are kind of discouraged for this reason. I don't know how to change the way bazel executes the script, but the && patch could be a quick fix.

A better approach might be to just write some java code that does this part of the rule, and then bazel can build the tooling as part of the whole build. That may be the ideal for portability.

[johnynek]

Also note that bazel comes with jar since it depends on jdk, and you can access that one with @bazel_tools//jdk:jar or something, so we can also just switch to using that.

[softprops]

Without too much context https://bazel.build/versions/master/docs/be/pkg.html may come in handy here. Ive actually hit this problem with some custom rules that used zip in a docker container that didnt have zip installed

[softprops]

Nm this ticket is about _un_packing