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.

Sign up for GitHub

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

Cleanup rules, esp. executables, to be more consistent #171

Merged
merged 2 commits into from
Mar 27, 2017
Merged

Cleanup rules, esp. executables, to be more consistent #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c2b89b
Cleanup rules, esp. executables, to be proper
Mar 23, 2017
afdf4a5
Add jvm_flags to all binaries, scala_repl stty
Mar 25, 2017
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
203 changes: 128 additions & 75 deletions scala/scala.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ _srcjar_filetype = FileType([".srcjar"])
# TODO is there a way to derive this from the above?
_scala_srcjar_filetype = FileType([".scala", ".srcjar", ".java"])

def _get_runfiles(target):
runfiles = depset()
runfiles += target.data_runfiles.files
runfiles += target.default_runfiles.files
return runfiles

def _get_all_runfiles(targets):
runfiles = depset()
for target in targets:
runfiles += _get_runfiles(target)
return runfiles

def _adjust_resources_path(path):
# Here we are looking to find out the offset of this resource inside
Expand Down Expand Up @@ -84,14 +95,16 @@ def _build_nosrc_jar(ctx, buildijar):
cp_resources=cp_resources,
out=ctx.outputs.jar.path,
manifest=ctx.outputs.manifest.path,
java=ctx.file._java.path,
java=ctx.executable._java.path,
Copy link
Member

Choose a reason for hiding this comment

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

I didn't know these triggered different side effects. Good to know.

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 shouldn't change the local operation here. However, I needed this so that whatever rule is emitting the _java label was not restricted to emitting a single file. (For example, any binary output by java_binary will still be executable and resolve under ctx.executable but have multiple files and thus not resolve under ctx.file).

I also think it is a little clearer since we are using java as an executable in the actual command.

jar=_get_jar_path(ctx.files._jar))
outs = [ctx.outputs.jar]
if buildijar:
outs.extend([ctx.outputs.ijar])

