Improving rules_meta

[fmeum]

@cpsauer Feel free to create issues or add your suggestions to this one. If you think that a live discussion could be fruitful, just send me an email or a DM and we can schedule a meeting.

[cpsauer]

Hey, Fabian. Sorry to be slow in getting back to this. I'd be inclined to start here in the open, but if you'd ever rather email or live, just say and I'm happy to :)

Just to start with goals: My aim here is to get to you whatever ideas might be useful from my implementation, rather than just have them sit in our scrapyard.

I do have some concerns about what's fully doable with the Starlark APIs we have--more on that in the backlinked issue--so maybe some of my Bazel issue cross-refs will also be handy.

At a high level, I think two ideas worth raising as potentially valuable are:

• API-wise, I think probably the function will probably need two configuration dictionaries, both the one you have, and another for lists that should be replaced.
  • I like your auto-detect approach. (If I'm reading your implementation correctly, append if list, replace otherwise.)
  • But the issue is that some of the lists (sadly) don't make sense to append to. (As examples: --fat_apk_cpu, --macos_cpus) I wish there were an API to detect what the command line does for each option, but I don't know of one.
  • Another, possibly better option would be to have a list of flags that should be replaced, with some good defaults, at least until there's such an API?
• All else being equal, it might be handy to choose an attribute name for the rule being passed through that'll be automatically followed by aspect based tooling, like, say, the IntelliJ plugin. "exports" might do the trick, while being semantically right.
  • (For a more full list, see DEPS in https://github.com/bazelbuild/intellij/blob/master/aspect/intellij_info_impl.bzl)

And here's the implementation I spun up one afternoon. Note that, as a quick effort, it doesn't handle a bunch of the cases you're aiming to, especially around executables. (We were just trying to configure libraries.) But it is very short and readable. Feel free to use whatever you'd like--or not, no pressure of course.

"""Easily creates rules that can be used to add configuration ("transition") nodes in the build graph.
And macros to create auto-configured versions of existing rules.

A (functioning) work in progress, that we halted work on because at the time, it turned out we didn't need to add custom transitions after all. Saved because we may very well need to solve this in the future, and this will save some tricky boilerplate."""


def _transition_rule_builder(settings_to_override_dict={}, settings_to_add_dict={}):
    """Helps create rules that apply the configurations in the arguments to the target they export."""
    return rule(
        attrs = {
            "exports": attr.label(mandatory = True),
            # Required for transitions: https://docs.bazel.build/versions/main/skylark/config.html#user-defined-transitions
            "_allowlist_function_transition": attr.label(default = "@bazel_tools//tools/allowlists/function_transition_allowlist"),
        },
        # Ideally we'd return *all* the providers of the exported rule, but alas, there's no api. https://github.com/bazelbuild/bazel/issues/14669.
        # Might be able to do better--or solve problems with executables, with some tricks in https://github.com/fmeum/rules_meta/blob/main/meta/internal/forwarding.bzl, linked from https://github.com/bazelbuild/bazel/issues/14673 
        implementation = lambda ctx : ctx.attr.exports[DefaultInfo],
        cfg = transition(
            implementation = lambda settings, attr : dict(settings_to_override_dict).update({k : settings[k] + settings_to_add_dict[k] for k in settings}),
            inputs = settings_to_add_dict.keys(),
            outputs = settings_to_override_dict.keys() + settings_to_add_dict.keys()
        ),
    )


def _configure_macro_creator(settings_to_override_dict={}, settings_to_add_dict={}):
    """Helps create a macro that emulates the ability to create configured variants of rules.
    
    Needed because you can't otherwise attach transitions to existing rules."""

    # transition_rule is conceptually hidden, but we need to return it to workaround https://github.com/bazelbuild/bazel/issues/14673. TODO If we pick this back up, we may be able to workaround this better by adding, e.g. name = "pure_transition" to the rule definition above, and then not have to return/export it.
    transition_rule = _transition_rule_builder(settings_to_override_dict, settings_to_add_dict)
    
    def configure(rule_to_configure):
        def macro_wrapper(name, **kwargs):
            wrapped_name = name + "-configured"
            transition_rule(name=name, exports=wrapped_name)
            rule_to_configure(name=wrapped_name, **kwargs)
        return macro_wrapper
    return configure, 

##################################################
# Some usage examples

apple_configure, _apple_transition_rule = _configure_macro_creator(
    settings_to_add_dict = {
        "//command_line_option:cxxopt": ["-fno-aligned-allocation"],
        "//command_line_option:objccopt": ["-fno-aligned-allocation"],
    }
)

# As usage example for configuration.bzl
# TODO a fuller implementation would also hide, e.g. empty.plist and maybe bundle versioning

"""Rules for building for apple platforms--like bazelbuild/rules_apple, but with better default configuration for Hedron.

If you need to bazel build a rule that rule from bazelbuild/rules_apple that's not here, you should wrap it, at least with apple_configure, or things won't build right, just like in the implementation of the rules below."""

load("@//Bazel/Platforms:configuration.bzl", "apple_configure")
load("@build_bazel_rules_apple//apple:ios.bzl", "ios_framework")
load("@build_bazel_rules_apple//apple:macos.bzl", "macos_bundle")


def hedron_ios_framework(name, families=["iphone", "ipad"], **kwargs):
    """ios_framework, but properly configured for Hedron and with some better defaults.

    For more, see https://github.com/bazelbuild/rules_apple/blob/master/doc/rules-ios.md#ios_framework"""
    return apple_configure(ios_framework)(name=name, families=families, **kwargs)

def hedron_macos_bundle(name, **kwargs):
    # TODO
    return apple_configure(macos_bundle)(name=name, **kwargs)

Hopefully there was something handy in there! If not--well, you're far ahead of me on this one :) And if there's anything you'd like to discuss, just shoot.

All the best,
Chris

[fmeum]

Hi Chris,

thank you very much for the valuable insights!

I switched to exports instead of target in 8d57be3. I had never looked into the IntelliJ aspect before, but certainly learned a lot from it.

I also added functionality to replace list-valued settings as well as a warning for the settings you mentioned as well as a few more in 0d8442f.

I am also on the lookout for rulesets that might adopt this macro and will work on cleaning up the implementation in the meantime.

[cpsauer]

Great! I'm so glad there was something useful to you in there :)

For users: I always find commenting back on the issues a good way to get users--maybe that'd be a good first move once you've got things release ready, documented, licensed, etc.? For your target user, I'm guessing rulesets that define things from scratch can already share enough code between rules without this (rules_apple is the example in my head). But what about an open source end user that would otherwise require lots of configuration from the command line? like Tensorflow, maybe?

[Feel free to close this down, but I'm always a message away if you need/want me.]

[cpsauer]

@fmeum, is this still something you're planning on working on and releasing? Found another use case for it in our repo, but didn't want to use it without your permission (and license).

[fmeum]

@cpsauer I haven't found a home for it yet, but did improve the API to the point where I would feel comfortable using it (at least experimentally). I just added a LICENSE file and created a v0.0.2 release. Let me know if you need anything else. I will work on improving the docs when I have time.

[cpsauer]

🥳 Reporting back that we've swapped in rules_meta for our homegrown implementation, and it's working great.
Our use case is fairly simple, but you handle all sorts of things our implementation didn't. Amazing!
We're delighted to get to use yours, now and into the future :)

Thank you so much, @fmeum, for doing such a great job on this and for sharing it with the world. I also super appreciate your responsiveness to feedback and just all around great attention to detail.

-Chris

[cpsauer]

Just wanted to report that we're still super happy to be using this, @fmeum. Thank you!

[fmeum]

@cpsauer Glad to hear that, thanks for the update!

I just tagged https://github.com/fmeum/rules_meta/releases/tag/v0.0.4, which comes with full support for env and env_inherit on native rules with recent rolling releases. Let me know if you get to give it a try.

[cpsauer]

Yay! Great! Will upgrade
[We're on Bazel rolling, though not working with the env much.]

Again, thanks so much! There's so much need for this in Bazel.

[cpsauer]

[Commenting here, converting it into a space for us to discuss bazelbuild/bazel/issues/16301, since this one's still open and I think otherwise resolved. ]

Assuming it lands, how are you thinking we should work around it?

In particular, any better ideas than returning both the rule and macro to the user, with the instructions to store the rule in an arbitrarily-named, unused variable? (Assuming that still works.)

Thanks again :)

Chris

[fmeum]

Yes, I believe that once the incompatible flag goes away, that's our only option. I expect it to continue to work that way since I actually used this pattern in the beginning before I discovered the name attribute.

[cpsauer]

Put a bit smile on my face hearing you in the meeting today, including the configurability nod!

[fmeum]

@cpsauer I am planning to submit a BazelCon talk on a complete "v2" overhaul of this ruleset.

[cpsauer]

🥳

[cpsauer]

Fabian, I just updated up to the latest bazel rolling (from 5/30 or so) and noticed that --noincompatible_remove_rule_name_parameter now seems to be a no-op, sadly, meaning rules_meta can't be used, I don't think.

Any chance you'd be willing to push up the also-return-the-rule fix, above, just to unblock things? Or any chance you have an early version of that v2 we could hop onto?

[fmeum]

You can already find examples for v2 on the redo branch. It doesn't have docs yet and hasn't been cleaned up, but it should mostly work, including direct support for select()s. It does require a recent rolling release.

[cpsauer]

Sweet! Thank you. Will give it a whirl when I get back coding tm, but in the meantime, thoughts on just making that the main branch, since limited audience and no longer working with bazel rolling?

[cpsauer]

Also, I've always meant to ask: Are you a Googler, Fabian? I see the MTV Android in you profile picture--but saw other company info on your bio?

[fmeum]

Sweet! Thank you. Will give it a whirl when I get back coding tm, but in the meantime, thoughts on just making that the main branch, since limited audience and no longer working with bazel rolling?

I am planning to do a final cleanup this week and then make it the main branch. I may rename the repo to with_cfg along the way as rules_meta always felt a bit... pretentious?

Also, I've always meant to ask: Are you a Googler, Fabian? I see the MTV Android in you profile picture--but saw other company info on your bio?

I am not, it's just a picture I took during an internship at Google and kept since for lack of an equally fun one :-)

[cpsauer]

Awesome. Thank you! Maybe I should just hop on it next week when you're ready? Could I ask you to comment back, pinging me when you release?

with_cfg definitely succinctly describes the use case--I like it!

:) I too have fond Google internship memories ('15 and '17)! Wish we'd gotten to know each other there, if there was overlap. What team(s) were you on?

[fmeum]

I published the repo: https://github.com/fmeum/with_cfg/blob/main/docs/defs.md

An actual README and releases will follow.

(GMail, in late summer of 2015)

[cpsauer]

Just switched our team over! You have your first happy user :) Thanks so much, Fabian.

And that means we did overlap! I was on Research that summer :) Bummed I didn't get to meet you then.