Will it break people to make `--modify_execution_info` additive

61 views
Skip to first unread message

Romain Chossart

unread,
Sep 27, 2022, 4:07:03 AM9/27/22
to bazel-discuss
Hi,

I currently have an open PR to make the `--modify_execution_info` additive.

This is technically a breaking change; for example, given the following .bazelrc configuration...

    build --modify_execution_info=.*=+mylabel1
    build:specialconfig --modify_execution_info=.*=+mylabel2

...invoking `bazel build --config specialconfig //:mytarget` will tag the actions differently:
- before the change, it will only add `mylabel2`
- after the change, it will add `mylabel1` and `mylabel2`

I would like to gather feedback from people to understand if this will break people in practice, and hence, if this warrants the addition of an `--incompatible` flag or if we can avoid adding one of those altogether.

Thanks,
Romain

David Turner

unread,
Sep 27, 2022, 6:46:27 AM9/27/22
to Romain Chossart, bazel-discuss
Why not add a new flag like "--add_execution_info" instead?
Apart from being a breaking change, changing the meaning of "modify" from "in-place replacement" to "append to previous values" is very confusing.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/b685c962-0cc1-46de-afc9-a5af29b49cacn%40googlegroups.com.

Romain Chossart

unread,
Sep 27, 2022, 7:03:59 AM9/27/22
to David Turner, bazel-discuss
I think `--add_execution_info` would be misleading because we can both add (`--modify_execution_info=Mnemonic=+foo`) and remove (`--modify_execution_info=Mnemonic=-bar`) execution info. That behavior is not changing. 

Moreover, it is presumably more confusing to have two flags that do the same thing (add and remove exec info) where one is additive and the other isn't; especially when the new flag can do everything that the old flag can do.

Thanks,
Romain
--
Romain Chossart
Message has been deleted

Konstantin

unread,
Nov 12, 2022, 4:37:25 PM11/12/22
to bazel-discuss
When I had to use ` --modify_execution_info` flag I naturally implied its semantics to be additive. It took me quite a lot of headbanging that because I specify it twice (for two different mnemonics) the second one simply cancels the first - I totally expected both of them to be in effect. Now I specify them as a single flag and it harder to read and comprehend.
I support Roman's idea as a definite step towards common sense, but as it is formally a breaking change nevertheless I guess guarding it with the matching `--incompatible...` flag is in order.

Konstantin
Reply all
Reply to author
Forward
0 new messages