inputs = ctx.files.resources + ctx.files._jdk + ctx.files._jar + [
ctx.outputs.manifest, ctx.file._java
inputs = ctx.files.resources + [
ctx.outputs.manifest,
ctx.executable._jar,
ctx.executable._java,
]

ctx.action(
Expand Down Expand Up @@ -124,7 +137,7 @@ def _compile(ctx, _jars, dep_srcjars, buildijar):
ijar_cmd_path = ""
if buildijar:
ijar_output_path = ctx.outputs.ijar.path
ijar_cmd_path = ctx.file._ijar.path
ijar_cmd_path = ctx.executable._ijar.path

java_srcs = _java_filetype.filter(ctx.files.srcs)
sources = _scala_filetype.filter(ctx.files.srcs) + java_srcs
Expand Down Expand Up @@ -174,7 +187,7 @@ SourceJars: {srcjars}
ijar_cmd_path=ijar_cmd_path,
srcjars=",".join([f.path for f in all_srcjars]),
javac_opts=" ".join(ctx.attr.javacopts),
javac_path=ctx.file._javac.path,
javac_path=ctx.executable._javac.path,
java_files=",".join([f.path for f in java_srcs]),
# these are the flags passed to javac, which needs them prefixed by -J
jvm_flags=",".join(["-J" + flag for flag in ctx.attr.jvm_flags]),
Expand Down Expand Up @@ -202,10 +215,10 @@ SourceJars: {srcjars}
ctx.files.plugins +
ctx.files.resources +
ctx.files.resource_jars +
ctx.files._jdk +
[ctx.outputs.manifest,
ctx.file._ijar,
ctx.file._java,
ctx.executable._ijar,
ctx.executable._java,
ctx.executable._javac,
ctx.file._scalacompiler,
ctx.file._scalareflect,
ctx.file._scalalib,
Expand Down Expand Up @@ -280,45 +293,49 @@ def write_manifest(ctx):
output=ctx.outputs.manifest,
content=manifest)


def _write_launcher(ctx, jars):
def _write_launcher(ctx, rjars, main_class, jvm_flags, args, run_before_binary, run_after_binary):
classpath = ':'.join(
["$0.runfiles/%s/%s" % (ctx.workspace_name, f.short_path) for f in jars]
)
["$JAVA_RUNFILES/{repo}/{spath}".format(
repo = ctx.workspace_name, spath = f.short_path
) for f in rjars])

content = """#!/bin/bash
export CLASSPATH={classpath}
$0.runfiles/{repo}/{java} {name} "$@"
""".format(
repo=ctx.workspace_name,
java=ctx.file._java.short_path,
name=ctx.attr.main_class,
deploy_jar=ctx.outputs.jar.path,
classpath=classpath,
)
ctx.file_action(
output=ctx.outputs.executable,
content=content)


def _write_test_launcher(ctx, jars):
if len(ctx.attr.suites) != 0:
print(
"suites attribute is deprecated. All scalatest test suites are run"
)
case "$0" in
/*) self="$0" ;;
*) self="$PWD/$0" ;;
esac

if [[ -z "$JAVA_RUNFILES" ]]; then
if [[ -e "${{self}}.runfiles" ]]; then
export JAVA_RUNFILES="${{self}}.runfiles"
fi
if [[ -n "$JAVA_RUNFILES" ]]; then
export TEST_SRCDIR=${{TEST_SRCDIR:-$JAVA_RUNFILES}}
fi
fi

export CLASSPATH={classpath}
{run_before_binary}
$JAVA_RUNFILES/{repo}/{java} {jvm_flags} {main_class} {args} "$@"
BINARY_EXIT_CODE=$?
{run_after_binary}
exit $BINARY_EXIT_CODE
""".format(
classpath = classpath,
repo = ctx.workspace_name,
java = ctx.executable._java.short_path,
jvm_flags = " ".join(jvm_flags),
main_class = main_class,
args = args,
run_before_binary = run_before_binary,
run_after_binary = run_after_binary,
)

content = """#!/bin/bash
{java} -cp {cp} {name} {args} -C io.bazel.rules.scala.JUnitXmlReporter "$@"
"""
content = content.format(
java=ctx.file._java.short_path,
cp=":".join([j.short_path for j in jars]),
name=ctx.attr.main_class,
args="-R \"{path}\" -oWDS".format(path=ctx.outputs.jar.short_path))
ctx.file_action(
output=ctx.outputs.executable,
content=content)

output = ctx.outputs.executable,
content = content,
)

def collect_srcjars(targets):
srcjars = set()
Expand Down Expand Up @@ -384,6 +401,9 @@ def _lib(ctx, non_macro_lib):
transitive_compile_exports=texp.compiletime,
transitive_runtime_exports=texp.runtime
)

# Note that rjars already transitive so don't really
Copy link
Member

Choose a reason for hiding this comment

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

should we collect just the direct runtime jars and not transitive here? Is there any win to not manually accumulating the rjars ourselves? I didn't know about transitive runfiles (maybe they didn't exist when this was written?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just so there is no confusion, adding to transitive_files doesn't seem to invoke any sort of transitive searching. (In all my tests, you could just .to_list() whatever was being added to runfiles.transitive_files and add that to the runfiles.files instead for no difference).

I am fairly certain transitive_files is just for slightly improved performance over files (since it takes a depset). I haven't yet had time to really dig into the bazel implementation to verify this. My current best guess is that transitive_files is resolved and flattened into the runfiles lazily, rather thus can skip that step if the file doesn't actually show up as a runfile. In this case, we already had to linearize the rjars to pass them along in the provider so no cost.

I'm not too worried about making these runfiles perfect because I imagine we will soon move over to using java_common.create_provider anyway and then just setup runfiles and providers using that.

# need to use transitive_files with _get_all_runfiles
runfiles = ctx.runfiles(
files=list(rjars),
collect_data=True)
Expand Down Expand Up @@ -425,11 +445,15 @@ def _scala_binary_common(ctx, cjars, rjars):
_build_deployable(ctx, list(rjars))

runfiles = ctx.runfiles(
files = list(rjars) + [ctx.outputs.executable] + [ctx.file._java] + ctx.files._jdk,
files = list(rjars) + [ctx.outputs.executable],
transitive_files = _get_runfiles(ctx.attr._java),
collect_data = True)

jars = _collect_jars(ctx.attr.deps)
rule_outputs = struct(ijar=outputs.class_jar, class_jar=outputs.class_jar, deploy_jar=ctx.outputs.deploy_jar)
rule_outputs = struct(
ijar=outputs.class_jar,
class_jar=outputs.class_jar,
deploy_jar=ctx.outputs.deploy_jar,
)
scalaattr = struct(outputs = rule_outputs,
transitive_runtime_deps = rjars,
transitive_compile_exports = set(),
Expand All @@ -446,41 +470,59 @@ def _scala_binary_impl(ctx):
cjars += [ctx.file._scalareflect]
rjars += [ctx.outputs.jar, ctx.file._scalalib, ctx.file._scalareflect]
rjars += _collect_jars(ctx.attr.runtime_deps).runtime
_write_launcher(ctx, rjars)
_write_launcher(
ctx = ctx,
rjars = rjars,
main_class = ctx.attr.main_class,
jvm_flags = ctx.attr.jvm_flags,
args = "",
run_before_binary = "",
run_after_binary = "",
)
return _scala_binary_common(ctx, cjars, rjars)

def _scala_repl_impl(ctx):
jars = _collect_jars(ctx.attr.deps)
rjars = jars.runtime
rjars += [ctx.file._scalalib, ctx.file._scalareflect]
rjars += [ctx.file._scalalib, ctx.file._scalareflect, ctx.file._scalacompiler]
rjars += _collect_jars(ctx.attr.runtime_deps).runtime
classpath = ':'.join(["$0.runfiles/%s/%s" % (ctx.workspace_name, f.short_path) for f in rjars])
content = """#!/bin/bash
env JAVACMD=$0.runfiles/{repo}/{java} $0.runfiles/{repo}/{scala} {jvm_flags} -classpath {classpath} {scala_opts} "$@"
""".format(
java=ctx.file._java.short_path,
repo=ctx.workspace_name,
jvm_flags=" ".join(["-J" + flag for flag in ctx.attr.jvm_flags]),
scala=ctx.file._scala.short_path,
classpath=classpath,
scala_opts=" ".join(ctx.attr.scalacopts),

args = " ".join(ctx.attr.scalacopts)
_write_launcher(
ctx = ctx,
rjars = rjars,
main_class = "scala.tools.nsc.MainGenericRunner",
jvm_flags = ["-Dscala.usejavacp=true"] + ctx.attr.jvm_flags,
args = args,
run_before_binary = """
# save stty like in bin/scala
saved_stty=$(stty -g 2>/dev/null)
if [[ ! $? ]]; then
saved_stty=""
fi
""",
run_after_binary = """
if [[ "$saved_stty" != "" ]]; then
stty $saved_stty
saved_stty=""
fi
""",
)
ctx.file_action(
output=ctx.outputs.executable,
content=content)

runfiles = ctx.runfiles(
files = list(rjars) +
[ctx.outputs.executable] +
[ctx.file._java] +
ctx.files._jdk +
[ctx.file._scala],
files = list(rjars) + [ctx.outputs.executable],
transitive_files = _get_runfiles(ctx.attr._java),
collect_data = True)

return struct(
files=set([ctx.outputs.executable]),
runfiles=runfiles)
files = set([ctx.outputs.executable]),
runfiles = runfiles)

def _scala_test_impl(ctx):
if len(ctx.attr.suites) != 0:
print(
"suites attribute is deprecated. All scalatest test suites are run"
)
deps = ctx.attr.deps
deps += [ctx.attr._scalatest_reporter]
jars = _collect_jars(deps)
Expand All @@ -494,17 +536,32 @@ def _scala_test_impl(ctx):
ctx.file._scalaxml
]
rjars += _collect_jars(ctx.attr.runtime_deps).runtime
_write_test_launcher(ctx, rjars)

args = " ".join([
"-R \"{path}\"".format(path=ctx.outputs.jar.short_path),
"-oWDS",
"-C io.bazel.rules.scala.JUnitXmlReporter ",
])
# main_class almost has to be "org.scalatest.tools.Runner" due to args....
_write_launcher(
ctx = ctx,
rjars = rjars,
main_class = ctx.attr.main_class,
jvm_flags = ctx.attr.jvm_flags,
args = args,
run_before_binary = "",
run_after_binary = "",
)
return _scala_binary_common(ctx, cjars, rjars)

_implicit_deps = {
"_ijar": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:ijar"), single_file=True, allow_files=True),
"_ijar": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:ijar"), allow_files=True),
"_scalac": attr.label(executable=True, cfg="host", default=Label("//src/java/io/bazel/rulesscala/scalac"), allow_files=True),
"_scalalib": attr.label(default=Label("@scala//:lib/scala-library.jar"), single_file=True, allow_files=True),
"_scalacompiler": attr.label(default=Label("@scala//:lib/scala-compiler.jar"), single_file=True, allow_files=True),
"_scalareflect": attr.label(default=Label("@scala//:lib/scala-reflect.jar"), single_file=True, allow_files=True),
"_java": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:java"), single_file=True, allow_files=True),
"_javac": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:javac"), single_file=True, allow_files=True),
"_java": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:java"), allow_files=True),
"_javac": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:javac"), allow_files=True),
"_jar": attr.label(executable=True, cfg="host", default=Label("//src/java/io/bazel/rulesscala/jar:binary_deploy.jar"), allow_files=True),
"_jar_bin": attr.label(executable=True, cfg="host", default=Label("//src/java/io/bazel/rulesscala/jar:binary")),
"_jdk": attr.label(default=Label("//tools/defaults:jdk"), allow_files=True),
Expand Down Expand Up @@ -587,11 +644,7 @@ scala_test = rule(

scala_repl = rule(
implementation=_scala_repl_impl,
attrs= _implicit_deps +
_common_attrs +
{
"_scala": attr.label(executable=True, cfg="data", default=Label("@scala//:bin/scala"), single_file=True, allow_files=True)
},
attrs= _implicit_deps + _common_attrs,
outputs={},
executable=True,
)
Expand Down
31 changes: 31 additions & 0 deletions test/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,34 @@ scala_binary(
srcs = ["src/main/scala/scala/test/only_java/Alpha.java"],
main_class = "scala.test.Alpha",
)

# Make sure scala_binary works in test environment
[sh_test(
name = "Run" + binary,
srcs = ["test_binary.sh"],
args = ["$(location %s)" % binary],
data = [":%s" % binary],
) for binary in [
"JavaBinary",
"JavaBinary2",
"ScalaBinary",
"ScalaLibBinary",
"MoreScalaLibBinary",
"MixJavaScalaLibBinary",
"JavaOnlySources",
]]

# Make sure scala_binary works in genrule environment
genrule(
name = "ScalaBinaryInGenrule",
tools = [":ScalaBinary"],
outs = ["scala_binary_out.txt"],
cmd = "$(location :ScalaBinary) > $@",
)

sh_test(
name = "TestScalaBinaryInGenrule",
srcs = ["test_binary.sh"],
args = ["cat $(location :ScalaBinaryInGenrule)"],
data = [":ScalaBinaryInGenrule"],
)
4 changes: 4 additions & 0 deletions test/test_binary.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash

echo "Executing: " $@
$@