Enable extra features for popular crates

[PiotrSikora]

Some of the crates use build.rs to detect and enable extra features based on:

• rust version, e.g. libc crate: https://github.com/rust-lang/libc/blob/master/build.rs,
• target platform, e.g. log crate: https://github.com/rust-lang/log/blob/master/build.rs (see: Log 0.4.8 and 0.3.9 generation issue --cfg=atomic_cas #110).

which forces every user of cargo-raze to independently discover and manually enable those features, which more often than not results in missed features and/or suboptimal builds.

Right now, my Cargo.toml includes:

[package.metadata.raze.crates.libc.'0.2.80']
additional_flags = [
  "--cfg=libc_priv_mod_use",
  "--cfg=libc_union",
  "--cfg=libc_const_size_of",
  "--cfg=libc_align",
  "--cfg=libc_core_cvoid",
  "--cfg=libc_packedN",
  "--cfg=libc_cfg_target_vendor",
]

[package.metadata.raze.crates.log.'0.4.11']
additional_flags = [
  "--cfg=atomic_cas",
]

even though that crates doesn't directly depend on libc, and maintaining those rules for all transitive dependencies is definitely prone to errors.

Unfortunately, those features are detected by running code, and not declared, so I'm not sure if we can automatically extract those features, but perhaps we could include rules for some of the popular crates when features are gated only on rust version and target platform?

cc @acmcarther @UebelAndre

[dae]

Have you tried default_gen_buildrs = true? At least for my use case, that allowed me to get rid of most of manual flag specifications I was needing before.

[UebelAndre]

Yeah, I've found default_gen_buildrs = true to solve a lot of these problems but I haven't investigated what the cost is or specifically why true is not the default.

[PiotrSikora]

@dae Yes, it looks that this undocumented feature is indeed what I was looking for. Thanks a lot!

We can close this, unless we want to discuss enabling default_gen_buildrs by default and/or documenting this feature?

[dfreese]

In general, build.rs files are harmful to hermetic builds, and defaulting to true leaves a giant whole for arbitrary code execution, so leaving that as false by default is the safer option.

[UebelAndre]

Is there no way to allow the build scripts to run in a sandboxed environment such that it's no longer harmful? I lot of build.rs files I've seen check for things that should be available from a rust_toolchain

[PiotrSikora]

@dfreese which brings back my original though of maybe enabling this for some hand-selected popular/trusted crates? e.g. both libc and log crates are under github.com/rust-lang org, so they should be pretty safe.

[dae]

If cargo-raze were to do that, it would then need to keep the provided flags up to date when the upstream projects changed, which may be a bit of a maintenance burden.

[PiotrSikora]

Sorry if that wasn't clear, but in my previous comment I was talking about automatically enabling gen_buildrs for select crates, which would save users from specifying:

[package.metadata.raze.crates.libc.'=0.2.80']
gen_buildrs = true

[package.metadata.raze.crates.log.'=0.4.11']
gen_buildrs = true

in their Cargo.toml, which should be less demanding than maintaining the flags.

[UebelAndre]

I feel like the following:

    # Start with the default shell env, which contains any --action_env
    # settings passed in on the command line.
    env = dict(ctx.configuration.default_shell_env)

is the root of most of the problems with build.rs in Bazel. Is this not something that can be opt-in to make running build.rs more hermetic?

Note: This is more of a question about rules_Rust than cargo-raze. One day I think the two projects should merge but that's a conversation for another issue 😄

[dae]

That was only added a month ago:
bazelbuild/rules_rust#447

If it's introduced problems for you, perhaps you could continue the conversion over there?

[UebelAndre]

That was only added a month ago:
bazelbuild/rules_rust#447

If it's introduced problems for you, perhaps you could continue the conversion over there?

I made bazelbuild/rules_rust#490

To be clear. I'm not asserting that's an issue. I want to better understand the claim from @dfreese that build.rs is non-hermetic. I would agree this is true in general when using Cargo but there may be a way to make it so within Bazel and I feel that's worth a followup conversation on that issue.

edit: wordsmithing

[PiotrSikora]

This was more-or-less solved by #325.