Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let targets set their flags / platforms without command-line flags #14669

Open
cpsauer opened this issue Jan 29, 2022 · 30 comments
Open

Let targets set their flags / platforms without command-line flags #14669

cpsauer opened this issue Jan 29, 2022 · 30 comments
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability Issues for Configurability team type: feature request

Comments

@cpsauer
Copy link
Contributor

cpsauer commented Jan 29, 2022

Hello, wonderful Bazel folks! Thanks for all you do.

[Readers: As an update, note that this issue has since evolved considerably, as we've moved up the goal backtrace, so you'll need to read more than just this first post.]

This is an extensibility feature request.

It'd be awesome to be able to get the list of providers in a Target inside a rule implementation function, via ctx.attr.<name>.providers or similar.

I think this is not currently possible from Starlark, both from the docs linked above and from poking around with dir() and such, and that there is no previous open issue about this, but I'm hoping you'll correct me if I'm wrong.

In case it's useful--or there's a better way, here's the backtrace of goals that led me to this:

  • Define a rule that passes through all providers.
  • Create a rule whose only function is to add a transition that configures existing rules.
  • Configure existing rules that cross-compile to another platform to automatically use special flags just on that platform. (So they can directly be built, rather than needing being paired with --config=)

Thanks for your consideration,
Chris
(ex-Googler)

P.S. I also wasn't able to quickly find how you'd add a feature like this. Any chance someone know where the right docs are--or a good example? I found

public interface TransitiveInfoCollectionApi extends StarlarkValue {}
but wasn't sure how things were actually hooked up.

@cpsauer cpsauer changed the title Expose List of Providers From Rule Expose List of Providers From Rule in Starlark Jan 29, 2022
@gregestren
Copy link
Contributor

gregestren commented Jan 31, 2022

I believe this is an intentional design decision (but I wasn't one of the deciders so I don't quite know the rationale).

Passing providers through transition wrappers is exactly where I learned about this too. :p And why we haven't gotten a better solution to that.

@cpsauer
Copy link
Contributor Author

cpsauer commented Feb 4, 2022

Indeed... I've since run across #11458, for example, where the use case it was being requested for is decided to be misuse.

I guess the broader question is: If there's a good use for it for configuration (like to accomplish the goals in our discussion in #14673), is disallowing it still the right intentional decision?

@brandjon
Copy link
Member

Greg's right that it was an intentional decision to not make the list of available providers on a target programmatically accessible to client code. (Even for native code inside Bazel it's difficult to get that.) The rationale is that you should be required to have your hands on a provider symbol in order to be able to manipulate instances of that provider. I.e., you can ask if the target has a FooInfo, assuming you loaded FooInfo, but you can't just iterate over all provider instances to see if there's one by that name.

This gives the provider author (and code written to use that provider) better control and guarantees over its instances, so that other rule sets, including generically written code, doesn't violate whatever invariants the provider logic has.

Perhaps there are arguments for relaxing this protective approach. But it would be a significant departure from where we started back when symbolic providers were added to replace struct providers.

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Feb 10, 2022
@gregestren
Copy link
Contributor

Ah yes, that is a good argument.

I'm not sure if it's quite the same argument as symbol vs. struct, but there's a broad API vs. implementation concern. I remember back before providers even existed (in any form). Parent targets would directly access their dependencies, usually by looking for a specific rule type, and often reach into their implementations. Even worse, this allowed recursive access to their dependencies' dependencies. Which created serious efficiency issues: you couldn't analyze a target without having its complete subgraph available. And general build structure brittleness, inflexible graph structures, leaky APIs, etc.

Exposing the full list of providers doesn't bring us back to that state. But it opens more opportunities to infer more about a dep than providers are intended to expose.

If this is really strictly about transition wrappers, which I agree don't work well for this purpose, we could explore custom APIs for that use case, maybe something along the lines of aliases.

@fmeum
Copy link
Collaborator

fmeum commented Feb 10, 2022

If this is really strictly about transition wrappers, which I agree don't work well for this purpose, we could explore custom APIs for that use case, maybe something along the lines of aliases.

