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.

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

Issue locating runfiles directory for toolchains dependencies #5463

Closed
katre opened this issue Jun 25, 2018 · 12 comments
Closed

Issue locating runfiles directory for toolchains dependencies #5463

katre opened this issue Jun 25, 2018 · 12 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) type: bug

Comments

@katre
Copy link
Member

katre commented Jun 25, 2018

Originally reported by @johnynek, during testing of bazelbuild/rules_scala#530

If you take the PR, and sync the rules_scala repo to commit 2071aa4, and run bazel clean && bazel build //src/..., you get an error:

$ bazel clean && bazel build //src/...
INFO: Starting clean.
INFO: Build options have changed, discarding analysis cache.
INFO: Analysed 20 targets (57 packages loaded).
INFO: Found 20 targets...
ERROR: /Users/jcater/repos/rules_scala/src/scala/scripts/BUILD:25:1: scala //src/scala/scripts:scala_proto_request_extractor failed (Exit 1)
bazel-out/host/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac: Cannot locate runfiles directory. (Set $JAVA_RUNFILES to inhibit searching.)
INFO: Elapsed time: 4.403s, Critical Path: 3.84s
INFO: 37 processes: 29 darwin-sandbox, 4 local, 4 worker.
FAILED: Build did NOT complete successfully

Oddly, syncing to the parent commit (afa6403) does not produce this error.

There's nothing obvious about the changes in 2071aa4 that would affects the runfiles directory, except that it shows more use of toolchains dependencies.

See the bazel-discuss thread at https://groups.google.com/forum/#!topic/bazel-discuss/4P3jiFsUggw for additional context.

@katre katre assigned aragos and katre and unassigned aragos Jun 25, 2018
@katre
Copy link
Member Author

katre commented Jun 25, 2018

The error "Cannot locate runfiles directory. (Set $JAVA_RUNFILES to inhibit searching.)" is from the bash Java launcher script that is wrapping scalac. Looking further to see why this problem is happening.

@katre
Copy link
Member Author

katre commented Jun 25, 2018

And now I can reproduce this on Linux, when I couldn't last week.

@katre katre changed the title Issue locating runfiles directory on macOS Issue locating runfiles directory for toolchains dependencies Jun 25, 2018
@katre
Copy link
Member Author

katre commented Jun 25, 2018

Okay, I've tracked down the symptoms here:

  • The scala_library rule declares the toolchain dependency on @io_bazel_rules_scala//scala:toolchain_type
  • The scala_toolchain rule depends on @io_bazel_rules_scala//src/java/io/bazel/rulesscala/scalac, which is a java_binary target.
    • scala_toolchain gets the binary as ctx.executable.scalac, and saves that in ToolchainInfo
  • scala_library_impl (eventually) gets the ToolchainInfo from ctx.toolchains, gets the scalac tool (at this point, it's a File, as that's what scala_toolchain's ctx.executable.scalac evaluated to), and uses that in ctx.actions().run()
  • The actual execution fails, because scalac's runfiles directory didn't get included into the scala_library's dependencies

It the last part that I am currently trying to figure out: where should that have been handled, and why isn't it working?

@jhnj
Copy link

jhnj commented Jun 25, 2018

Me and @andyscott encountered a similar problem last week while trying to pass an executable through a provider. I was able to get this building using the same fix we used for that. Not sure if this is of any help when fixing this. Anyways, here are the steps I used:

  • Pass scalac as a Label instead of File in the toolchain (ctx.executable.scalac -> ctx.attr.scalc)
  • Get the executable as well as the input manifests from the label (in _compile):
  _, _, input_manifests = ctx.resolve_command(tools = [toolchain.scalac])
  ctx.actions.run(
      inputs = ins,
      outputs = outs,
      executable = toolchain.scalac.files_to_run.executable,
      input_manifests = input_manifests,

bazel clean && bazel build //src/... works for me after that, created a branch where it's working: https://github.com/jhnj/rules_scala/tree/johan/toolchain-scalac-executable-fix

To get it to work from another workspace we also needed to add [toolchain.scalac.files_to_run.runfiles_manifest] to ins but that wasn't needed just to get it to build.

@katre
Copy link
Member Author

katre commented Jun 25, 2018

@jhnj Thanks for the pointer, that does seem to work for me. I hadn't realized that the inputs also needs the runfiles_manifest.

This seems to be a consequence of the fact that java_binaries (unlike cc_binaries) aren't a single file, but a launcher and a runfiles directory.

I am seeing another error:

$ bazel-out/host/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac @bazel-out/k8-fastbuild/bin/src/scala/io/bazel/rules_scala/scrooge_support/focused_zip_importer_worker_input
Error: Could not find or load main class io.bazel.rulesscala.scalac.ScalaCInvoker

and I am trying to see if I can figure that out.

@johnynek Any ideas about what's wrong with ScalaCInvoker?

@johnynek
Copy link
Member

@katre I don't know what the issue is there. focused_zip_importer is a library used with thrift generation, so the fact that it can't find ScalaCInvoker does not stand out as special (except that thrift generation happens in macro generally:
https://github.com/bazelbuild/rules_scala/blob/master/twitter_scrooge/twitter_scrooge.bzl#L292
)

@katre
Copy link
Member Author

katre commented Jun 25, 2018

focused_zip_importer is the specific target I picked to test so I wasn't getting breakpoints on every target. I'm not sure why it can't find ScalaCInvoker.

@katre
Copy link
Member Author

katre commented Jun 25, 2018

I've attached a patch that makes the original PR seem to work (I can't figure out the ScalaCInvoker issue, but the scalac worker is actually starting).

I think it'd be nicer if toolchains just automatically supplied their runfiles to actions, but this needs some thinking and I'll want to discuss it with others on the team. Using manual runfiles handling like in this patch has the advantage that you can precisely control which actions are rerun when inputs change.

@katre
Copy link
Member Author

katre commented Jun 25, 2018

scala-runfiles.patch.txt

@johnynek
Copy link
Member

Thanks for looking into it.

I would really like it to be able to work as zipper does: we shouldn't have to care if a target executable is a java rule or a C++ rule. It should be transparent. Otherwise, refactoring upstream code can break downstream.

@benjaminp
Copy link
Collaborator

SkylarkActionFactory.run will magically add the the runfiles trees of executable rule attributes to the spawn inputs if it sees the corresponding executable in inputs (deprecated) or tools. That behavior is probably what's missing for toolchains; if you arranged for the executable toolchain attributes to end up in SkylarkAttributesCollection.executableRunfilesMap, the Scala rules would probably work as-is.

What I would like to see is the executable and tools parameters of ctx.actions.run to taking a FilesToRunProvider directly, so one didn't have to rely on implicit behavior.

@lberki
Copy link
Contributor

lberki commented Jan 15, 2019

@benjaminp , that looks like the exact right thing to do. Since that is more general than this bug, I started a thread about it here:

https://groups.google.com/forum/#!topic/starlark/3Oot_a5ife0

weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this issue Jan 31, 2019
… tools= and executable=.

This makes sense because FilesToRunProvider is the data structure that actually represents an executable. Previously, we had to resort to fishing out the runfiles of binaries using a back channel that knew only about the direct inputs of the rule being analyzed.

Fixes bazelbuild#5463.

RELNOTES: None.
PiperOrigin-RevId: 229699575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) type: bug
Projects
None yet
Development

No branches or pull requests

6 participants