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

Target-specific features #1197

Open
alexcrichton opened this issue Jan 20, 2015 · 40 comments
Open

Target-specific features #1197

alexcrichton opened this issue Jan 20, 2015 · 40 comments
Assignees
Labels
A-features

Comments

@alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Jan 20, 2015

It would be nice to support target-specific features for cases such as when certain functionality is only available on one platform or the default set of functionality should vary per-platform.

@alexcrichton alexcrichton added the A-features label Jan 20, 2015
@alexcrichton
Copy link
Member Author

@alexcrichton alexcrichton commented Jan 20, 2015

cc @huonw

@nox
Copy link
Contributor

@nox nox commented Jul 10, 2016

Shouldn't this work?

[target.'cfg(not(any(target_os = "android", target_os = "windows")))'.dependencies]
ipc-channel = {git = "https://github.com/servo/ipc-channel"}

[target.'cfg(any(target_os = "android", target_os = "windows"))'.dependencies]
ipc-channel = {git = "https://github.com/servo/ipc-channel", features = ["inprocess"]}

@alexcrichton
Copy link
Member Author

@alexcrichton alexcrichton commented Jul 11, 2016

@nox in theory yeah that seems like it should work, but today due to the structure of Resolve it doesn't. The entire target-independent resolution graph is created and then only later filtered down to the relevant set of packages. As an artifact of creating Resolve at the beginning it resolves all features, basically with a map from package to list of features. That means that all instances of ipc-channel in your example would have the inprocess feature enabled, regardless of what platform it was on.

I think that's a fixable bug, however, although likely quite invasive as it would require deferring calculation of features to the compilation phase rather than the resolution phase.

@jethrogb
Copy link
Contributor

@jethrogb jethrogb commented Nov 5, 2016

Ran into this extremely confusing bug today...

@gyscos
Copy link

@gyscos gyscos commented Dec 20, 2016

Would this also allow specifying a target-specific features.default array?
Something like:

[target.'cfg(not(target_os = "windows"))'.features]
default = ["general_feature"]

[target.'cfg(target_os = "windows")'.features]
default = ["winonly_replacement"]

I suppose a workaround to get this would be an intermediate crate which depends on the actual crate with target-dependent features, but it doesn't sound very convenient.

@alexcrichton
Copy link
Member Author

@alexcrichton alexcrichton commented Dec 20, 2016

@gyscos that seems like a nifty and natural way to encode this information! I haven't though too much about the implications of target-specific features, but I don't particularly see any immediate reason to block anything.

@nox
Copy link
Contributor

@nox nox commented Mar 4, 2017

This isn't enough, I don't want my crate to stop working because someone used no-default-features.

I guess one could use @retep998's trick of enabling a feature programatically from a build.rs script.

@retep998
Copy link
Member

@retep998 retep998 commented Mar 4, 2017

@nox That enables features in code programmatically. It doesn't let me control dependencies, but for some people just controlling their code might be enough. The reason I even wrote that trick was due to the lack of support for cyclic features. https://github.com/retep998/winapi-rs/blob/dev/build.rs#L193

@philn
Copy link

@philn philn commented Oct 18, 2017

What's the status of this issue? It would be interesting to use target-specific features in gecko-media (enable pulseaudio only for linux for instance).

@alexcrichton
Copy link
Member Author

@alexcrichton alexcrichton commented Oct 18, 2017

@philn AFAIK no progress has been made

@praetp
Copy link

@praetp praetp commented May 3, 2018

Hope we can see progress on this issue...

@drahnr
Copy link

@drahnr drahnr commented Aug 19, 2021

I think the current syntax makes this more confusing that it needs to be. The current [features] section does two things: it declare what the features are, and specifies the dependencies between features.

I disagree, it is in line with the target specific dependency declaration so imho it's consistent.

I think based on what the others say there is good need for target-specific dependencies. Note that this includes target specific default features as they are target-specific dependencies of the "default" feature.

However, there is no need for the mere existence of features to vary between platforms. It would be much better to solve that with something like Cabal's "buildable: false". Not only can that nicely express not all features work everywhere, it also can be used to express that certain features are incompatible wihout violating the additivity of features! (Technically, one can express "buildable: false" today with an impossible dependency, so we know it's safe. This just takes that existing functionality and makes it more ergonomic.)

Good catch! Couldn't this be solved by an impl detail to check if a feature exists at all before checking if it exists for the current target, and show a sufficiently detailed error message? Iiuc that would be equal to an explicit buildable flag. Could you reference where this is outlined? Thanks!

@JAicewizard
Copy link

@JAicewizard JAicewizard commented Aug 19, 2021

However, there is no need for the mere existence of features to vary between platforms

I dont think that is true. We can already request optional dependencies (which are features) form another platform to be included, and this works on stable. I think my implementation also has a similar behaviour to these dependencies. On targets where the features dont exist, they are just ignored/are empty.

It would be much better to solve that with something like Cabal's "buildable: false".

That would require the users to select the features, I want to give the user one stable API on diferent platforms without the users having to ask for a diferent feature on each platform.

@Ericson2314
Copy link
Contributor

@Ericson2314 Ericson2314 commented Aug 19, 2021

@JAicewizard

We can already request optional dependencies (which are features) form another platform to be included, and this works on stable.

Requesting conditionally is perfectly fine. It's providing features optionally to downstream creates conditionally that's suspicious.