An idea that wouldn't break provider encapsulation bit would be very useful in transition wrappers: Make it possible to forward all providers from a given target except those that are returned explicitly. This is more general than alias, but allows for necessary modifications such as to DefaultInfo.

@keith
Copy link
Member

keith commented Jun 7, 2022

An idea that wouldn't break provider encapsulation bit would be very useful in transition wrappers: Make it possible to forward all providers from a given target except those that are returned explicitly.

Something like this seems necessary to be able to use custom transitions for common cases. A very common case I see with transitions is just trying to do something like this trivial example from the docs https://docs.bazel.build/versions/4.0.0/skylark/config.html#incoming-edge-transitions, which you cannot actually do for many cases unless you manually enumerate all possible providers that you want to re-propagate. Even if you are willing to do this you hit 2 non-obvious issues, 1 is private providers which you likely can't access from the dep (maybe you can use some getattr hacks to get it), 2 is configuration specific providers like those used for code coverage that aren't always present.

@fmeum
Copy link
Collaborator

fmeum commented Jun 7, 2022

Maybe the right API for this would be based on this proposal by @comius: Turn a Target into something mutable, mutate it and return it from a rule instead of a list of providers.

There is one other point that is very tricky when forwarding providers: The requirement that an executable has to be generated by the rule that declares it. This means that DefaultInfo has to be changed in non-trivial ways. If there were a way to lift that requirement, it seems that more often than not would it be possible to just forward all providers unchanged. Can anyone shed some light on why that requirement exists?

@gregestren
Copy link
Contributor

@cpsauer: is it fair to say this issue is about transition wrappers? And that an API to get a target's providers is just one possible way to support that? If so, could we rephrase the issue for the general problem so we don't get caught up on the challenges of a particular solution?

@keith naive question, but how does that fit with the incoming edge transition docs? Isn't this more an issue with outgoing edge transitions? i.e. for an incoming transition the rule self-applies its transition so there's no other rule to forward providers for.

@fmeum I have a sync scheduled with @brandjon soon. I'll bring these latest thoughts up then, however we process through them.

@cpsauer
Copy link
Contributor Author

cpsauer commented Jun 23, 2022

@gregestren: Definitely; thanks for bring us back up the "goal backtrace." Better to solve the need the right way rather than having us get too focused on the way I'd initially tried to hack a solution together.

