Next steps for Starlarkifying native rules

[Lukács T. Berki]

Hey there,

About a year and half ago, we made the plan to require load() statements for every rule currently implemented in Java so that we can seamlessly migrate them to Starlark.

In the meantime, it turned out that that's way too intrusive a change because it requires changes to literally every single BUILD file, which is too much, even with the help of Buildozer, and we'd still have a version skew problem between the rule definitions and Bazel itself.

We are still planning to rewrite the native rules in Starlark, but the plan is different: we'll include the .bzl files in Bazel and inject them into BUILD files using the "Injecting Starlark Values Into Builtins" plan by Jon. That way, we'll not need any changes to BUILD files and we won't have to deal with the version skew.

To this end, I (admittedly somewhat too unilaterally) made the --incompatible_load_*_rules_from_bzl flags for protobuf and Java no-ops and I'm planning to do the same for the rest. In addition, well remove the addition of the load statements from Buildozer.

Does this sound like a good plan? (if not, I'll atone for my sins by rolling back the above changes to protobuf and Java!)

--

Lukács T. Berki | Software Engineer | lbe...@google.com | 

Google Germany GmbH | Erika-Mann-Str. 33  | 80636 München | Germany | Geschäftsführer: Paul Manicle, Halimah DeLaine Prado | Registergericht und -nummer: Hamburg, HRB 86891

[Laurent Le Brun]

Is it the long term plan to keep the .bzl files inside Bazel, or is it just for the migration?

Laurent

--
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/CAOu%2B0LUGYGGaFAPnqYY7Xw8dYcdFX92FycYiRGd2AxqMLZ5rgw%40mail.gmail.com.

[Andreas Ames]

Hello Lukács,

how to proceed with those BUILD files, which have already been changed according to the original plan? Will there be support to remove them in "buildifier" or something?

Thanks in advance ...

--

[Lukács T. Berki]

@Laurent: I think the eventual end game is for them to be moved out of the binary, but I think it's far enough that I'm not even planning for that future.

@Andreas: good question. I think it's best to let Buildozer remove these statements, but it's quite a bit of churn. Alternatively, we can leave them in and ignore them. Jon, WDYT?

[Yannic Bonenberger]

If the rules will eventually live in rules_{cc,java,proto,python}`, then I’d vote for not adding a buildifier

step to remove the loads. It doesn’t sound like a good idea to add them, remove them, and then add

them again. For folks that care about the loads being not present, we can add a generic

 `bulldozer remove_load` command (which *should* be relatively straight forward to implement).

I’d also vote for keeping the buildifier fixes for the statements, but to disable them by default.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAOu%2B0LUGJ6F4d8uduLPY_%3DdSQzy%2BgSXE%3DB6r76eQyf66h_2O_A%40mail.gmail.com.

[Lukács T. Berki]

The churn would indeed be a problem, that's why I'm reluctant to just pull the plug. I guess we could work around that by just stipulating that the native rule proxy rules never change in the rules_* repositories and then Bazel works the same regardless of whether the load() statements are there or not?

That would have the side effect of not letting us remove the declaration of rules_{proto,cc,java} from the WORKSPACE file fragments embedded in Bazel, which would be nice. But then again, we can only do that if everyone removes their load() statements which is yet another very intrusive change of the likes I spoke against in the message that started this thread...

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/63A40F6B-BBD3-45C8-BB4A-EFD048CDF51F%40yannic-bonenberger.com.

[Keith Smiley]

+1 on not removing the loads. I'm not sure who is hit by too much churn from adding the loads but we happily added the loads when that rule was originally added to buildifier and would prefer they remain explicit especially if that's the long term plan.

[Jon Brandvein]

The original reason we didn't proceed with the load_*_rules_from_bzl flip was that it was a ton of churn, not all of which was automatable at the time, for no immediate benefit to the user. It also would have created version skew issues that we now will avoid by migrating to Starlark before debundling the .bzl files from Bazel. The only reason we made a push to do the flip last year was that we thought we'd get this invasive change out of the way in time for Bazel 1.0.

Long term, bazelbuild/rules_cc (etc.) should still be the home of the debundled rules, so it still makes sense for the flags to exist, even though we're not ready to flip them. In the meantime, migrating early should cause no harm, and we should continue to support (but not necessarily encourage) migration in buildifier. Note that migration does not currently create version skew issues, because there's no internal API between Bazel and rules_cc, aside from one secret handshake tag that tells Bazel not to complain about rules_cc instantiating the native rules.

I certainly don't think migrating away from load()s and back to them will help anything. The load()s also are an ideological reminder that we do intend to debundle these rules eventually. But I also don't oppose adding a buildifier undo-the-load()s operation if users want to go that route.

One more detail: Bazel has an optional prelude file that causes, on a per-repo basis, some symbols to be predeclared in BUILD files. We have ambiguous feelings about this prelude feature, but some users really like the ability to customize their BUILD dialect so that they don't have to load frequently used symbols. Assuming we don't mind encouraging more people to use this feature (i.e., if we commit to keeping it around), we could have people migrate by using the prelude instead of adding load()s everywhere. Though they'll still have to update their WORKSPACE file and any bzl files that access native.cc_library.

[Xudong Yang]

My idea surrounding this is that we can use the new external deps ("bzlmod") launch and subsequent migration to our advantage.

So in the new world where repository rules are no longer a thing, we need to get rid of Bazel's WORKSPACE suffixes. A natural place for them to go would be into the rules_* modules. This would mean that, for a user to use bzlmod and e.g. cc_library, they would have to add rules_cc as a dependency, since that's where the local toolchains are defined, among other things. At this point it feels natural that the actual cc_library BUILD rules should be loaded from rules_cc as well. (This is of course predicated on the fact that we do eventually want to ship those BUILD rules separately from Bazel.)

bzlmod is definitely going to need some sort of automatic migrator script, to help people along with the migration even though it might not take care of everything. One thing the script probably needs to do is to scan all the BUILD files, see that there are cc_library/cc_binary calls, and automatically add a dependency to rules_cc. Then it could conveniently also just insert a `load` statement at the top of those BUILD files. Suddenly, it doesn't seem like so much churn anymore, since it's all done in one fell swoop!

So all in all, we have a path to an eventual "clean" world, with a way to mitigate the churn.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAEoyrevL%3DwdhR2Mc0%3DVk%3DtO_D2X6V1SBeH9_RWWGEnTpeBVHcA%40mail.gmail.com.

[Lukács T. Berki]

Hey,

Apologies for the late reply! What I'm hearing is that we should keep the load() statements since they still point to the end state where we want to be and adding them, removing them, then adding them again would be way too much churn. This, as Xudong says, leaves the question of how we load these repositories if not with workspace prefixes / suffixes. I thought that since the embedded Starlark rules will be special-cased anyway, it doesn't harm anyone too much to special- case the @rules_cc / @rules_java repository names also that they also point to .bzl files embedded within Bazel itself. Xudong, WDYT about this not very principled, but definitely simple and straightforward solution?

[Jon Brandvein]

From my recollection last time Xudong and I spoke: The issue isn't so much the special casing of the @rules_cc name in Bazel. It's that @rules_cc will not have an implementation within Bazel anymore, since non-trivial (i.e., non-"local_repository") repository rules won't exist within WORKSPACE files. Thus, a user who for whatever reason avoids using a tool like bzlmod to generate their WORKSPACE file will be responsible for providing whatever WORKSPACE content is needed for cc_library and friends to work.