Exec bazel instead of subprocessing #566
Conversation
|
@aaron-skydio I like this PR - I think it's a more definitive solution to the signal handling issues I partially fixed in edab5e0. Mind rebasing onto master so we can get this merged? |
|
Done! |
|
|
||
| // RunBazeliskWithArgsFuncAndConfigAndOut runs the main Bazelisk logic for the given ArgsFunc and Bazel | ||
| // repositories and config, writing its stdout to the passed writer. | ||
| func RunBazeliskWithArgsFuncAndConfigAndOut(argsFunc ArgsFunc, repos *Repositories, config config.Config, out io.Writer) (int, error) { |
There was a problem hiding this comment.
The removal of this function will break users (#526), so I'd like to keep it.
Also, in the current version the out parameter of runBazel is basically useless since it will always be nil.
There was a problem hiding this comment.
Ah thanks, I didn't realize bazelisk supported use as a library so I just looked for uses in bazelisk itself. Added it back.
Are there going to be callers who need to call one of these RunBazelisk functions with out == nil and have it return instead of execing? The way I've written this right now doesn't support that, and I'm not sure how you'd rather me do that if we need that - duplicate all these public functions with ExecBazelisk copies? Or add a flag to exec if possible or not?
There was a problem hiding this comment.
Ok I think if I were a user I'd want to have the behavior of these preserved to return instead of execing, so I've added Exec versions of the ones that make sense (without out) and left the Run ones to still run in a subprocess and return
Instead of running bazel as a subprocess of bazelisk, replace bazelisk with bazel in the same process (in the case when bazelisk is only being used to invoke bazel once and then exit, as opposed to invoking bazel multiple times or post-processing output from bazel). This eliminates several classes of issues.
A couple concrete examples:
bazel run //:something_that_segfaultswill not cause bash to display that the process segfaulted if bazelisk subprocesses to bazel, but it does if bazeliskexecs bazel.bazel, even oncebazel runhasexecd the binary it built, since the running top-level process is still bazelisk; they should display the name of the binary, and they do after this changesyscall.Execis not supported on windows, so this only applies to linux and macos.