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

building workspaces can't use --features flag #5015

Open
SamPeng87 opened this issue Feb 6, 2018 · 22 comments
Open

building workspaces can't use --features flag #5015

SamPeng87 opened this issue Feb 6, 2018 · 22 comments

Comments

@SamPeng87
Copy link

@SamPeng87 SamPeng87 commented Feb 6, 2018

i define feature in some sub-create

    [features]
    default=["product","dangerous_configuration"]
    dangerous_configuration=[]
    develop=[]
    product=[]

then,execute cargo cargo build --features="develop"

it can't work,only work in sub-create default features selectors.

cargo build --no-default-features --all not work too

Is it a Bug?

@alexcrichton

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Feb 6, 2018

Yes right now [features] is only allowed on individual crates rather than workspace configurations.

@SamPeng87

This comment has been minimized.

Copy link
Author

@SamPeng87 SamPeng87 commented Feb 6, 2018

Thank your report.Wish this feature can quick complete. I use two [[bin]] point some other needed feature.it is too low.but thank god this can good work.

@SamPeng87 SamPeng87 closed this Feb 6, 2018
@SamPeng87

This comment has been minimized.

Copy link
Author

@SamPeng87 SamPeng87 commented Feb 6, 2018

En..Can I push feature arguments with 'cargo build' pass to sub-creat?like my example. I think that looks very natural

@SamPeng87 SamPeng87 reopened this Feb 6, 2018
@alexcrichton

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Feb 6, 2018

You should be able to do --features foo/bar I think?

@sanmai-NL

This comment has been minimized.

Copy link

@sanmai-NL sanmai-NL commented Feb 24, 2018

@SamPeng87: could you please test alexcrichton’s suggestion, so that the issue can perhaps be closed?

@Nemo157

This comment has been minimized.

Copy link
Contributor

@Nemo157 Nemo157 commented Mar 5, 2018

Attempting to reference a feature in a member of a virtual manifest/workspace does not work (testing with cargo 0.26.0-nightly (1d6dfea44 2018-01-26) and almost rust-lang/futures-rs@2666a72):

.../futures-rs > cargo build --features futures-core/std -p futures-core -v
error: Package `futures-core v0.2.0 (file:///.../futures-rs/futures-core)` does not have these features: `futures-core`

.../futures-rs > cd futures-core

.../futures-rs/futures-core > cargo build --features futures-core/std -p futures-core -v
error: Package `futures-core v0.2.0 (file:///.../futures-rs/futures-core)` does not have these features: `futures-core`

