Refactor write_launcher #449
Conversation
|
I wish I understood runfiles better. :( So, I tried to add a default repl rule, but it does not work yet and I don't know why. I can't seem to get a runfiles directory created for the library target. Without a runfiles directory, the script we write fails. I'm confused because return runfiles in the rule: we aren't using DefaultInfo, which seems to be the current way of doing things, but it seems that returning a struct should also work. I don't see what the difference is here: which does work. As a start, I tried to just copy as much of the scala_binary as possible, but like I said, nothing I did seemed to make it work... I have to say, I am very sympathetic to people who get frustrated about bazel because issues like this are pretty hard. You can google for related confusion on runfiles, so clearly I'm not the only one who has issues with this. If anyone has time to look at what is going on, I would love any help. PS: I'm sure the first thing any bazel team member will say is we are using like 10 deprecated things. Yes, that's what happens when you release software that deprecates APIs every month. cc @dslomov I have asked you previously about runfiles, and I thought the answer was just return a runfiles attribute from the rule implementation. Am I making any obvious errors? |
|
@ittaiz despite not yet being able to get the default repl support working I still think this is worth merging. At least it removes the duplication of the wrapper file name. |
|
@ittaiz what about this one? Can we merge? |
|
I'm sorry :( didn't get around to it yet. |
|
take your time... (to a point. ;) |
scala/scala.bzl
Outdated
| """This creates a wrapper that sets up the correct path | ||
| to stand in for the java command.""" | ||
|
|
||
| runfiles_root = _runfiles_root(ctx) | ||
| # RUNPATH is defined here: |
There was a problem hiding this comment.
shouldn't this comment move to _write_executable?
|
ping @johnynek about the comment and you have a conflict |
|
I'll try to get back to this, I was very busy at work this week and I got a new computer, so I have to setup the machine still to be able to push. |
|
@ittaiz okay, I have merged with master and moved the comment. See what you think. |
* Refactor write_launcher * move a comment to the appropriate place
This is a minor refactor of write_launcher to prepare for a change to make a repl script for all targets.
One cleanup here is that we had repeated ourselves with the name of a launcher script. Here we explicitly plumb this through so we can define it in a single place and be more honest about the dependencies.
Secondly, we make it clear that we only need a single wrapper script for each target (really each directory I think), so we can share a wrapper between the repl launcher and, for instance, the binary executable.
ptal @ittaiz