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

Add uefi os constraint #97

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Add uefi os constraint #97

merged 1 commit into from
Jul 25, 2024

Conversation

amari
Copy link
Contributor

@amari amari commented Jul 13, 2024

A duplicate of #76, since that PR seems to be abandoned after almost a year of no updates from the original author.

This PR creates a uefi OS constraint. UEFI is a firmware environment with clear parallels to a conventional OS:

Linux Windows UEFI
Binary Format EFI PE/COFF PE/COFF
Entrypoint main main efi_main
The address of a "symbol" dlsym GetProcAddress SystemTable->BootServices->LocateProtocol

There are compilers that treat UEFI as an OS:

  • Rust's *-unknown-uefi targets.
  • LLVM treats UEFI as a Windows subsystem (subsystem:efi_application.)

This is also blocking at least one project downstream. (bazelbuild/rules_rust#2142)

This closes #98

@amari
Copy link
Contributor Author

amari commented Jul 23, 2024

Please take a look at this. Thanks.

@aiuto
@katre
@meteorcloudy
@Wyverald

@meteorcloudy meteorcloudy requested a review from katre July 24, 2024 09:35
@katre
Copy link
Member

katre commented Jul 24, 2024

The typical standard we use for PRs like this is "how many projects need the same OS constraint?" (the alternate solution is "projects define their own custom OS constraint").

What are the downsides? Are there cases where projects are trying to build code with multiple language rules for the same uefi platform, and this hinders interoperability?

@amari
Copy link
Contributor Author

amari commented Jul 24, 2024

I am currently working on one such project (a type 1 hypervisor).

We defined //platforms/os:uefi, and the platforms x86_64-none and x86_64-uefi. We use toolchains_llvm to get/register our C++ compilers and linkers, and rules_rust and rules_cc. We can produce freestanding ELF binaries using x86_64-none without issue via cc_* and rust_* targets.

The issue arises when compiling rust code using our local //platforms/os:uefi constraint:

ERROR: Traceback (most recent call last):
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/rules_rust~/rust/extensions.bzl", line 57, column 33, in _rust_impl
                rust_register_toolchains(
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/rules_rust~/rust/repositories.bzl", line 230, column 14, in rust_register_toolchains
                maybe(
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/bazel_tools/tools/build_defs/repo/utils.bzl", line 268, column 18, in maybe
                repo_rule(name = name, **kwargs)
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/rules_rust~/rust/repositories.bzl", line 1043, column 61, in rust_repository_set
                all_toolchain_names.append(rust_toolchain_repository(
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/rules_rust~/rust/repositories.bzl", line 597, column 58, in rust_toolchain_repository
                target_compatible_with = triple_to_constraint_set(target_triple)
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/rules_rust~/rust/platform/triple_mappings.bzl", line 362, column 44, in triple_to_constraint_set
                constraint_set += system_to_constraints(triple_struct.system)
        File "/private/var/tmp/_bazel_amari/ec42e892d6acc4bd9e8359264a57a8a1/external/rules_rust~/rust/platform/triple_mappings.bzl", line 246, column 13, in system_to_constraints
                fail("System \"{}\" is not supported by rules_rust".format(system))
Error in fail: System "uefi" is not supported by rules_rust
ERROR: Analysis of target '//kernel-loader:x86_64-uefi' failed; build aborted: error evaluating module extension rust in @@rules_rust~//rust:extensions.bzl
INFO: Elapsed time: 0.221s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
FAILED: 
    Fetching module extension toolchains in @@rules_java~//java:extensions.bzl; starting
    Fetching module extension rust in @@rules_rust~//rust:extensions.bzl; starting
    Fetching repository @@toolchains_llvm~~llvm~llvm_toolchain; starting

Tracing through this, we get to this line in system_to_constraints:

return ["@platforms//os:{}".format(sys_suffix)]

It's clear that rules_rust expects the os constraint to be included upstream. There's a PR already approved by the maintainers (bazelbuild/rules_rust#2142). The alternative would be declaring a project-local os constraint. (rules_rust used to have them, but support for them was removed two years ago.)

When it comes to other projects, I believe that @monogon-dev also needs this for rules_rust and rules_go. (@lorenz from monogon-dev opened the rules_rust PR.)

@katre
Copy link
Member

katre commented Jul 25, 2024

Thanks for clarifying, I agree that this is a valid use case.

rules_rust should be capable of handling constraints that aren't in @platforms, however.

@katre katre merged commit d3b78bb into bazelbuild:main Jul 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for UEFI targets
2 participants