Add a cargo_dep_env rule for setting build.rs environment variables

[bsilver8192]

From the docs:

A rule for generating variables for dependent cargo_build_scripts
without a build script. This is useful for using Bazel rules instead
of a build script, while also generating configuration information
for build scripts which depend on this crate.

[illicitonion]

Thanks for the contribution! I think having this kind of mechanism generally makes sense - just one question about the shape of it.

[illicitonion]

Suggested change

	fail("Only a dependency or a dep_env_file may provide build information")
	fail("Only one of a dependency or a dep_env_file may provide build information")

[illicitonion]

I'm not sure this should be a common attr - if I'm understanding correctly, this is something a library sets to propagate to its dependents? It feels like it should either only live on rust_library (I don't see a use-case for rust_binary or rust_test setting a dep_env_file), or possibly be a standalone rule:

cargo_dep_env(
    name = "set_b",
    dep_env_file = "set_b.env",
)

I'd lean towards the standalone rule, which just populates a BuildInfo provider, as this doesn't seem like it would be particularly coupled to a rust_library, but I could be easily convinced otherwise.

[UebelAndre]

This feels like it blurs the lines farther for whether or not cargo_build_script is a a part of the core rules or not. I think changes like these only make sense if there's a desire to promote cargo_build_script into //rust. @illicitonion What do you think?

Additionally, @bsilver8192, would you be willing to share a concrete use case you have for this functionality?

[bsilver8192]

My use case is pointing libz-sys at a Bazel-compiled zlib (without using its build.rs), but using the build.rs for libgit2 (which depends on libz-sys). https://github.com/frc971/971-Robot-Code/blob/8e815fbb728ddc4df9714f72beb925cb8c93159d/third_party/cargo_raze/cargo_raze.patch is my patches (also includes some unrelated changes to platforms and warning flags). I'm planning to submit that to cargo_raze if we get something merged here.

PS: Ignore the # TODO(Brian): Set buildrs_additional_environment_variables for upstreaming. in those cargo_raze patches, I noticed that just now, that's from my hacks before the approach from this PR...

More background on why I want to do this: libz-sys is pretty easy to write a BUILD file for, and is also commonly a shared dependency with C++ code which needs a cc_library to link against. libgit2 looks complicated to build (judging by the build.rs), and I don't have any C++ dependencies on it.

[bsilver8192]

This feels like it blurs the lines farther for whether or not cargo_build_script is a a part of the core rules or not. I think changes like these only make sense if there's a desire to promote cargo_build_script into //rust. @illicitonion What do you think?

Would cargo_dep_env next to cargo_build_script address that? Or is it more of a concern around adding functionality to something that's still some level of experimental (which would be nice to mention in the docs)?

[illicitonion]

Would cargo_dep_env next to cargo_build_script address that?

Yes, I think this would avoid the concern of cargo "polluting" the rest of rules_rust.

It's also worth noting that this rule wouldn't have to live in rules_rust - the BuildInfo provider is public, so you could even implement cargo_dep_env in your own repo if you wanted to - just a small rule that takes a path and populates a field on a provider. Either way, I think it's worth implementing (and we can have a discussion about where it lives :))

Or is it more of a concern around adding functionality to something that's still some level of experimental (which would be nice to mention in the docs)?

AFAIK the issue isn't so much that cargo support is experimental (I wouldn't describe it as such); the two concerns around the cargo support are:

1. Purity. cargo is its own build system with its own ideas separate from Bazel. It's theoretically possible to use rust in Bazel and completely avoid cargo's model of the world - from a purity perspective, adding "this is for cargo interop" pieces to the core rules_rust rules may make the rust rules themselves less clear, more confusing, more complicated, or just harder to understand, than separating the concerns so that //rust is effectively an independent driver of rustc and //cargo is a modeller of cargo concepts.
2. Stability / cleanliness. We are pretty open to making behaviour changes (potentially breaking ones) or performing heuristic hacks in //cargo if it improves how we're emulating cargo in what it does (e.g. we do some hacky things around interpreting and interpolating ${pwd} into some of the outputs of cargo build scripts). We try to keep these localised to //cargo because that package is explicitly designed to do as close to what cargo would do as we can manage, whereas //rust is more designed to do its own thing as well as it can.

[illicitonion]

Thanks so much!