That would require the users to select the features, I want to give the user one stable API on diferent platforms without the users having to ask for a different feature on each platform.

That is what the conditional default features do. Isn't that what you want?

@drahnr

I disagree, it is in line with the target specific dependency declaration so imho it's consistent.

[target."....".features] is consistent with [target."....".dependencies], but only the former has the issue I mention I believe? It's precisely the fact that doing the consistent runs into issues that means we should perhaps back up and fine something more different ("a more flexible language for specifying dependencies") so we can be consistent and have good semantics.

Good catch! Couldn't this be solved by an impl detail to check if a feature exists at all before checking if it exists for the current target, and show a sufficiently detailed error message? Iiuc that would be equal to an explicit buildable flag.

That somewhat works, but i don't think is as clean. What we want to express is "Foo depends on macOS or Linux", not "If mac or linux exists there is a Foo". I think the framing really matters as to what users end up writing and how they feel about it.

Could you reference where this is outlined? Thanks!

https://cabal.readthedocs.io/en/3.4/cabal-package.html?highlight=buildable%3A#pkg-field-buildable. The docs mention it in conjunction with an autoconf-ish setup, but it is more commonly used these days just with regular conditional deps, such as in https://github.com/reflex-frp/reflex-dom/blob/master/reflex-dom/reflex-dom.cabal (which incidentally I think is pretty close to @JAicewizard's use-case).

@JAicewizard
Copy link

@JAicewizard JAicewizard commented Aug 19, 2021

@Ericson2314

Requesting conditionally is perfectly fine. It's providing features optionally to downstream creates conditionally that's suspicious.

I dont think you understand what I mean. Currently we can already provide features that are target specific.

[target."cfg(target_os=linux)".dependencies]
dep1 = {version = x, optional = true}

@Ericson2314
Copy link
Contributor

@Ericson2314 Ericson2314 commented Aug 19, 2021

@JAicewizard can a downstream create refer to dep1? I thought dependencies overloaded features as if they were "private" features, since the crate can refer to its own optional dependencies but not a downstream crate.

I agree in that this is no good, but if it doesn't like into the crate's public (cargo-side) interface, then it is at least local and could be fixed with an edition.

@JAicewizard
Copy link

@JAicewizard JAicewizard commented Aug 19, 2021

@Ericson2314 Yes, if you are on a target with no explicitly listed dependency with that name it will just ignore it. At least from my testing.

@Ericson2314
Copy link
Contributor

@Ericson2314 Ericson2314 commented Aug 19, 2021

@JAicewizard say that again? In downstream crates I would expect trying to refer to any dependency in an upstream crate (that doesn't share a name with a feature in which case we are referring to the feature) to be an error, not silently ignored.

I take it you mean within the same crate declaring the target-specific dependency?

@Cu3PO42
Copy link

@Cu3PO42 Cu3PO42 commented Aug 21, 2021

I believe I have a use-case for this. I am working on a cross-platform tool that needs to interact with a USB device. I use libusb via rusb to achieve the majority of the abstraction. On Windows, I need a special, patched version of libusb to get around a limitation. I therefore enable the vendored feature on the libusb1-sys crate, which builds and statically links libusb.

My dependencies are as follows:

[workspace]
[package]
# ...
edition = "2018"

[dependencies]
rusb = "0.8.1"
# ...

[target.'cfg(windows)'.dependencies]
libc = "0.2"
windows = "0.17.2"
win32_bindings = { path = "src/backend/windows/win32_bindings" }
libusb_internals = { path = "src/backend/windows/libusb_internals" 

where libusb_internals itself has the following dependencies:

[dependencies]
libusb1-sys = { version = "0.5.0", features = ["vendored"] }
win32_bindings = { path = "../win32_bindings" }

[build-dependencies]
cc = "1.0"

On other platforms, I would rather dynamically link libusb, however, the "vendored" feature appears to be picked up and libusb is being built. I also see libusb_interals-* folders in target/debug/build/ which makes sense given that target-specific dependencies are only filtered later in the process. All this wouldn't be a huge problem if it weren't for the fact that libusb1-sys apparently doesn't set up the correct environment on (older versions of) macOS and fails to build libusb. I can manually build libusb or install it from Homebrew without problem, though. Manually commenting out the Windows-specific libusb_internals dependency lets my project build correctly on macOS.

This leaves me at an impasse where it seems impossible to configure my project correctly for multiple target platforms.

barrettottej added a commit to barrettottej/amethyst7 that referenced this issue Mar 1, 2022
@kaimast
Copy link

@kaimast kaimast commented Mar 22, 2022

Somewhat related: Cargo.lock currently tracks dependencies/features across all build targets which can be a problem.

Here's an example: I have an codebase I build for WebAssembly and as a native binary. In the latter case I use the rand crate with std and getrandom.
However, getrandom does not support WebAssembly so it won't compile, even when I explicitly disable all default features on the wasm target.

@CryZe
Copy link

@CryZe CryZe commented Mar 22, 2022

@kaimast Are you using Rust 2021 and / or resolver 2? This should not be happening if either of those are the case. (Unless of course a crate legitimately pulled in by WASM does indeed activate that feature)

Although wait, what do you even mean? getrandom does support WASM, you actually need to activate a feature for it to work, not vice versa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features
Projects
None yet
Development

No branches or pull requests