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

Set default-features = false on rkyv #416

Merged

Conversation

EmilioLaiso
Copy link
Contributor

@EmilioLaiso EmilioLaiso commented Jul 14, 2023

Using default features pulls in the size_32 feature, but the choice of the size_x feature should be left to the top-level crate as it was thought of in #223 (comment) and #223 (comment), and should be working now that glam-rs is on edition 2021 and rust >= 1.50.

So that when building `--all-features` (turns on `rkyv`) `--all-targets`
(builds `benches` etc), there is no error that at least one `size_xx`
must be set.
@MarijnS95
Copy link
Contributor

@bitshifter what's your preference to set these "required" features on dependent crates to have the least friction while developing glam? We've put it in [dev-dependencies] so that --all-features (as long as it is used with e.g. --all-targets), but that obviously isn't taken into account for cargo doc.

@bitshifter
Copy link
Owner

I suspect I left default features enabled on rkyv because I ran into this problem. Ideally default features would be disabled for all dependencies. I wonder if there is something rkyv could do to make this easier to deal with.

@MarijnS95
Copy link
Contributor

MarijnS95 commented Jul 21, 2023

@bitshifter Unfortunately I don't think cargo really allows for something better... except --cfg in RUSTFLAGS which can be controlled globally and don't require this mess as long as there's a sensible default somewhere.

@bitshifter
Copy link
Owner

I guess I can live with rkyv being a dev-dependency. Adding --features "rkyv/size_32" to the doc build should fix it I think, at least that worked locally.

@MarijnS95
Copy link
Contributor

@bitshifter that works indeed. I'm not too keen on having it hardcoded there, but it seems to be the only solution 👌

@bitshifter bitshifter merged commit abf1a3c into bitshifter:main Jul 22, 2023
15 checks passed
@MarijnS95 MarijnS95 deleted the rkyv-no-default-features branch July 22, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants