Make loom optional

[faern]

I have come to realize that with:

[target.'cfg(loom)'.dependencies]
loom = { version = "0.7.2", features = ["futures"] }

loom is pulled in as a mandatory dependency and pollutes Cargo.lock. This means it pollutes the dependency tree by quite a lot. This should be a small and lightweight library, so that's not ideal. Downstream users of this library should not need to have the entire loom dependency tree in their supply chain, they will/should never use it!

This PR explores making loom a proper optional dependency instead.

[faern]

This PR was inspired by crossbeam-rs/crossbeam#666. However, I do not understand why they went with putting the loom dependency both behind a cfg and a feature. I might be missing something here, but I can't see what benefit the cfg makes. It only complicates stuff. Cargo is not competent enough to see this cfg logic, and loom will be listed as an optional dependency even when put behind a cfg like this. That was the original problem, that loom was part of the dependency tree even when put behind a cfg.

Putting it behind a cfg as well as making it optional only introduced issues for me. Building with cargo build --features loom failed, because the feature was activated, but the dependency was not pulled in etc.

Making loom an optional feature only seems to solve the problem rather OK. Except for the fact that it's not really an additive feature, nor is it supposed to be used by consumers of this library. It would have been better if cargo had some "internal features" support.

[faern]

@Wuelle Maybe you would like to take a look? You helped improve the situation around loom just a few weeks ago. EDIT: I'm going to merge this, but please feel free to come with feedback and help me spot problems with this approach.

[simonwuelker]

@Wuelle Maybe you would like to take a look? You helped improve the situation around loom just a few weeks ago. EDIT: I'm going to merge this, but please feel free to come with feedback and help me spot problems with this approach.

Ah sorry, I totally missed this! I don't understand the combination of cfg + feature used by oneshot either - they probably want to keep their features additive so it does nothing unless the cfg is also enabled? Doesn't really make much sense imo... Your changes look fine, its what I would have done too. Keeping the dependency tree small is more important than strictly additive features in my opinion.

Also I agree, the way cargo deals with cfg parameters is quite - lacking. Another problem I personally ran into in another project is that since the cfg is usually also available to dependencies it invalidates the cache and causes huge recompiles... Using cargo rustc fixes that, but then cfg-dependent dependencies don't work anymore - anyways, I digress