Allow disabling the FFI integration with a feature

[bjorn3]

cargo check -Zunstable-options --compile-time-deps is taking almost 2 minutes because it is building capstone and libffi. The entire rest of the rust workspace takes only 1m34s to build with these options. When rust-lang/rust#143785 lands, this means that half of the time it takes rust-analyzer before it can do any analysis with a fresh build dir would be taken up by miri.

[RalfJung]

Yeah I noticed capstone taking forever as well...
Cc @nia-e

[nia-e]

Doesn't CI build with --all-features? Then if we add a feature to disable it, we'll never be able to test it in CI...

[RalfJung]

Features can't disable things, they must be positive. Cargo doesn't work well otherwise.

Given that libffi is also not super fast to build, we could indeed have a feature for the entire native-lib support, and put all these dependencies behind it. It's fine for work in the Miri repo, one can use --feature and our CI already sets --all-features.

But in the rustc repo -- we'd want the dist builder and CI to still enable some but not all features. Also when I work on Miri in the rustc repo I need a way to test native-lib mode. I don't think that is currently possible, which means I don't think we currently can move this behind a feature gate. Cc @Kobzol

Of course, this also always risks people breaking the build and then noticing that only when CI runs -- and then they'll not know how to reproduce the failure locally. I don't think anything else in the rustc repo has feature that are "off by default when building locally but turned on on CI"?

half of the time it takes rust-analyzer before it can do any analysis with a fresh build dir would be taken up by miri.

Clippy AFAIK still takes longer to build than Miri, so the other question here is -- why do you even build Miri in your RA setup?

[RalfJung]

Fundamentally the real problem is that we don't even need capstone and libffi to build their C code for a check build, but cargo has no support for skipping C dependency builds in check builds (rust-lang/cargo#4001). So all we can do is hack around this in various ways...

[bjorn3]

The tool_extended!() macro has an add_features field with which you can enable arbitrary features when a tool is built. Also conditionally on doing a check mode using builder.kind == Kind::Check.

[RalfJung]

Seems like bootstrap now supports user-configurable features for a particular tool: rust-lang/rust#143733.

@rust-lang/infra what is the best way to have a different set of features enabled in Miri on CI, and in the default configuration for contributors? I could imagine that CI (and dist) use the default features, but contributors use --no-default-features, or maybe contributors use default features and CI/dist have a list of extra features they enable.

[marcoieni]

I guess for contributors it's easier to use the default instead of specifying additional flags, right?

[RalfJung]

"Default" is an overloaded term. :)

• I was referring to cargo's notion of default features.
• When contributors just do the normal build without special config, that's the "default" they see, but that may involve passing --no-default-features to cargo, so it doesn't have to be the same as the default cargo features.

[marcoieni]

• I was referring to cargo's notion of default features.

me too 👍

• When contributors just do the normal build without special config, that's the "default" they see, but that may involve passing --no-default-features to cargo, so it doesn't have to be the same as the default cargo features.

I'm not sure I understood 100%.
can the --no-default-features flag be passed implicitly? 🤔
Anyway, for infra I think both solutions should work

[RalfJung]

can the --no-default-features flag be passed implicitly?

Bootstrap invokes cargo, so yeah it can pass anything it wants implicitly.^^

But what is the best way for bootstrap to distinguish "default contributor setup" from "CI"? Should we literally use is_ci or is there some place where "the bootstrap config for CI" is stored that we can/should edit?

[marcoieni]

Oh, ok, personally I don't know. Maybe someone from @rust-lang/bootstrap knows better 🤔

[Kobzol]

can the --no-default-features flag be passed implicitly?

Bootstrap invokes cargo, so yeah it can pass anything it wants implicitly.^^

But what is the best way for bootstrap to distinguish "default contributor setup" from "CI"? Should we literally use is_ci or is there some place where "the bootstrap config for CI" is stored that we can/should edit?

If it is possible to configure what you want with config.toml, then we can make the default in bootstrap be the behavior we want for contributors, and then change that default in bootstrap/defaults/bootstrap.dist.toml, which is used as a baseline config for all of CI. I would prefer something like this vs an explicit is_ci check in bootstrap, because these checks obviously make it hard(er) to reproduce CI errors (and behavior) locally.

I wonder if we should instead special case this to --compile-time-deps instead though? We could just detect such a check build in bootstrap, and disable the FFI integration for it specifically. Then contributors will get faster RA init, and only people working on miri (which is probably not that many ppl out of the whole contributor pool) will have to build the FFI if they build miri locally.

In any case, miri would have to provide a way of turning off the capstone build, probably by hiding it behind a (default) feature.