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 rustc env to rust toolchain #822

Closed

Conversation

nlopezgi
Copy link

  • Add rustc_env to RustToolchainInfo
  • This enables overriding at the toolchain level env vars such as RUSTUP_TOOLCHAIN and RUSTUP_HOME which otherwise would need to be set on each rust_* target - https://rust-lang.github.io/rustup/environment-variables.html
  • Setting RUSTUP_TOOLCHAIN and RUSTUP_HOME allows for remote execution of rust_* rules where the rust toolchain is preinstalled in the remote execution environment (i.e., inside a container defined in a platform configured for remote execution).
  • Setting these env vars at the rust target level is not ideal as they are only needed when using an RE toolchain, and not generally for other use cases (i.e., when using a local execution toolchain).
  • It would be ideal for the same targets to be either compiled locally or remotely based solely on the selection of the --target-platforms via the command line. If these are defined on the rust_* targets they will need select statements on each target.
    Example rust_* target with select statement (not ideal):
rust_library(
    name = "library",
    # We set these env vars to configure the RE Rust toolchain. Unfortunately the prelude does not provide a way to set these at the rust_toolchain level. We override the HOME location so rustp creates its `.rustp` directory in this location.
    env = select({
        "//platforms:engflow": {
            "RUSTUP_TOOLCHAIN":"/usr/local/rustup/toolchains/1.83.0-x86_64-unknown-linux-gnu",
            "RUSTUP_HOME": "buck-out/.rustup",
        },
        "DEFAULT": {},
    }),
    srcs = glob(
        ["src/**/*.rs"],
    ),
    exec_compatible_with = select({
        "//platforms:engflow": ["//platforms:remote_platform"],
        "DEFAULT": [],
    }),
)

Example rust toolchain with env vars (once rustc_env is added with this PR):

remote_rust_toolchain(
    name = "rust",
    default_edition = "2021",
    visibility = ["PUBLIC"],
    rustc_env = {
        "RUSTUP_TOOLCHAIN":"/usr/local/rustup/toolchains/1.83.0-x86_64-unknown-linux-gnu",
        "RUSTUP_HOME": "buck-out/.rustup",
    },
)

Full sample of rust with RE staged in EngFlow/example#361 (currently without use of rustc_env attr).

@facebook-github-bot
Copy link
Contributor

Hi @nlopezgi!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 17, 2024
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@nlopezgi
Copy link
Author

I was suggested in the buck2 discord that:

I don't understand why a build action needs to care about a package manager (rustup)'s environment variables 🤔 
Only rustc binary should be used here (and maybe the standard library) no?
It sounds like your Rust toolchain setup is not a standard one?

https://discord.com/channels/1100487223746510850/1100487225889796118/1318896818955878474

I did further investigation to verify if there is a way to configure the rust toolchain in the container in a way that it is not necessary to set these env vars. These are my findings:

This is the error I get when RUSTUP_TOOLCHAIN is not defined:

error: rustup could not choose a version of rustc to run, because one wasn't specified explicitly, and no default is configured.

help: run 'rustup default stable' to download the latest stable release of Rust and set it as your default toolchain.

This is the error that I get when I dont define RUSTUP_HOME:

error: could not create home directory: '/.rustup': Permission denied (os error 13)

Note I have tested this using the AWS provided rust image as well as the docker hub one https://hub.docker.com/_/rust and these errors occur in both cases.

I even went further to see if it was possible to create a custom Docker image where the rustup toolchain was configured before hand.

I tried to create a custom container with the following Dockerfile:

FROM rust:1.83.0-bullseye

RUN rustup toolchain link local /usr/local/rustup/toolchains/1.83.0-x86_64-unknown-linux-gnu
RUN rustup default local

I then start up the container and when I run rustup default I get the following:

error: no default toolchain configured

However, if I start the rust vanilla bookworm container and run:

root@xxx:/# rustup toolchain link local /usr/local/rustup/toolchains/1.83.0-x86_64-unknown-linux-gnu
root@xxx:/# rustup default local
info: default toolchain set to 'local'
root@xxx:/# rustup default
local (default)

It seems for some reason the state of the default toolchain (set via RUN rustup default local ) is not saved in a way that I can create a docker image with it set and then start a container and find the rustup toolchain is properly defined.

I also tried to create a docker image with the env vars set via the following Dockerfile:

FROM rust:1.83.0-bullseye

ENV RUSTUP_HOME=.rustup
ENV RUSTUP_TOOLCHAIN=/usr/local/rustup/toolchains/1.83.0-x86_64-unknown-linux-gnu

And then tried to use that for remote execution, but even in that case I got the error:

error: rustup could not choose a version of rustc to run, because one wasn't specified explicitly, and no default is configured.

help: run 'rustup default stable' to download the latest stable release of Rust and set it as your default toolchain.

My impression is that it is because the default env in the container is not used when the rustc command gets generated, the process_env is used instead - https://github.com/facebook/buck2/blob/main/prelude/rust/build.bzl#L1355.

After this follow up investigation I am pretty confident there is no way to run rust_* targets in remote execution unless the env vars are passed to the rustc action. The option of adding the rustc_env attr to the rust toolchain seems like the cleanest alternative to avoid having these env vars be spread through the rust_* targets.

@nlopezgi
Copy link
Author

Thanks for your help. We found a way to avoid needing these env vars by directly passing the paths to the compiler and other rust toolchain tools. Closing.

@nlopezgi nlopezgi closed this Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants