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

Is there a reason build_scripts does not include flags? #97

Open
mstg opened this issue Apr 27, 2019 · 4 comments
Open

Is there a reason build_scripts does not include flags? #97

mstg opened this issue Apr 27, 2019 · 4 comments

Comments

@mstg
Copy link
Contributor

mstg commented Apr 27, 2019

Hey!

Currently some crates has conditional behaviour in their build scripts that can be specified with --cfg flags. Currently cargo-raze does not pass on flags to the build_script targets. Is there a reason for that?

Just wanted to check in, I can create a PR if it's acceptable to pass them to the build_script target.

@mstg
Copy link
Contributor Author

mstg commented Apr 28, 2019

I think I may have understood something fundamental about the CARGO_CFG_<cfg> envs as they may not be related to --cfg? I'm still new to Rust, so I'm not sure.

@ghost
Copy link

ghost commented May 2, 2019

Fighting the same problem at the moment. Trying to compile libpnet crate which depends on CARGO_CFG_TARGET_ENDIAN variable. I believe this one should be provided by cargo as per here. But I believe both cargo-raze and rules-rust circumvent cargo and call rustc directly.

I am also willing to help this project, but as beginner at Bazel and semi-experienced rust programmer I am not sure where to start.

EDIT:
I think #38 is already tracking the implementation of CARGO_CFG_*

@acmcarther
Copy link
Member

acmcarther commented May 3, 2019

My recollection of these cargo specifics is unfortunately foggy (and incidentally the reason I punted on replying to this once it hit my inbox, though regretably I forgot about it until this subsequent ping).

I can say that we do properly propagate feature flags, which manifest as "CARGO_FEATURE_*". I also recall that CARGO_MANIFEST_DIR support was added manually due to the large number of build scripts that require that specific env var, but it seems that change was folded into the original commit for this feature. That variable alone required what I consider to be some absurd combinations of tera template substitution and arcane BUILD features, and my idealized form of this support would take the form of adding some kind of build script runner, instead of adding more crazy stuff to the genrule.cmd field.

Even if the change just includes an incremental addition to genrule.cmd, I would be happy and grateful to review PRs that include more of these environment variables. Some may be easier to furnish than others. I believe this is the authoritative source for these env vars.

@acmcarther
Copy link
Member

For my own (future) edification while this is fresh in my mind, I want to expand a bit upon the "build script runner" thing.

Build Script Runner Thing

I think even since build script support was first implemented, the current approach of running the build script as a genrule was considered hacky. I went with this approach above something more sophisticated because I wanted to avoid forcing the bazel workspace to have a dependency upon bazel content in cargo-raze. That is to say, requiring it to be loaded as an http_archive in addition to being cargo-install'd or whatever. This raised some questions I wasn't ready to answer, like

  1. What does it mean if the BUILD files are generated with a different version of cargo-raze than the one the workspace is importing? Can we provide compatibility guarantees at all?
  2. What is our story going to be for users who want to include the cargo-raze binary in their workspace, and as an in-org policy, use that binary instead of a cargo-install'd binary?
  3. As an extension to (2), will we provide support for running cargo-raze as part of the BUILD process itself? What does that look like?

It was easiest at the time to say "no" to these concerns and simply ensure that generated BUILD files never had external dependencies beyond rules_rust. This led to the abomination that is build_script.template.

In the real world though, or at least in Google, people build sensible utility binaries (e.g. py_binary) to do this kind of thing. Such a binary for our use case could:

  1. Accept the build script binary as an arg
  2. Accept some sort of "digest" of the "static"/non-environment-dependent properties of the crate. Maybe a flag to a json file?
  3. Accept a second "digest" containing the properties for the exact execution platform and target platform (ideally in bazel's terms, not in cargo's terms). Execution platform may be required to run the binary, target platform will be required to generate CARGO_VARS.
  4. Load the two files and execute the binary with env vars translated from fields in the "digests"
  5. Yield two outputs: OUT_TAR (consisting of generated files in a file tree parallel to the workspace), and a file containing the printed output from the build script. Further note on "output from build scripts" below.

While the handling of build script output (beyond generated files) is still an open question, I think capturing this stuff now is a good first step to eventually handling this. Perhaps we could locate requested rustc-link-lib from some provided registry? In any event, it's hard to imagine such a rosy future from the dumpster fire that is build_script.template circa 2019/05.

As an aside, perhaps such a utility binary could live in bazelbuild/rules_rust itself? I would be quite happy with an outcome like that, provided the utility binary was user friendly enough to be used by humans and not just generated BUILD files.

Output From Build Scripts

For reference, Cargo has a strange relationship with build scripts. Basically, it runs them, dumps the output into target/ (iirc?), and consumes stdout from the script. The list of valid directives from a build script is available here.

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

No branches or pull requests

2 participants