Bazel: add an install shortcut and an experimental attribute to codeql_pack
#18023
Conversation
This makes the first `codeql_pack` in a package add an `installer` target aliasing the `<name>-installer` one. This makes it so that one can for example do `bazel run //rust:installer` instead of the stuttering `bazel run //rust:rust-installer`. If a bazel package defines multiple `codeql_pack` targets, the first one only will get the `installer` alias.
github-actions
bot
added
Rust
redsun82
changed the title
installer shortcut to codeql_packinstall shortcut to codeql_pack
criemen
left a comment
criemen
left a comment
There was a problem hiding this comment.
Approving if you still want to merge this, despite the point I'm raising.
misc/bazel/pkg.bzl
Outdated
| visibility = ["//visibility:public"], | ||
| ) | ||
| _codeql_pack_install(internal("installer"), [name], install_dest = install_dest, apply_pack_prefix = False) | ||
| if not native.existing_rule("install"): |
There was a problem hiding this comment.
I don't like this implementation - per https://bazel.build/rules/lib/toplevel/native#existing_rule, this API will be deprecated and removed at some point:
If possible, use this function only in implementation functions of rule finalizer symbolic macros. Use of this function in other contexts is not recommened, and will be disabled in a future Bazel release; it makes BUILD files brittle and order-dependent.
Given the fairly small impact of this, I'm fine merging this now, but we will have to remove this feature again once bazel follows through with the removal of this function.
There was a problem hiding this comment.
good point! I wasn't even aware of that (or of the new macro definition framework)
I agree it's better to avoid that, and have changed the code to do that, and added an experimental flag to aid the definition of experimental packs. Alas, I still needed existing_rule because the internal repo defines two codeql_packs in a bazel package (the java single and multi kotlin variants). So for backward compatibility I've left that. Once the internal repo is updated (probably we can put install_dest = None on all codeql_packs there), we can remove TODOs here with a third PR.
redsun82
changed the title
install shortcut to codeql_packinstall shortcut and an experimental attribute to codeql_pack
criemen
left a comment
criemen
left a comment
There was a problem hiding this comment.
Thanks, that looks much better!
This makes the first
codeql_packin a package add aninstalltarget aliasing the<name>-installerone. This makes it so that one can for example dobazel run //rust:installinstead of the stutteringbazel run //rust:rust-installer. If a bazel package defines multiplecodeql_packtargets, the first one only will get theinstallalias.