search

bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
393 stars 180 forks source link

Support custom `mnemonic` and `progress_message` #491

Open joshua-k-harris opened 7 months ago

joshua-k-harris commented 7 months ago

Adds support for adding a custom mnemonic and progress_message for copy_file, copy_directory and run_binary rules.

Addresses #489 and #426

comius commented 6 months ago

cc @brandjon

Setting mnemonics from targets is something we had huge problems with internally.

joshua-k-harris commented 6 months ago

cc @brandjon

Setting mnemonics from targets is something we had huge problems with internally.

Does this impact the inclusion of this change or can you restrict the use of the attributes internally?

comius commented 2 months ago

Setting mnemonics from targets is something we had huge problems with internally.

Does this impact the inclusion of this change or can you restrict the use of the attributes internally?

We can't restrict them internally - we only monitor them :(.

The attributes should be restricted in Bazel and this would be a very welcome contribution. The idea is, that ctx.actions.run(mnemonic) can only be set to a literal string. This way you can still use your own mnemonics, but you're more limited and you can't do for i in range(10000000): my_macro(mnemonic=i). (Creating 1M different mnemonics happened to us). Technically Starlark already has a bag of all the literals, because it's interning them. One would need to check that mnemonics is in that bag.

This PR is acceptable after the PR in Bazel.

sitaktif commented 4 weeks ago

Thanks @comius for the context. I opened https://github.com/bazelbuild/bazel/issues/23575 on the Bazel side.