wasm-bindgen-rayon
wasm-bindgen-rayon copied to clipboard
Using clippy got harder with the explicit target_features `atomics` check.
https://github.com/GoogleChromeLabs/wasm-bindgen-rayon/blob/4cd0666d2089886d6e8731de2371e7210f848c5d/src/lib.rs#L14-L15
This line causes clippy to fail if you didn't set the appropriate compiler flags and used the right --target
arguments.
Commenting out the lines prevents this failure as it then triggers in a part of the compiler clippy doesn't care about.
The problem with this is that it makes thinks like clippy check --all-features --all-targets
not usable (without some hacks),
and in turn complicates dev and CI workflows of projects/workspaces for which wasm
is just one of multiple targets.
I'm not sure what the best way to fix this is, as the error message without this "early fail" isn't the most intuitive one.
Once the other hand this only checks for atomics
but not for the other two required options :thinking:, so there
is a good chance people will run into this error anyway.
(What I do for now is to explicitly exclude the wasm
ffi
crate and run a separate clippy check on it.)
The problem with this is that it makes thinks like
clippy check --all-features --all-targets
not usable (without some hacks), and in turn complicates dev and CI workflows of projects/workspaces for whichwasm
is just one of multiple targets.
If wasm is just one of multiple targets, chances are you're already using cfg(target_arch = "wasm32")
in some places to exclude Wasm-specific dependencies. When you do that, Clippy issues should go away naturally.
OTOH, if it is a Wasm-specific code path, then it's best to add .cargo/config
as per README, and run Clippy with --target wasm32-unknown-unknown
, which should work correctly.
Thanks for the response.
exclude Wasm-specific dependencies
The problem is not that the wrong files are included (we made sure they are not) but that clippy checking everything including wasm fails.
The options to make clippy not fail for wasm will make it fail for other targets.
.cargo/config
seems like a way to set the options only for the right target, but in practice this doesn't work due to two reasons:
- ~How clippy handles the
--all-targets
flag conflicts withbuild-std
leading to following error:error: -Zbuild-std requires --target
(I will look into that and potentially open a issue with clippy if neccessary).~ (EDIT: I mixed up build targets and platform targets, sorry for that)- There is also the thing that the
build-std
option is not scoped to WASM which is a problem. - EDIT: you always need to supply the
--target
parameter manually which is sub-optimal
- There is also the thing that the
- I can't really use
.cargo/config
in my case, as we target bothwasm32
withatomics
andwasm32
without.
So what I did was:
In the workspace root set: resolver="2"
And then in the ffi-wasm
crate I included:
[target.'cfg(all(target_arch="wasm32", target_feature = "atomics"))'.dependencies]
wasm-bindgen-rayon = { version = "1.0.3", features = ["no-bundler"] }
non-target-specific-core = { path = "../non-target-specific-core", features=["parallel"] }
[target.'cfg(not(all(target_arch="wasm32", target_feature = "atomics")))'.dependencies]
non-target-specific-core = { path = "../non-target-specific-core" }
As well as some small changes in ffi-wasm
.
Due to how we split the code base into non-target-specific core parts and targets specific FFI bindings this works rather well.
Is it ok if I make a doc-PR adding a section which hints at target.'cfg'
+ resolver="2"
being useful?|
I also would close the issue with that. Given that people (should) search closed issues too when they have a problem, there would be little reason to keep it open once the documentation mentioned the less often used [target.'cfg']
and resolver="2"
options (which are not specific to this project but are something everyone who wants to e.g. optionally support parallelism likely needs).
I guess a doc PR would be good, but OTOH I'm a bit worried that docs are already quite overwhelming and this seems like one of more niche use-cases...
I'd say we can keep this issue open for now and see if anyone else runs into the same problem; if not, perhaps we'll just be able to close it and over time I'm hoping resolver="2"
will become the default for new projects on Cargo side.
resolver="2"
is the new default in the new edition.
you could set rustflags to use with clippy in .cargo/config (we do this so that a certain set of clippy rules is used):
[target.'cfg(feature = "cargo-clippy")']
rustflags = [
"-C", "target-feature=+atomics,+bulk-memory,+mutable-globals"
"-Aclippy::all", .. your favourite clippy include / exclude list here ...
]
I guess I'll close this since resolver=2 has been the default for some time now and, in my understanding, avoids the related issue.