Before I edit, I'm wondering if we shouldn't root things one level higher up still, and descend from there. The real goal motivating this (and #14673) is enabling "flagless builds" where targets can easily specify their necessary configuration in the build graph rather than requiring command line flags to build properly (or a bunch of custom rule work). I'm adopting your "flagless builds" name--from our discussion in #14673--because I think it's far and away the best name I've heard for the idea. Since I don't yet see a flagless builds issue, perhaps this should become it, since that's already this issue's root goal? I'm also totally happy to close this in favor of a higher-level flagless builds issue you control, if that's better or easier for you.

@cpsauer
Copy link
Contributor Author

cpsauer commented Jun 23, 2022

To take a quick shot at some of the other items:

At the risk of putting words in @keith's mouth, I think a natural mental model for users is that they're configuring the target node itself, and maybe that's what's motivating the incoming edge terminology. To explain more, I expect the first thought most people will have when they want flagless builds will be: "I want this target correctly configured whenever I build it" (which feels like adding an incoming transition) rather than "I'd like to add a new target node and outgoing edge that does configuration" (outgoing transition). Maybe @keith meant something different. But I know I thought this way originally, and thought that was worth saying.

@fmeum is way ahead of me on implementing the easily-configure-targets interface with his rules_meta, and we folded our implementation and joined up, using his. It's been great for our limited needs, and since we're now on top of his interface, I should definitely just back what he thinks on which new APIs would make a more robust implementation possible, though I do independently agree with what he's describing.

@cpsauer
Copy link
Contributor Author

cpsauer commented Jun 23, 2022

That said, part of me does wonder if configuring targets is fundamental enough that it should (also?) be part of bazel directly at some point. To copy in a condensed version of #14673 (comment) (the discussion in which is so related that I don't want the cross-ref to get lost)

I can imagine a great experience for this, but I think it'd benefit from new primitives. What if all rules had an optional "configurations" attribute (like name or visibility), that took a list of transitions and flag dictionaries? You could easily configure buildable targets right where they were declared. That is, you could easily express in a BUILD file "this target needs this configuration."

To sum: Adding transitions to new rules is easy because Bazel supports it directly. I think it's also desirable to be able to easily configure targets where they're declared--as well as easily adding transitions to existing rules, as came up in previous comments here. But probably some new Bazel support would be needed to make that experience great.

@gregestren
Copy link
Contributor

Lots to digest here.

At the risk of being redundant, the only thing stopping flagless builds as a core concept is concerns about performance implications and the ability to measure/understand those implications. For power users this is probably already manageable. I'm more concerned with "regular" folks just wanting to build their project who don't understand the possibly deep implications of attaching a simple {flag: val} to a target.

This is even more concerning for multi-project, multi-org builds: a library maintainer adds a transition that doubles the library's build time. Annoying but manageable. Some other project depends on that and other libraries in multiple places and their build is 20x slower. And they didn't even change their build structure! They just upgraded the library to a new version. And they can't easily fix it without awkward patches or downgrading.

In short, I think we need more robust analysis frameworks to help everyone manage these scaling concerns.

One mitigation is to restrict transitions to rule designers, not BUILD writers. Which is kind of what we have today with transition support only for Starlark rules. Not perfect, as this and other issues clearly identify. But it does keep more of the responsibility on the power user side.

I'm happy to brainstorm this as long as you want. The user experience argument for flagless builds is, I think clear. It's got great benefits. We core devs are constantly trying to nudge the needle closer to that paradigm being real. I'm just not sure how to do so quickly given the above. My intuition is incrementally evolving the APIs could ease the problems you and others already have while not blowing open the floodgates faster than we can manage. Maybe we can have some API that forces the consequences of transitions to be limited to the org/project owner who added them? But I'm open to debate on all this.

I talked with @brandjon today. We agreed with your opinion on solving the fundamental problems. Our discussion led to the idea that there's, perhaps, an even more general problem: rule composition. That includes transitions but isn't limited to them. The doc fmeum@ linked a few comments ago explores that theme. At its most general maybe the problem is: "how do I as a rule writer structure my logic for complicated rules which necessarily mix different structural units, packages, project ownership, inheritance, common base functionality, etc."?

I imagine the most general solutions are long-term. I'd love to see you all get unblocked, practically, in the interim.

Addendum to an already super-long comment:

Maybe understated above, but analysis/measurement tools play an important role here. They can help us, with confidence, decide what's safe and what's not, and focus efforts more precisely as a result.

The core devs are very much giving that energy now. I'd like us to continue giving it energy kind of indefinitely. We're trying to frame the basic story now: how does lack of measurement restrict us, what kinds of information can help us make better decisions and why, what projects could stem from that? Brainstorming input is beyond welcome.

Examples:

@gregestren
Copy link
Contributor

As for the issue title, I don't know. "Flagless builds" could work, but how actionable is that scope for a bug? Is unblocking you and all here a choice between a full on flagless API vs. nothing?

@cpsauer
Copy link
Contributor Author

cpsauer commented Jun 24, 2022

Thanks for being so consistently spectacularly great and thoughtful, Greg.

Right there with you on basically all fronts. Yay!

Syncing up on a couple remaining things:

  1. Definitely agree that we shouldn't make this full on flagless build support or nothing. Regardless of the path to flagless builds, it seems like we're all on board all that the API fmeum linked is broadly valuable for composability. And of course, it's also concretely useful here in partially enabling @fmeum's great efforts in the interim if the bazel solution can't come quickly/safely. (Noting also that he'd want/need some things around executable targets passing through, too.)
  2. To take you up on your openness, I'd love to quickly express my feeling that it might already be safe enough to open up the floodgates--with an appropriate note in the docs to "only use configuration on internal (non-root) targets you depend on if you know what you're doing." I'm sure you've thought about this much more than I have, but it did feel pretty immediately clear to this user how to use transitions without running into problems. Additionally, it feels like most problem cases arise from configuring internal nodes (which you can warn against, and thus confine to consenting, responsible users), and that regardless, in many of the problem cases, I'd imagine users would get very quickly alerted to the problem by the linker (or equivalent name collision). Basically, you're sure it's not worth just going for it, and safe enough for a goal as exciting as being able to configure targets?
    [Perhaps a side benefit is that any emerging user issues would help inform those measurement tools.]

