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

incompatible_make_rust_providers_target_independent #966

Closed
scentini opened this issue Oct 12, 2021 · 2 comments · Fixed by #1074
Closed

incompatible_make_rust_providers_target_independent #966

scentini opened this issue Oct 12, 2021 · 2 comments · Fixed by #1074
Assignees

Comments

@scentini
Copy link
Collaborator

scentini commented Oct 12, 2021

Motivation

Currently CrateInfo.deps and CrateInfo.proc_macro_deps have the type depset[Target]. In rustc_compile_action the Targets are inspected when creating the target’s DepInfo.
This is inconvenient if we want to compile a Rust source file from an aspect (that depends on other Rust source files compiled by the aspect), as there is no concrete target that we could put into the CrateInfo.{deps|proc_macro_deps} fields. The aspect will instead return a custom provider that will contain a CrateInfo and DepInfo.

Incompatible changes

Instead of depset[Target], we’ll make CrateInfo.deps and CrateInfo.proc_macro_deps be of type depset[DepVariantInfo]. DepVariantInfo is a new provider that wraps the providers a crate’s dependency may have. We’ll also need an additional field in CrateInfo: CrateInfo.owner, which will contain the label that the CrateInfo was created from. This is because we need to know the label so we can memorize the alias for the crate.

The //rustc/settings:incompatible_make_rust_providers_target_indepent guards this incompatible change. Once flipped, rustc_compile_action will check that the CrateInfo.owner field is present, and also that CrateInfo.deps and CrateInfo.proc_macro_deps have depset[DepVariantInfo] type.

Migration steps

One should pass a depset[DepVariantInfo] to CrateInfo.deps and CrateInfo.proc_macro_deps, as well as the owner label:

...

toolchain = ctx.toolchains[Label("//rust:toolchain")]
make_rust_providers_target_independent = toolchain._incompatible_make_rust_providers_target_independent

deps = [DepVariantInfo(
    crate_info = dep[CrateInfo] if CrateInfo in dep else None,
    dep_info = dep[DepInfo] if DepInfo in dep else None,
    build_info = dep[BuildInfo] if BuildInfo in dep else None,
    cc_info = dep[CcInfo] if CcInfo in dep else None,
) for dep in ctx.attr.deps] if make_rust_providers_target_independent else ctx.attr.deps

proc_macro_deps = [DepVariantInfo(
    crate_info = dep[CrateInfo] if CrateInfo in dep else None,
    dep_info = dep[DepInfo] if DepInfo in dep else None,
    build_info = dep[BuildInfo] if BuildInfo in dep else None,
    cc_info = dep[CcInfo] if CcInfo in dep else None,
) for dep in ctx.attr.proc_macro_deps] if make_rust_providers_target_independent else ctx.attr.proc_macro_deps

...

crate_info = cc_common.create_crate_info(
    ...
    deps = depset(deps),
    proc_macro_deps = depset(proc_macro_deps),
    owner = ctx.label,
    ...
)

...
@UebelAndre
Copy link
Collaborator

What's the use case for compiling files in aspects? Just out of curiosity

@hlopko
Copy link
Member

hlopko commented Oct 12, 2021

A typical example is code generation from some format into multiple languages. Imagine protobuf, where the data format is defined in a language independent .proto file, and you generate language specific implementation for the format.

If the data format specification was standalone, meaning one target fully defined the format, you don't need aspect, you can just fine live with rules:

yolo_proto_library( 
  name = "foo",
  src = "foo.yolo_proto"
)
rust_yolo_proto_library( 
  name = "foo_rs",
  dep = ":foo",
)

But once your data format allows for reuse and for dependencies, you need an aspect:

proto_library(
  name = "base",
  src = "base.proto",
)
proto_library(
  name = "top",
  src = "top.proto",
  deps = [":base"],
)

rust_proto_library(
 name = "top_rs",
 # here we want to apply an aspect on the `dep` attribute, so it visits `:top` and all its deps. Benefits are:
 # * we don't have to repeat whole transitive dependency tree of proto library in the rust proto library
 # * we can actually reuse intermediate Rust artifacts of individual transitive dependencies (which we couldn't if rust_proto_library was a rule that duplicated all transitive dependencies of the proto_library)
 dep = ":top",
)

scentini added a commit that referenced this issue Oct 12, 2021
This flag guards the incompatible changes needed in order to be able to compile files in aspects. For more information see #966.
@scentini scentini self-assigned this Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants