atlassian / bazel-tools

Reusable bits for Bazel
Apache License 2.0
113 stars 36 forks source link

Support setting environment variables and additional arguments #45

Closed ash2k closed 5 years ago

Globegitter commented 5 years ago

As a first impression, it does make it a bit more verbose but I do quite like the idea of separating it out this way. I will give it a try in our project and then come back with feedback.

Globegitter commented 5 years ago

What I also stumbled across and seems somewhat related: https://github.com/bazelbuild/bazel/issues/3862 but I guess that would mean one would need to use an "alias" to run the same command multiple times. I guess the other option could be having a string_list_dict_list and then we could have something like:

commands = [
 {
    "cmd": ["//comand"],
    "args": ["--arg1", "val", "-b"],
    "env": ["ENV1", "value1"]
  }
]

but even if any of that would exist it still feels a bit awkward having a list for the cmd property - anyway let's see how this proposal works out :)

Edit: Sorry I just keep thinking about different designs now before even having tried out the one you proposed so please just see this all as just laying down my thoughts somewhere that might just get disregarded. Writing that down just helps me thing through the options. Another option could be using the already existing string_list_dict:

multirun(
    name = "run_all",
    commands = [
        ":command1",
        "//some/other:label",
    ],
    args_for_commands = {
        "cmd1": ["--arg1", "value1", "-b"]
        "cmd2": []
    },
    envs_for_commands = {
        "cmd1": ["ENV1", "value1"]
        "cmd2": []
    }
)

where-as this really just a way to get nested lists to make the parsing easier but I guess that is also not so nice/clean because is the question if the dict has a stable key order, or we would have to rely on information in the key like map a key containing 1 to the first command and so on, also not feeling so clean.

ash2k commented 5 years ago

Yeah, I had this idea too. Instead of cmd1/etc we could use label's name to avoid relying on iteration order but that may be even more verbose. Also, we may want to add more customisation to each command in the future so separating each one out may make more sense because it is more future-proof.