@gregestren
Copy link
Contributor

Full disclosure: I haven't fully read the Coding guidelines for Starlark rules doc yet. I've just skimmed and haven't fully internalized.

Re: 2: I love your enthusiasm to bravely move forward on new and exciting APIs (all of you!). We core devs tend to be especially cautious. There's a long history of seemingly straightforward features creating chronic problems as they expand into multiple projects with multiple owners. We don't want to block experimentation and progress: quite the opposite. But we do want to be really careful in how we open up and try to at least foresee the challenges if not resolve them.

I mentioned we're already exploring this space. Both @katre and @aiuto are doing research to explore this. We should link this discussion into their plans. On a practical note, @aiuto is off the grid for two weeks, I'll soon be too, and @katre may have some summer plans. Let's follow up with them later in July.

While we're brainstorming, one mitigation for my concerns is limiting support to *_binary targets. They're less likely to build across multiple projects. We actually tried something like this at Google a while back on android_binary which fell out of favor due to performance implications. But there's precedent we could discuss.

Or we could do some weird allowlisting thing where BUILD targets could set transitions but only from an allowlist controlled by the org's build manager or rule designer.

@cpsauer
Copy link
Contributor Author

cpsauer commented Jun 28, 2022

Excited indeed! And really appreciative of you all + pumped where this is headed.

Definitely hear you on the careful approach--so I should probably get out of the way and let @fmeum discuss what things would maximally unblock him.

[Some last notes, though, since I can't help myself: I think it'll be really useful also to have this on other nodes that are actually libraries for distribution. I'd still advocate power-user-only warnings over hard limits! And I'm way curious what happened with android_binary...because one of the places I use transitions is with my custom, distributable, maven-uploadable AAR rule. Turns out to be critical to make things work with platforms and selects.]

Lmk how or if you'd still like me to rename! I'd still propose something like "Enable flagless builds" and consider unblocking @fmeum and "enabling identity and configuration rules" a waypoint, but we could also have that super-goal be a separate issue? Also, should I be asking for a recategorization from P4, if we've found ways to do this that aren't non-goals for you guys?

@gregestren
Copy link
Contributor

gregestren commented Jul 25, 2022

It's late July. I've added this to my next sync with @katre and @aiuto and other configurability.

My current takeaways are:

  • Exposing a target's providers is sort of a hack solution. It lets people write wrapper targets that set transitions over other targets. But often what people really want is just a single target that sets its configuration.
  • But letting that run free creates performance and memory consequences. One concrete followup we can offer is tooling you can throw at a build that analyzes precisely what differences to expect. That can build confidence in where it is and isn't safe to flip this bit.
  • It's possible we can still get value from hackish approaches for certain practical use cases, if anyone wanted to sketch out their precise scope

@gregestren
Copy link
Contributor

We did chat about this. It's really just a timing and commitment issue on how to advance a design everyone is comfortable with. @katre is working on a design within that realm that is probably worth following - exact timeframe I'm not sure.

@gregestren gregestren changed the title Expose List of Providers From Rule in Starlark Let targets set their flags / platforms without command-line flags Aug 2, 2022
@gregestren
Copy link
Contributor

Just changed the title - not necessarily the best wording but I believe more in line with the goal. Support any further adjustments!

@cpsauer
Copy link
Contributor Author

cpsauer commented Aug 2, 2022

👍🏻 Thanks for doing and discussing, @gregestren!

@katre
Copy link
Member

katre commented Aug 3, 2022

Proposal review thread: https://groups.google.com/g/bazel-dev/c/QK7CI__ReDM

@cpsauer
Copy link
Contributor Author

cpsauer commented Aug 4, 2022

Thanks so much for writing and posting!

