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

Disabling features doesn't disable dependencies #36

Closed
mcobzarenco opened this issue Dec 28, 2024 · 4 comments · Fixed by #39
Closed

Disabling features doesn't disable dependencies #36

mcobzarenco opened this issue Dec 28, 2024 · 4 comments · Fixed by #39
Assignees
Labels
bug Something isn't working

Comments

@mcobzarenco
Copy link

mcobzarenco commented Dec 28, 2024

First off -- thank you for this crate 🙌, great stuff!

I am trying to use it with rand_chacha, but even if that's the only cargo feature I include, the crate still depends on wyrand, rand_xoshiro and rand_pcg. I thought these are all different "backends" which you can select from using cargo features.

E.g.

~ cargo add bevy_rand --no-default-features --features rand_chacha
    Updating crates.io index
      Adding bevy_rand v0.8.0 to dependencies
             Features:
             + rand_chacha
             - experimental
             - rand_pcg
             - rand_xoshiro
             - serialize
             - thread_local_entropy
             - wyrand
    Updating crates.io index
    Blocking waiting for file lock on package cache
     Locking 5 packages to latest compatible versions
      Adding bevy_prng v0.8.0
      Adding bevy_rand v0.8.0
      Adding rand_pcg v0.3.1
      Adding rand_xoshiro v0.6.0
      Adding wyrand v0.2.1
@Bluefinger
Copy link
Owner

Okay, this is a weird one. It seems that bevy_prng is pulling these in due to the serialize feature... but for rand_pcg, it is specified using rand_pcg?/serde1. The ? should make this optional and only take effect if something else has enabled rand_pcg. Hmmm.... I'll have to poke further

@Bluefinger Bluefinger added the bug Something isn't working label Dec 29, 2024
@Bluefinger Bluefinger self-assigned this Dec 29, 2024
@Bluefinger
Copy link
Owner

Actually, looking at this, this seems to be mostly to do with just the Lockfile and locking dependency versions. It doesn't actually pull in the dependencies themselves. @mcobzarenco Could you double check your basic app/crate setup with cargo tree and post what you see after doing the install as described above? You should see something like the following:

└── bevy_rand feature "rand_chacha"
    ├── bevy_rand v0.9.0 (https://github.com/Bluefinger/bevy_rand/?branch=feature-dep-fixing#938aae23)
    │   ├── bevy_reflect v0.15.0-dev (https://github.com/bevyengine/bevy#64efd08e) (*)
    │   ├── bevy_app feature "default" (*)
    │   ├── bevy_ecs feature "default" (*)
    │   ├── rand_core feature "default" (*)
    │   ├── rand_core feature "std" (*)
    │   ├── getrandom feature "default" (*)
    │   └── bevy_prng feature "default"
    │       ├── bevy_prng v0.9.0 (https://github.com/Bluefinger/bevy_rand/?branch=feature-dep-fixing#938aae23)
    │       │   ├── bevy_reflect v0.15.0-dev (https://github.com/bevyengine/bevy#64efd08e) (*)
    │       │   ├── serde feature "default" (*)
    │       │   ├── serde_derive feature "default" (*)
    │       │   ├── rand_chacha feature "default"
    │       │   │   ├── rand_chacha v0.3.1 (*)
    │       │   │   └── rand_chacha feature "std" (*)
    │       │   ├── rand_core feature "default" (*)
    │       │   └── rand_core feature "std" (*)
    │       └── bevy_prng feature "serialize"
    │           ├── bevy_prng v0.9.0 (https://github.com/Bluefinger/bevy_rand/?branch=feature-dep-fixing#938aae23) (*)
    │           ├── rand_chacha feature "serde1"
    │           │   ├── rand_chacha v0.3.1 (*)
    │           │   └── rand_chacha feature "serde"
    │           │       └── rand_chacha v0.3.1 (*)
    │           └── rand_core feature "serde1"
    │               ├── rand_core v0.6.4 (*)
    │               └── rand_core feature "serde"
    │                   └── rand_core v0.6.4 (*)
    └── bevy_prng feature "rand_chacha"
        └── bevy_prng v0.9.0 (https://github.com/Bluefinger/bevy_rand/?branch=feature-dep-fixing#938aae23) (*)

Check that the tree doesn't show rand_pcg/rand_xoshiro/wyrand and only rand_chacha. If it does, then something weird is going on and probably should be shunted to cargo.

@mcobzarenco
Copy link
Author

Okay, this is a weird one. It seems that bevy_prng is pulling these in due to the serialize feature... but for rand_pcg, it is specified using rand_pcg?/serde1. The ? should make this optional and only take effect if something else has enabled rand_pcg. Hmmm.... I'll have to poke further

I've also done a bunch of poking yesterday, everything seems correctly configured AFAICT

Actually, looking at this, this seems to be mostly to do with just the Lockfile and locking dependency versions.

Interesting, I am a bit surprised cargo would lock dependency versions for unused dependencies of a crate -- given that I can depend on it in a different way. it seems that this should not factor into dependency resolution 🤔

Digging a bit more into this, there is a cargo issue which seems relevant.

@mcobzarenco Could you double check your basic app/crate setup with cargo tree and post what you see after doing the install as described above? You should see something like the following:

I've tried it, looks good 👍 -- TIL

On a different note, I noticed there's a dependency on serde in bevy_prng as it has default = ["serialize"]. As bevy_rand depends on bevy_prng with default features enabled and features are additive, I don't think there's any way to disable it (?).

I wonder if that shouldn't be by default off or bevy_rand could disable default features when depending on bevy_prng and selectively enable stuff.

@Bluefinger
Copy link
Owner

Yeah, I am rethinking that default, so the PR I put up will probably change that as well, since it shouldn't be pulling in serde by default for what is a more low-level crate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants