Cargo build scripts executing rustc

[vitalyd]

I'm not sure if this issue/question is more appropriate for this repo/project or cargo raze, but I'll file it here as it'll likely touch upon rules_rust anyway.

There are Cargo projects with a custom build.rs that wants to execute rustc, e.g. to get the compiler version. For example, protobuf.

Is there a way to get the path, in a genrule, to rustc of the current (i.e. selected/executing) Rust toolchain?

Somewhat relatedly, the aforementioned build.rs also wants the CARGO_PKG_VERSION envvar. This is set in the toolchain, but the genrule does not (re)export it: https://github.com/google/cargo-raze/blob/8138a751a0960f7975a3d5b969667e0e68f53a7e/impl/src/templates/partials/build_script.template#L28-L52

I found that if I manually added export CARGO_PKG_VERSION=$$(CARGO_PKG_VERSION) to the genrule, it worked. Not sure that's the right/expected way to do that. Should raze export all these CARGO_* envvars to build rules? Sorry, this one is very likely a raze issue, but it's so closely related to the above that I thought I'd plop it here :).

[mfarrugi]

Is there a way to get the path, in a genrule, to rustc of the current (i.e. selected/executing) Rust toolchain?

I don't think there's a way to reference a toolchain in a genrule, but a workaround would be to do something like $$(location @my_rust_toolchain//:rustc.

Apparently toolchains can be passed to genrules, and then we could export make variables from starlark.. though that doesn't sound great to me.

Somewhat relatedly, the aforementioned build.rs also wants the CARGO_PKG_VERSION envvar. This is set in the toolchain, but the genrule does not (re)export it: https://github.com/google/cargo-raze/blob/8138a751a0960f7975a3d5b969667e0e68f53a7e/impl/src/templates/partials/build_script.template#L28-L52

I found that if I manually added export CARGO_PKG_VERSION=$$(CARGO_PKG_VERSION) to the genrule, it worked. Not sure that's the right/expected way to do that. Should raze export all these CARGO_* envvars to build rules? Sorry, this one is very likely a raze issue, but it's so closely related to the above that I thought I'd plop it here :).

Adding the var seems reasonable, worth mentioning that I think @damienmg was working on a nicer build-script-executor rule to replace the genrule, and then a proper rule would be able to reference the toolchain as well.

In general I think that the genrules have ended up complicated and finicky enough that writing rules would be worth it to avoid the genrule/bash semantics

[vitalyd]

@mfarrugi, thanks for your thoughts.

Finally had a bit of time to play with this today, and was able to get the RUSTC env var plumbed through to the genrule:

1. Modified toolchain.bzl to:

-    return [toolchain]
+    variables = platform_common.TemplateVariableInfo({'RUSTC': toolchain.rustc.path})
+    return [toolchain, variables]

2. Modified the generated genrule for protobuf by adding:

        + " export RUSTC=$$PWD/$(RUSTC);"

and setting the toolchain:

    toolchains = [
      "@rust_stable//:stable_impl",
    ],

But yes, this is quite fickle and awkward. Is there a way to avoid hardcoding the toolchain name?

I do think it probably makes sense to at least add the RUSTC Make variable, as above, regardless of where we go with actually using it in build scripts; seems like a decent standalone change.

Also, I was wrong about:

I found that if I manually added export CARGO_PKG_VERSION=$$(CARGO_PKG_VERSION) to the genrule, it worked.

That "works" in the sense that the envvar is set, but it's empty. Protobuf builds fine since it allows an empty version, but this isn't the value set here. In fact, I couldn't get at that value in the genrule which I think makes sense since it's only set in the protobuf_build_script rust_binary rule, and not the genrule. I think.

Is there more info on what @damienmg is working on?

[damienmg]

Hi @vitalyd,

This was what I was working on at some point: damienmg@9e272e7

I got very busy with other projects and I did not used rust for some time (except for google/starlark-rust), but I would love to see that happens.

[vitalyd]

Hi @damienmg, thanks for the pointer! Could you say a bit about what state those changes are in? i.e. what would it take, in your mind, to get them over the finish line?

[damienmg]

If I remember correctly, the code just needed a bit more testing and to be rebased. The baseline is pretty old and when I looked at it last time, there were some significant rehauling needed to rebase.

…

On Wed, Jul 10, 2019 at 6:44 PM vitalyd ***@***.***> wrote: Hi @damienmg <https://github.com/damienmg>, thanks for the pointer! Could you say a bit about what state those changes are in? i.e. what would it take, in your mind, to get them over the finish line? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#237?email_source=notifications&email_token=AA4MO7YZVYR64IO6YDIPLR3P6YGVFA5CNFSM4H5JJU32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZUBZAQ#issuecomment-510139522>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA4MO7ZZZYSZBRVXVFSSYELP6YGVFANCNFSM4H5JJU3Q> .

[ali5h]

so is there a way to have RUSTC env variable exposed (specially for build.rs)? some crates use it. https://github.com/SergioBenitez/version_check/blob/master/src/lib.rs#L98

[damienmg]

#320 added support for build.rs, and the RUSTC env is also set.

[damienmg]

In respect to the original bug, @vitalyd, now that the cargo_build_script rule is in I wonder if that makes sense to extend it to be able to inject more variable (e.g. the CARGO_PKG_VERSION in your case)?

[UebelAndre]

With the additions of #1416 and #1377 as well as the fact that cargo_build_script always sets RUSTC as an environment variable. I think there are plenty of ways for Cargo build.rs scripts to invoke rustc. I'll close this out for now but for any subscribers here, please ping the ticket and I can open it again or open a new one to track a specific issue.