Propagate configurations to execution dependencies

[yxdai-nju]

I'm trying to implement Rust's bootstrap process with Buck2.

I've set up three target platforms (also serving as execution platforms) in order: :use_stage0, :use_stage1, and :use_stage2, to constrain which version of the Rust toolchain is used.

For example, when using :use_stage1 as the target platform, it uses the stage1 version of the Rust toolchain.

The problem is: if I have a build target :a with an execution dependency :b, configured like this to use the stage1 toolchain:

configured_alias(name = "a", actual = ":_a", platform = ":use_stage1")

my_target(name = "_a", exec_deps = [":b"])

Buck2 independently resolves the execution platform for :b, and will still select :use_stage0 by default, which is not the behavior I'd expect.

Is there a way to make :b use the same platform as :a?

---

This occurs when building Rust projects, for example with a project containing build.rs. If the build target is :my_crate, the dependency chain looks like:

:my_crate
-[dep]->
:my_crate-build-script-run
-[exec_dep]->
:my_crate-build-script-build

Solving this is critical for bootstrapping, because even if I specify using the stageX toolchain to build :my_crate, when building :my_crate-build-script-build, it might still select a toolchain other than stageX.

[dtolnay]

Buck2 independently resolves the execution platform for :b, and will still select :use_stage0 by default, which is not the behavior I'd expect.

In case you haven't come across this already, execution platform resolution is described briefly here: https://buck2.build/docs/rule_authors/configurations/#execution-platform-resolution. There is an ordered list of execution platforms, and each target independently uses the first compatible one.

Either on _a or on some other target that is a "toolchain_dep" of _a (in Meta's case we have this on the target that provides the RustToolchainInfo), you need to add:

exec_compatible_with = select({
    "//constraints:stage0": ["//constraints:stage0"],
    "//constraints:stage1": ["//constraints:stage1"],
    "//constraints:stage2": ["//constraints:stage2"],
})

where the constraints are defined as:

# //constraints

constraint_setting(
    name = "bootstrap_stage",
)

constraint_value(
    name = "stage0",
    constraint_setting = ":bootstrap_stage",
)
...

and the platforms as:

# //platforms

platform(
    name = "stage0",
    constraint_values = ["//constraints:stage0"],
    ...
)
...

This will make it so that when building _a for target platform stage1 (for example), execution platform resolution will require an execution platform that satisfies the stage1 constraint, i.e. only the stage1 execution platform.

[yxdai-nju]

Thanks! It worked like a charm and solved the build script issues, just by adding exec_compatible_with setting 😄

But I'm encountering a new similar issue when building a crate with procedural macros:

The platforms changes from ...:rust_beta (by my configured_alias configuration) to ...:rust_bootstrap_stage2 (the first compatible one) when handling the procedural macro.

[JakobDegen]

But I'm encountering a new similar issue when building a crate with procedural macros

Yeah, I see why it's a bit surprising that you would hit this, but the change on rust_proc_macro_alias is not an appropriate solution. Instead, there are a couple other directions for solutions:

First, note that the exec_dep from rust_proc_macro_alias to the underlying proc macro is entirely a convenience for the purpose of getting "build a proc macro on the command line" working:

buck2/prelude/rust/proc_macro_alias.bzl

Lines 14 to 31 in 469aa6d

	# FIXME(JakobDegen): The rules themselves do not need any of the exec configured providers.
	# They only ever access the `RustProcMacroMarker` provider and do everything else through
	# plugins. However, we cannot return just that provider, since it results in
	# `buck2 build :proc_macro` building nothing. We would get the right behavior out of the
	# command line build invocation by taking a regular target dep on the aliased target and
	# returning those targets. However, that also does not work because it would result the target
	# configuration for a Rust library needing to be compatible with the target configuration for
	# its proc macro deps (a Linux-only proc macro should still be usable in a Rust build targeting
	# Mac).
	#
	# Instead, we take an exec dep on the aliased target and return those providers. This is a
	# compromise solution. The upside is that it avoids introducing any unnecessary compatibility
	# constraints. The downside is that it's still slightly wrong - because of the exec transition,
	# the returned artifacts won't correctly obey opt level/sanitizer/platform constraints in the
	# target configuration. However, this matches the behavior in fbsource today.
	#
	# Once config modifiers are implemented and roll out, it should not be too hard to get the fully
	# correct behavior

That exec dep is not actually used for anything when the proc macro is used in the course of a normal build.

In a situation where you are doing a Rust bootstrap build, the downsides of that convenience are larger than usual; the issue described in the FIXME is more likely to be important because of staging, and furthermore this might lead to the introduction of a large number of additional dependencies, which hurts analysis times. As a result, I think one appropriate option to take here would be to add a buckconfig that allows a project to globally disable that convenience, having RustProcMacroAlias return

Second, with modifiers having landed, I think we should be able to now address that fixme, which should also solve your problem if done correctly. I can try and take that, but no promises.

Finally, though: The end effect of what's going on here is that you have a (functionally) non-Rust target taking an exec dep on a Rust library and just saying "build this for me in an exec platform." Phrased like that it doesn't sound so common, but it's structurally quite similar to the much more realistic case of there being a genrule somewhere with an exec_dep on a rust_binary. While adding an additional exec_compatible_with here might work, expecting all such genrule authors to add one as well is obviously inappropriate. More specifically, the configuration error you're encountering there indicates that you've set up your exec platforms incorrectly. My guess is that your "top" exec platform is not an exec platform in which Rust builds "just work," and that likely needs fixing

[yxdai-nju]

First, note that the exec_dep from rust_proc_macro_alias to the underlying proc macro is entirely a convenience for the purpose of getting "build a proc macro on the command line" working:
[buck2/prelude/rust/proc_macro_alias.bzl]
That exec dep is not actually used for anything when the proc macro is used in the course of a normal build.

I see, so the plugin_dep is the one actually used during building, and I shouldn't have assumed an exec_dep in my mental model.

I tried removing that exec_dep, but ended up discovering that other parts of prelude are already depending on its existence (if I was doing it correctly), which I'll describe in a later comment.

Second, with modifiers having landed, I think we should be able to now address that fixme, which should also solve your problem if done correctly. I can try and take that, but no promises.

That'd be really helpful! I recently also found some documentation about modifiers in PRs like #848, I find it to be a promising new feature. I'm also willing to learn about it and help test it out!

it's structurally quite similar to the much more realistic case of there being a genrule somewhere with an exec_dep on a rust_binary. While adding an additional exec_compatible_with here might work, expecting all such genrule authors to add one as well is obviously inappropriate.

I see, expecting all rules to propagate exec_compatible_with settings to their execution dependencies would be unreasonable. But since rust_proc_macro_alias is specifically used in Rust builds and the exec_dep currently exists, I believe my fix is appropriate, as an interim solution.

More specifically, the configuration error you're encountering there indicates that you've set up your exec platforms incorrectly. My guess is that your "top" exec platform is not an exec platform in which Rust builds "just work," and that likely needs fixing

You're absolutely right! Actually I configured an intentionally non-working platform as the top one to make execution platform resolution problems explicit errors rather than silently using the wrong toolchain. I apologize. I should have made this clear earlier 😅

[yxdai-nju]

I created a demo repo for this issue, simplified from my actual bootstrapping setup. The platforms are ordered [stage2, stage1, stage0], where only stage0 is working.

With this repo, I would ask Buck2 to build a Rust binary for stage0. This binary target depends on a library with procedural macros, it triggers execution platform resolution and errors if resolving to a platform other than stage0. This helped me verifying that fix.

I tried removing that exec_dep, but ended up discovering that other parts of prelude are already depending on its existence (if I was doing it correctly), which I'll describe in a later comment.

When I now try to remove the exec_dep:

prelude/rust/proc_macro_alias.bzl

-   providers = list(ctx.attrs.actual_exec.providers)
+   providers = [DefaultInfo()]
    providers.append(RustProcMacroMarker(label = ctx.attrs.actual_plugin))
    return providers

rust_proc_macro_alias = rule(
    impl = _impl,
    attrs = {
-       "actual_exec": attrs.exec_dep(),
        "actual_plugin": attrs.plugin_dep(kind = RustProcMacroPlugin),
-       "_rust_toolchain": toolchains_common.rust(),
    },
)

prelude/rust/rust_library.bzl

def rust_library_macro_wrapper(rust_library: typing.Callable) -> typing.Callable:
    def wrapper(**kwargs):
        if kwargs.get("proc_macro") == True:
            name = kwargs["name"]
            if kwargs.get("crate", None) == None and kwargs.get("crate_dynamic", None) == None:
                kwargs["crate"] = name.replace("-", "_")

            rust_proc_macro_alias(
                name = name,
-               actual_exec = ":_" + name,
                actual_plugin = ":_" + name,
                default_target_platform = kwargs.get("default_target_platform", None),
                visibility = kwargs.pop("visibility", []),
            )
            kwargs["name"] = "_" + name

        rust_library(**kwargs)

I encountered this error:

yxdai-nju@rust:~/buck2-proc-macro-issue-demo$ buck2 build //:thiserror_demo-stage0
Build ID: ee3fa1c8-bc20-4019-9e21-d08e7b06dc50
Command: build.                                                                                                                                                                      
Time elapsed: 0.0s
BUILD FAILED
Error running analysis for `root//:thiserror_demo-stage0 (bootstrap//platforms:default#e2afd151e1796551)`

Caused by:
    0: Error running analysis for `root//:thiserror_demo (bootstrap//platforms:rust_bootstrap_stage0#b31153f19b1a9c3f)`
    1: Error running analysis for `root//third-party:thiserror (bootstrap//platforms:rust_bootstrap_stage0#b31153f19b1a9c3f)`
    2: Error running analysis for `root//third-party:thiserror-2.0.12 (bootstrap//platforms:rust_bootstrap_stage0#b31153f19b1a9c3f)`
    3: Traceback (most recent call last):
         File <builtin>, in <module>
         * prelude/rust/rust_library.bzl:415, in rust_library_impl
             providers.append(rust_analyzer_provider(
         * prelude/rust/rust-analyzer/provider.bzl:80, in rust_analyzer_provider
             transitive_target_set = _compute_transitive_target_set(ctx, rust_deps),
       error: Object of type `NoneType` has no attribute `transitive_target_set`
         --> prelude/rust/rust-analyzer/provider.bzl:58:23
          |
       58 |         target_sets = dep.get(RustAnalyzerInfo).transitive_target_set
          |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          |

It looks like that some rule under 'prelude/rust/rust-analyzer' (added after the exec_dep was introduced) is expecting a RustAnalyzerInfo provider through the exec_dep. I'm still working to fully understand this part and will follow up once I better understand what's happening.