bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23k stars 4.03k forks source link

Validate that action mnemonics are string literals - makes allowing custom mnemonics safer #23575

Open sitaktif opened 1 week ago

sitaktif commented 1 week ago

Description of the feature request:

Customizing mnemonic for actions is a useful feature that improves the output for users and also allows build engineers to use features such as --modify_execution_info more effectively.

However, to avoid issues related to an explosion of mnemonics (which may cause problems) it was suggested that a safeguard should be added to Bazel to ensure that mnemonics are literals. This prevents users from creating a large number of actions named eg. "some-name-" + i in a loop or list comprehension.

Related issue: https://github.com/bazelbuild/bazel-skylib/pull/491

Which category does this issue belong to?

Starlark Integration

What underlying problem are you trying to solve with this feature?

Trying to improve safety.

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

sitaktif commented 1 week ago

@comius mentioned here that interning of string literals happens already. It looks like it is happening here.

Are we happy surfacing the the set of literals with an accessor and then bubbling it up? My slightly vague understanding is that the set of literals is relatively far from where we would do the check: we would check the mnemonic argument in a place where we understand that the expression is a ctx.actions.{write,run,run_shell} function.

An alternative that would make the check closer is to simply only allow literals for attributes named mnemonic in any function but that doesn't feel quite right as that would yield some false positives.