keith / rules_multirun

Bazel rules for running multiple commands in parallel in a single bazel invocation
Apache License 2.0
67 stars 14 forks source link

Target environment variables are not respected #3

Closed ashlineldridge closed 1 year ago

ashlineldridge commented 1 year ago

Hi Keith, thanks for the project.

I believe I've come across a bug where hardcoded environment variables aren't passed through to the target when it is invoked by multirun.

E.g., given the following BUILD.bazel:

load("@rules_multirun//:defs.bzl", "multirun")

sh_binary(
    name = "lint_script",
    srcs = ["lint.sh"],
    env = {
        "MULTIRUN_VAR1": "abc",
        "MULTIRUN_VAR2": "def",
    },
)

multirun(
    name = "lint",
    commands = [
        "lint_script",
    ],
)

And lint.sh:

#!/usr/bin/env bash

echo "I'm a linter!!!"
env | grep MULTIRUN
echo "Done"

I would have expected that bazel run //:lint would output the MULTIRUN_* variables passed through by the lint_script target, however it doesn't. But if I run the sh_binary target directly via bazel run //:lint_script then the variables are output as expected.

keith commented 1 year ago

Thanks for the issue. I agree we should figure out what provider has the env and fix this. But also the intent is that every entry in the commands list is actually a command target, which has an environment attribute that is respected. It's a bug that that isn't currently enforced.

ashlineldridge commented 1 year ago

Ah, OK. I noticed that the environment variables are passed through as expected when I wrap the target in a command but didn't realise that this should always be the case. Do you see any problem with a multirun target including another multirun target in its list of commands? Or should that also be wrapped in a command?

keith commented 1 year ago

The intent was for multiuns to only be able to depend on commands. But I'm not opposed to opening it up more for these use cases either