Buck: disable_implicit_native_rules doesn't work in Skylark parser

[linzhp]

This commit introduced a new parser config called "disable_implicit_native_rules". However, when the parser is set to Skylark, the value of disable_implicit_native_rules is ignored and implicit native rules are always disable in bzl files.

[philipjameson]

Well, that definitely shouldn't have happened. Lemme take a look .

[philipjameson]

Actually, on a reread, this was intentional after discussing with @ttsugriy. It wasn't actually supposed to work that way to start with, we just did it because it was easier for some of our migrations. @ttsugriy , thoughts on making this configure extension files as well?

[linzhp]

This is not very backward compatible. After pulling your commit, all our builds failed in a sudden...

[philipjameson]

No it's not, though I believe the skylark parser is marked experimental and cards on the table, I didn't know that anyone really was using it yet other than FB. Mind if we meet and the middle and just toss a config flag to re-export them? I think going forward we'd like new projects to be explicit about using native rules.

[linzhp]

To be future-proof, should we replace all go_library with native.go_library and genrule with native.genrule in both BUCK and .bzl files?

[philipjameson]

.bzl files should use native.go_library and native.genrule. By default (unless you disable it), go_library and genrule will work in BUCK files without the prefix.

[linzhp]

OK. Out of curiosity, could you explain the design decision behind requiring explicit native functions? Or give me a pointer to some existing discussions/documents?

[ttsugriy]

@linzhp There are a number of reasons we're doing it this way, but they can be broadly grouped into following:

• we would like to keep the global namespace pollution as much as possible. It makes it easier to reaon about code without having to scan an entire document to check if function call is for a native rule or a macro. Unfortunately it's not uncommon for macro writers to override/shadow native rules, so having an explicit namespace for them makes this distinction easier.
• we'd like to be as close to Bazel API as reasonably possible. We've met Google, Twitter and Microsoft and would love to work with them on standardizing extension definitions to democratize them.

Is there anything we can do to make this transition smooth for you? It was arguably my mistake to allow native rule usages without native. prefix, which was mostly prompted by migration from Python DSL and I'll be happy to help you with any migration issues you have.

[linzhp]

Thanks for the explanation. It's very helpful. Migration wise, we are fine right now, as we only have one centralized bzl file, and it's just a simple string search and replace, although it came out as a big surprise yesterday, and we were trying to understand whether it was a bug or feature.

The doc on parser.disable_implicit_native_rules doesn't point out in intricacy of its effect on Python DSL/Skylark parser and BUCK/bzl files, which may be confusing:

PYTHON_DSL:

• BUCK & bzl: whether to require native rules with native. prefix

Skylark:

• BUCK: whether to allow native rules at all. When parser.disable_implicit_native_rules is on, no native rule is allowed in BUCK, even with native. prefix
• bzl files: no effect. native. prefix is always required regardless of the setting.

Can you verify whether these are the desired behavior. If they are, you may want to make it explicit in the doc.

[ttsugriy]

It's my bad, @linzhp. Because we are so heavily invested in internal migration to Skylark, I didn't focus much on providing documentation and higher level picture as to how these parsers compare. We have plans to change this and create a dedicated documentation for Skylark on buckbuild.com. I'll try to work on it as soon as I find some cycles (hopefully in the next couple of weeks). We don't plan any large changes for Skylark API anymore, but please let us know if we accidentally introduce some incompatibility. Thank you for using Skylark!