Skip to content

Commit

Permalink
Add _jdk dependency to necessary places (#172)
Browse files Browse the repository at this point in the history
The default @local_jdk just imports _java and _javac as raw binaries
without establishing a dependency on/runfiles of _jdk. Thus, all uses of
those should manually add _jdk as a dependency.

In the scalac execution ation that compiles scala, also added _jdk in
case _scalac ever was replaced by a binary that wasn't a java_binary.

_jar_bin does not need a _jdk dependency since is itself a java_binary

Note that earlier versions of this PR had a change to use ijars. This
was reverted (although the comment referring to using ijar amended to
the proper line) to keep this PR as a purely bugfix PR that should
strictly increase the chance something compiles
  • Loading branch information
sdtwigg authored and johnynek committed Mar 28, 2017
1 parent eabdf11 commit 21ebd9c
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions scala/scala.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ def _build_nosrc_jar(ctx, buildijar):
if buildijar:
outs.extend([ctx.outputs.ijar])

inputs = ctx.files.resources + [
# _jdk added manually since _java doesn't currently setup runfiles
inputs = ctx.files.resources + ctx.files._jdk + [
ctx.outputs.manifest,
ctx.executable._jar,
ctx.executable._java,
Expand Down Expand Up @@ -207,6 +208,9 @@ SourceJars: {srcjars}
outs = [ctx.outputs.jar]
if buildijar:
outs.extend([ctx.outputs.ijar])
# _jdk added manually since _java doesn't currently setup runfiles

This comment has been minimized.

Copy link
@or-shachar

or-shachar Jun 11, 2017

Contributor

Hi @sdtwigg ! I am trying to play a bit with the classpath (for my custom fork), and I noticed that this code refers to $RUNPATH but that variable is not available in runtime.

The runpath is defined here

Reading your comment here - I think it might be connected. If so - shouldn't we open a GH issue about it?

(cc @johnynek , @ittaiz )

# _scalac, as a java_binary, should already have it in its runfiles; however,
# adding does ensure _java not orphaned if _scalac ever was not a java_binary
ins = (list(jars) +
list(dep_srcjars) +
list(srcjars) +
Expand All @@ -215,6 +219,7 @@ SourceJars: {srcjars}
ctx.files.plugins +
ctx.files.resources +
ctx.files.resource_jars +
ctx.files._jdk +
[ctx.outputs.manifest,
ctx.executable._ijar,
ctx.executable._java,
Expand Down Expand Up @@ -362,7 +367,7 @@ def _collect_jars(targets):
# see JavaSkylarkApiProvider.java,
# this is just the compile-time deps
# this should be improved in bazel 0.1.5 to get outputs.ijar
# compile_jars += [target.java.outputs.ijar]
# compile_jars += [output.ijar for output in target.java.outputs.jars]
compile_jars += target.java.transitive_deps
runtime_jars += target.java.transitive_runtime_deps
found = True
Expand Down Expand Up @@ -444,8 +449,9 @@ def _scala_binary_common(ctx, cjars, rjars):
outputs = _compile_or_empty(ctx, cjars, [], False) # no need to build an ijar for an executable
_build_deployable(ctx, list(rjars))

# _jdk added manually since _java doesn't currently setup runfiles
runfiles = ctx.runfiles(
files = list(rjars) + [ctx.outputs.executable],
files = list(rjars) + [ctx.outputs.executable] + ctx.files._jdk,
transitive_files = _get_runfiles(ctx.attr._java),
collect_data = True)

Expand Down Expand Up @@ -509,8 +515,9 @@ fi
""",
)

# _jdk added manually since _java doesn't currently setup runfiles
runfiles = ctx.runfiles(
files = list(rjars) + [ctx.outputs.executable],
files = list(rjars) + [ctx.outputs.executable] + ctx.files._jdk,
transitive_files = _get_runfiles(ctx.attr._java),
collect_data = True)

Expand Down

0 comments on commit 21ebd9c

Please sign in to comment.