bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Optionally infer name in `use_extension` and `use_repo_rule`

Open fmeum opened this issue 1 year ago • 8 comments

Following the documented best practices for rules and extensions, a symbol named foo_deps should be available from a .bzl file called foo_deps.bzl. While this already reduces cognitive load during "Where do I get this extension from?", it still results in pretty repetitive MODULE.bazel lines:

foo_deps = use_extension("@rules_foo//foo/extensions:foo_deps.bzl", "foo_deps")

Instead, Bazel now uses this convention to infer the name if not given:

foo_deps = use_extension("@rules_foo//foo/extensions:foo_deps.bzl")

This makes typos less likely and also nudges ruleset authors to adopt the new convention by providing nicer syntax.

RELNOTES: use_extension and use_repo_rule now infer the name from the basename of the .bzl file if not provided explicitly.

fmeum avatar Apr 29 '24 06:04 fmeum

Note to self: Make buildozer aware of this too when accepted.

fmeum avatar Apr 29 '24 06:04 fmeum

This seems a bit too magical to me; I don't think saving the extra parameter is worth the potential confusion. Open to be convinced otherwise though.

Wyverald avatar Apr 29 '24 22:04 Wyverald

My own thoughts on why I personally consider this a net win:

  • Since use_extension differs from load in so many ways already, losing one more similarity with it doesn't matter too much.
  • In a future world where every ruleset follows the style guide, the two arg variant will just no longer be relevant.
  • A number of languages already follow the "filename is main entrypoint" paradigm, for example Java and JS modules.
  • Compared to other approaches such as enumerating all symbols in the file and picking the unique one if it exists, there isn't any dynamic magic at play here, just string manipulation on an explicitly provided argument.

I will try to get more eyes on this by posting about it in the #bzlmod channel.

fmeum avatar Apr 30 '24 20:04 fmeum

My two cents. I like this magic. More wizardry and unicorns, please. 🦄🧙‍♂️

cgrindel avatar Apr 30 '24 21:04 cgrindel

@alexeagle @aherrmann @keith @illicitonion for their opinion on this idea

fmeum avatar May 08 '24 10:05 fmeum

I also fear that this is a bit too magical.

On the pro side

  • it does reduce noise and boilerplate a bit,
  • foo = use_extension is different from load in that it only brings one symbol into scope and that is returned and assigned,
  • arguably there is precedent for this kind of thing with the @foo and //bar label short-cuts.

On the cons side

  • it seems to be optimizing more for writing than reading code,
  • it's yet another special case for people to learn,
  • I fear it will raise an expectation that the same kind of short-cut exists for load("@rules_lang//lang/lang_binary.bzl").

Question

  • Why is use_extension so different from load? Why don't we write use_extension("@rules_foo//foo/extensions:foo_deps.bzl", "foo_deps")? That would also eliminate one repetition of foo_deps, and a different name could be assigned with kwargs as with load.

aherrmann avatar May 08 '24 11:05 aherrmann

Bazel has existing semantics for the default label in a package, which uses the same convention for "the symbol named the same as the package"). I see that as an argument that the same concept could apply in starlark.

I fear it will raise an expectation that the same kind of short-cut exists for load("@rules_lang//lang/lang_binary.bzl").

I had the same thought - every time I load from skylib I'm typing the same thing twice, but no one is making "default symbol to load"

Since use_extension differs from load in so many ways already, losing one more similarity with it doesn't matter too much

I think this does matter quite a bit - it increases the cognitive burden of understanding how Bazel works.

symbol named foo_deps should be available from a .bzl file called foo_deps.bzl.

We still have most repos with extensions.bzl which was the common convention for a year or so - and since they don't plan to grow more extensions it will probably stay that way. In those cases we don't feel this problem.

alexeagle avatar May 08 '24 13:05 alexeagle

We still have most repos with extensions.bzl which was the common convention for a year or so - and since they don't plan to grow more extensions it will probably stay that way. In those cases we don't feel this problem.

Very good point. AFAIK there is also no way for Bazel modules to change this without incurring a breaking change, see "extension identity" docs. So, it seems unlikely that we'll see a wider shift to the new recommendation soon.

aherrmann avatar May 08 '24 15:05 aherrmann

I fear it will raise an expectation that the same kind of short-cut exists for load("@rules_lang//lang/lang_binary.bzl").

That's a very good point. Let's wait until Starlarkification has progressed to the point where these loads are actually found in all build files. If load gets this behavior as a way to reduce boilerplate, then use_extension probably should as well, but if it never gets to that, the increased cognitive load would not be worth it.

I will close this PR for now and revisit it later.

fmeum avatar May 15 '24 07:05 fmeum