.../futures-rs/futures-core > cargo build --features std -p futures-core -v
   Compiling futures-core v0.2.0 (file:///.../futures-rs/futures-core)
     Running `rustc --crate-name futures_core futures-core/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=314425e352544566 -C extra-filename=-314425e352544566 --out-dir .../futures-rs/target/debug/deps -C incremental=.../futures-rs/target/debug/incremental -L dependency=.../futures-rs/target/debug/deps`
    Finished dev [unoptimized + debuginfo] target(s) in 0.85 secs

Also, --no-default-features does not work in a workspace (note the --cfg 'feature="default"' in the rustc line):

.../futures-rs > cargo build --no-default-features -p futures-core -v
   Compiling futures-core v0.2.0 (file:///.../futures-rs/futures-core)
     Running `rustc --crate-name futures_core futures-core/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=314425e352544566 -C extra-filename=-314425e352544566 --out-dir .../futures-rs/target/debug/deps -C incremental=.../futures-rs/target/debug/incremental -L dependency=.../futures-rs/target/debug/deps`
    Finished dev [unoptimized + debuginfo] target(s) in 0.41 secs

I'm not sure whether there's a good behaviour for these to all have, but if they're not going to apply when building from a virtual manifest then maybe they should warn/error to inform the user?

@Nemo157

This comment has been minimized.

Copy link
Contributor

@Nemo157 Nemo157 commented Apr 16, 2018

See also #5364, that appears to cover most of the relevant behaviour and should hopefully close this.

@jhpratt

This comment has been minimized.

Copy link

@jhpratt jhpratt commented Apr 16, 2019

@alexcrichton Passing a feature flag works for crates in a workspace, but how can one disable a default feature flag?

@alexcrichton

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Apr 16, 2019

There is no way to selectively disable on default feature, rather you need to turn them all off with --no-default-features (or default-features = false) and then re-enable the other default features you still need manually

@jhpratt

This comment has been minimized.

Copy link

@jhpratt jhpratt commented Apr 16, 2019

It is possible to use the --no-default-featurees flag when building a workspace though? When I tried yesterday it didn't seem to work.

@alexcrichton

This comment has been minimized.

Copy link
Member

@alexcrichton alexcrichton commented Apr 17, 2019

Ah no, not currently, it has to be directed at a package not a workspace

@Manishearth

This comment has been minimized.

Copy link
Member

@Manishearth Manishearth commented Jul 26, 2019

You should be able to do --features foo/bar I think?

@alexcrichton this isn't working for me in https://github.com/servo/webxr/ (try --features webxr/ipc, it compiles but doesn't pull in ipc_channel)

This bug is particularly annoying because this forces me to build from a subfolder, but cargo sets the error message root as the workspace root, so i can't click on the errors to have them open in my editor

@Manishearth

This comment has been minimized.

Copy link
Member

@Manishearth Manishearth commented Jul 26, 2019

Ah, you can use --manifest-path to make it work (but you don't need to foo/bar it then)

@jonhoo

This comment has been minimized.

Copy link
Contributor

@jonhoo jonhoo commented Jul 28, 2019

I think having --features foo/bar work would still be better though. It would allow you to do things like "run all tests across all subcrates, and keep this feature enabled" without having to explicitly enumerate every crate (handy for things like general-purpose CI scripts).

@nox

This comment has been minimized.

Copy link
Contributor

@nox nox commented Aug 17, 2019

Is there any ongoing work for --features foo/bar? Or guidelines as to where to look at in the code to implement such a thing?

geigerzaehler added a commit to oscoin/oscoin-parity-wasm-prototype that referenced this issue Aug 28, 2019
Before this change the ledger code was compiled _without_ `no_std` to
the Wasm target. This did not cause problems yet since we did not use
any `std` code. However, in the future we would have run into issues if
we inadvertently used `std` code.

The reason why `no_std` was not enabled was that the
`--no-default-features` flag was not respected in
`tool/build-ledger-wasm` because of
rust-lang/cargo#5015.
@m4b

This comment has been minimized.

Copy link

@m4b m4b commented Sep 15, 2019

What happened here? This seems like a pretty big usability paper cut; I just refactored into a workspace, and now it's very unpleasant to build...

@paddyhoran

This comment has been minimized.

Copy link

@paddyhoran paddyhoran commented Sep 18, 2019

Ran into this also, the fact that --no-default-features is not applied silently at the workspace level is misleading as CI builds all pass.

@l-x-u

This comment has been minimized.

Copy link

@l-x-u l-x-u commented Oct 4, 2019

Would also love this ability.

bors added a commit that referenced this issue Oct 15, 2019
Reject feature flags in a virtual workspace.

This generates an error if feature flags are used in the root of a virtual workspace. Previously these flags were completely ignored.  In the interest of avoiding confusion, I think it would be good to be explicit that these don't currently work.  This could alternatively be a warning, but I think it is better to reject it outright.

cc #4753, #3620, #5015, #6195, etc.
bors added a commit that referenced this issue Oct 15, 2019
Reject feature flags in a virtual workspace.

This generates an error if feature flags are used in the root of a virtual workspace. Previously these flags were completely ignored.  In the interest of avoiding confusion, I think it would be good to be explicit that these don't currently work.  This could alternatively be a warning, but I think it is better to reject it outright.

cc #4753, #3620, #5015, #6195, etc.
azasypkin added a commit to azasypkin/kroneum that referenced this issue Nov 26, 2019
ilammy added a commit to ilammy/themis that referenced this issue Dec 23, 2019
No idea what changed here, but for some reason Cargo does not allow to
use "--features" with virtual crates now. However, it has no issues with
it once we're in "themis" crate subdirectory and ask to document all
crates in the workspace anyway.

Apparently, "--features" was not supposed to work with workspaces in the
first place [1], but I have no clue it worked before.

[1]: rust-lang/cargo#5015
@ilammy ilammy mentioned this issue Dec 23, 2019
6 of 6 tasks complete
ilammy added a commit to cossacklabs/themis that referenced this issue Dec 23, 2019
* Fix clippy::needless-doctest-main

Recently released Clippy 1.40 ships with this lint enabled by default.
Since we run with "warnings as errors" policy, this lint causes build
failures which is not nice.

    error: needless `fn main` in doctest
      --> src/wrappers/themis/rust/libthemis-src/src/lib.rs:37:4
       |
    37 | //! fn main() {
       |    ^^^^^^^^^^^^
       |
       = note: `-D clippy::needless-doctest-main` implied by `-D warnings`
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main

    error: aborting due to previous error

I wouldn't say that explicit main() here is needless and some people
share the same sentiment [1]. However, since this is top-level module
docs, we cannot disable this lint for this place specifically without
turning it off in the entire module. Okay, Clippy, you win.

[1]: rust-lang/rust-clippy#4858

* Fix Cargo with "--features"

No idea what changed here, but for some reason Cargo does not allow to
use "--features" with virtual crates now. However, it has no issues with
it once we're in "themis" crate subdirectory and ask to document all
crates in the workspace anyway.

Apparently, "--features" was not supposed to work with workspaces in the
first place [1], but I have no clue it worked before.

[1]: rust-lang/cargo#5015
@drahnr drahnr mentioned this issue Mar 10, 2020
@nixpulvis

This comment has been minimized.

Copy link

@nixpulvis nixpulvis commented Mar 12, 2020

I'm sure there's a few reasons why this doesn't work, but a naive attempt to use --no-default-features in a workspace lead me here.

I feel like I'd expect a workspace to have effectively the union of the features of it's members. Members could even have the same features and they would be controlled as one. For example, in a workspace with:

[workspace]
members = [
    "foo",
    "bar",
]

I may have the following two manifests:

# foo/Cargo.toml
[features]
default = ["a", "c"]
a = []
b = []
c = []
d = []
# bar/Cargo.toml
[features]
default = ["a", "e"]
a = []
b = []
e = []
f = []

Here I think I'd expect the following:

  • cargo ... --no-default-features would disable all of foo/a, foo/c, bar/a and bar/e
  • cargo ... --features=b would enable both foo/b and bar/b
  • cargo ... --features=f would enable only bar/f

What am I missing? This seems somewhat straightforward. Could I take a stab at implementing this in cargo?

@Recmo

This comment has been minimized.

Copy link

@Recmo Recmo commented Mar 18, 2020

I'm stuck on this one now too.

Due to a different but related shortcoming I have bench and test features for all packages in the workspace (because dev-dependencies would enable std on some main dependencies, breaking the no-std build). To build it, I would just use cargo test --all-features in the root, because I can not specifically enable the test feature.

But now one of the packages in the workspace has a nightly-only feature, and using --all-features breaks stable.

I'm stuck. Any advice is appreciated.

@gilescope

This comment has been minimized.

Copy link
Contributor

@gilescope gilescope commented Mar 23, 2020

Isn't this a dupe of #4753 - can we close this one and continue the discussion there?

@nixpulvis

This comment has been minimized.

Copy link

@nixpulvis nixpulvis commented Mar 23, 2020

@gilescope I tried collecting all the related (possibly duplicate) issues in https://internals.rust-lang.org/t/features-and-workspaces-request-for-meta-issue/11964 hoping to aggregate them. No responses as of yet however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.