sh_config: Add BAZEL_SH

[meteorcloudy]

No description provided.

[fmeum]

Makes sense to me, but could be mildly breaking. Could you explain why you think we should change this now?

[meteorcloudy]

I was just debugging another issue, and found this comment. I think at least adding BAZEL_SH should be fine? Do you mean changing from local = True to configure = True could be breaking?

[fmeum]

Yes, that's the part I'm worried about. But adding the dependency on the variable seems like a clear win even if we keep local = True. We could just use getenv throughout.

[meteorcloudy]

I see, let's only add BAZEL_SH for now.

[fmeum]

Now that I've hit merge, I remember why I didn't do this immediately: I recall seeing docs that mentioned that you would only need to set BAZEL_SH when starting the Bazel server. That only remains effective if Bazel doesn't watch the environment variable. In that sense, this change is probably even more breaking without local = False and configure = True.

I'm not sure what's best, but we could just go through with your original PR.

[meteorcloudy]

I recall seeing docs that mentioned that you would only need to set BAZEL_SH when starting the Bazel server.

Hmm, I actually don't remember this, do you also remember why this is needed?

[fmeum]

Probably only for historic reasons, see https://bazel.build/reference/command-line-reference#flag--shell_executable. configure = True and local = False seem like the best compromise.

[meteorcloudy]

Thanks, let's do it them: #19