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

Enable target-required-features with --all-features #10332

Closed
pinkforest opened this issue Jan 26, 2022 · 4 comments
Closed

Enable target-required-features with --all-features #10332

pinkforest opened this issue Jan 26, 2022 · 4 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@pinkforest
Copy link
Contributor

pinkforest commented Jan 26, 2022

Problem

If you have multiple [[targets]]

It can be pretty exhausting to manually juggle with the individual deviating --features sets at CLI under [[target]]

Especially as --all-features does not take into account / resolve the dependency features which in turn overrides explicit --features [another issue]

There was a previous issue and stale PR to trigger dependency features under [[target]]
#1570
#2325

But required-features was decided instead
#3667

And without separating things into workspaces relying on --features can become big hurdlesome list.

Proposed Solution

Option 0 - Do nothing or Status Quo

Yes I could make for example feature-relay hack say under the package

[features]
example-features = ["dependency/*"]

This would enable them via --all-features but it's not elegant way as the user who runs these examples would just do cargo run --example foo without too much thinking.

Option 1 - --all-features to enable contextual required-features

If cargo run --all-features --example foo is ran -

I would expect cargo to enable the required-features on dependencies contextually on per-target basis.

Option 2 - features under [[target]]

Re-think #1570 and perhaps support feature enable trigger under [[target]]

Option 3 - fix --features with --all-features

See #10333

Notes

This was discussed in zulip
https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.60--all-features.60.20ignores.20.60--features.60.20and.20suggests.20using.20it/near/269409108

@pinkforest pinkforest added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jan 26, 2022
@ehuss
Copy link
Contributor

ehuss commented Feb 27, 2022

Sorry, I'm having a hard time trying to understand what this issue is asking for. Can you show an example project, and what is difficult to manage in it? Which cargo commands are you trying to run? I'm also not quite following what is meant by [[target]], is that referring to tables like [[example]]?

@pinkforest
Copy link
Contributor Author

pinkforest commented Feb 27, 2022

@ehuss I created this in collaboration (and written before it was merged) with Issue #10333 and is derived as an addition to that

#10333 now has fixed to a level that you can now manually enable additional --features together with --all-features

Core of the problem is --all-features solely represents root package defined features[]

Also we discussed this in zulip which led to these two issues -
https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.60--all-features.60.20ignores.20.60--features.60.20and.20suggests.20using.20it/near/269409108

This is for any [[target]] really.. what I was trying to convey here without repeating from #10333 that each [[target]] may have differing (additive) feature sets from dependencies derived features (not just root package features) and that has been recognised via the required-features field under each [[target]] - be it a test, an example or anything else.

However #10333 didn't go more in to describing that we probably don't want to be listing all the downstream package dependency features in our root package manifest features[] or toggle with --features manually when e.g. doing build for all targets - or [[test]] for cargo test or [[bench]] for bench etc.

Since anotherpkg/x would be additive when we already have anotherpkg as dependency we should be able to toggle more anotherpkg features:

e.g. this will not work with --all-features you have to manually toggle these features which is a pain

[dependencies]
anotherpkg = "0.1"

[[example]]
name = "example1"
required-features = ["anotherpkg/aaaaaaaa"]

[[example]]
name = "example2"
required-features = ["anotherpkg/bbbbbbbb"]

[[example]]
name = "example3"
required-features = ["anotherpkg/ccccccccc"]

My additional take on to #10333 was that was since every [[target]] has required features under and all within the root package resolution reach to understand about them already -

Can't we just say --all-features and be good?

e.g. when cargo builds [[example]] it should automatically enable those additive required-features to toggle on by itself when iterating the [[target]] in any relevant context.

Since otherwise you have to either proxy this via root package features or manually toggle every feature on via --features that the [[target]] asks via required features under it - which seems counterintuitive considering we are aiming for expressiveness.

@ehuss
Copy link
Contributor

ehuss commented Feb 27, 2022

I'm still having a hard time trying to follow, but I believe --all-features will not be able to enable features on all dependencies.

If you're asking that you want building an example to automatically enable a feature, there are other existing issues like #1982 (and accompanying RFCs like rust-lang/rfcs#3020 or rust-lang/rfcs#2887). That would make it so that in your example, cargo build --examples would provide a way to force enable features (instead of the current behavior of required-features which just makes the example skip if not enabled).

Does that cover your use case?

@pinkforest
Copy link
Contributor Author

pinkforest commented Feb 28, 2022

So looks like @pksunkara was going to work on changing something by adding lib-features under [[target]]

Considering the history of how required-features came to be which considered something similar but under a combined field:

#1570
#2325
#3667

I was merely proposing to work solely with required-features under [[target]] without changing it otherwise.

Whilst considering the above history and the RFCs -

I understand that using solely required-features to do what I was suggesting was considered under "Rationale and alternatives" on rust-lang/rfcs/#3020 -

I was simply proposing for cargo to enable features dynamically from required-features automatically instead of asking the user enable them manually or introducing another field to archieve something similar.

e.g. if I want to build all [[bin]] targets - that each could have different feature dependencies from downstream - I shouldn't be needing to enable the required-features manually already visible to cargo resolver -

Or - relevant to rust-lang/rfcs/#3020 - use another field to describe what features I need from elsewhere when the required-features already describes this - my opinion is that default features of root crate should be honored in any [[target]] as the design for the features was to be "additive" by it's core principle which the lib-features seems to go against.

The error also seems redundant that complains some required-features is not separately enabled when building the project and installing binaries - would the lib-features retain the redundant error?

Bottom line:

So is cargo going to preserve required-features and / or is it going to be changed with the addition of lib-features which seems redundant along the error as well as going against the original design intent / principle or nature of additive features?

Maybe I should roll a PR - maybe that is easiest....

In the meantime I will close this as rust-lang/rfcs/#3020 was going already on this - I will add my input to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

No branches or pull requests

2 participants