If useful feedback: This looks like a good step, and one we'd use, but this iteration wouldn't (yet) unblock most of the use cases that originally motivated this issue. (But maybe that's expected, and okay, especially if the main aim is something else!) That's for two reasons.

(1) We've been finding that the most valuable flags to bake into targets aren't the platform, but instead the other flags required to properly build a specific target per platform.
To explain why:

  • For us, cross-compiling rules (like android_binary) mostly auto-handle our cross-compiling platform setup needs, and target_compatible_with is there to help catch platform mistakes.
  • One often has to add flags (per target or platform) to get builds to work as intended. It's these other flags that are more error-prone and harder to weave into a build graph of preexisting rules, since rules aren't already set up to handle them. These flags are also more easily forgotten and harder to rediscover than the platform. It'd be very nice to able to bake in flags more generally than the platform.

To sum: I think it's at least as valuable to be able to set other flags, not just the platform!

(2) We're (mostly) building a library, so targets we'd want this on are mostly not binary rules. (And might or might not be built directly from the command line. AARs uploaded to maven are an interesting example.) We've already talk a bunch about the risk of uninformed users creating an exponential blowup, and I suspect that's what's motivating the binary-rule focus, so I won't belabor it. But I thought I should flag that binary-only does indeed block most of our use cases.

Thanks again for all you do! It's super well written and great to read.

@cpsauer
Copy link
Contributor Author

cpsauer commented Aug 4, 2022

Writing the above got me thinking about something related--but divergent.

What if .bazelrc had a mechanism for adding configurations that auto-applied to particular platforms? This would simplify a chunk of these remaining transition use cases (though certainly not all). It'd also dodge the exponential blowup case you guys are wary of.

Like, for example, what if one could write the following in .bazelrc:

build:@platforms//os:ios --cxxopt=-fno-aligned-allocation --objccopt=-fno-aligned-allocation

And have those flags auto apply, everywhere the platform matched!
(Real example: Those flags are needed to use C++17 and maintain reasonable backwards compatibility on Apple platforms, but aren't set by default.)


[I think I remember a related feature that didn't work with cross compilation, where a config would get auto applied if its name matched the host OS. If there's already something broader here that I don't know about, my apologies in advance. Update: Yep! Found it. --enable_platform_specific_config, but it is indeed specific to the host.]

@pcjanzen
Copy link
Contributor

pcjanzen commented Aug 4, 2022

isn't this provided by platform_mappings?

@cpsauer
Copy link
Contributor Author

cpsauer commented Aug 4, 2022

Derp. Yes. I totally should have cross-ref'd that. My bad. Good point.

Maybe amend the above to:
Perhaps, (in addition to the above) a similar platforms -> flags feature to the one temporarily in platform_mappings should get a permanent home in .bazelrc?

@gregestren gregestren added team-Configurability Issues for Configurability team next_month Issues that we will review next month. This is currently mainly used by Configurability team and removed team-Build-Language labels Nov 4, 2022
@cpsauer
Copy link
Contributor Author

cpsauer commented Nov 22, 2022

To add another potentially useful xref: I found myself linking to this while answering stack overflow questions around --enable_platform_specific_config and explaining its limitation in reflecting host (rather than target), platform.

Anyway, still seems to me that there's strong demand for a good, permanent API for both of these: Setting flags that are automatically applied to a target, and setting flags that are automatically applied to a target platform. Seeing that "next_month" discussion label makes me excited :)

Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 27, 2024
@cpsauer
Copy link
Contributor Author

cpsauer commented Jan 27, 2024

I think there's definitely lots here that's not stale, particularly around the goal of flawless builds!

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Jan 28, 2024
@gregestren
Copy link
Contributor

This issue isn't well-gardened, apologies. Going to update tags, but among other things:

  • in-progress platform-based flags automatically sets arbitrary flags you can associate with a platform
  • we're nudging on a roadmap oriented around more standardized configurations and auto-setting them for targets

I realize this issue has been around for a while. Will update the roadmap story as soon as I can. It's just a matter of getting enough support behind the ideas.

@gregestren gregestren added P2 We'll consider working on this in future. (Assignee optional) and removed P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) next_month Issues that we will review next month. This is currently mainly used by Configurability team labels Jan 29, 2024
@cpsauer
Copy link
Contributor Author

cpsauer commented Jan 30, 2024

Thanks for all your great work, Greg.
(If there are issues we should be following for either, please do lmk.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability Issues for Configurability team type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants