-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Intelligently exclude redundant items in powerset #81
Comments
Published in 0.4.6. |
Looks good! By no means perfect, but it's still a significant improvement. |
How much can I push this? 😛 I wasn't even expecting this to be implemented for quite a while, to the point I debated whether to even file the issue to begin with! It was more "this is theoretically possible" initially. CI is already relatively fast for me, as using existing flags in addition to this change has significantly cut the size of the powerset. I know you work on some projects that are larger than mine, so I suppose it depends on whether you're happy with your CI times. The time crate currently has the following (effectively, I've simplified a bit) [dependencies]
const_fn = "0.4.3"
quickcheck-dep = { package = "quickcheck", version = "0.9.2", optional = true }
rand = { version = "0.7.3", optional = true }
serde = { version = "1.0.117", optional = true }
time-macros = { version = "=0.2.0-optional = true }
[target.'cfg(windows)'.dependencies]
winapi = "0.3.9" Currently there is one unnecessary check that I see that isn't possible to work around with existing flags:
As I said, this isn't an issue for me personally; it's just an observation. My naïve thought is to effectively brute force the minimal powerset. A forest of initially identical graphs could be created, pruning each as necessary. I might write a proof of concept to generate a minimal powerset at some point. There's probably some algorithm to this effect, but I'm not aware of anything in particular. Of course, this is all assuming equal weight to each dependency, but that's something that certainly isn't worth the effort. Thank you for your work, of course! Just throwing some things out there. |
Note that technically you can write code that will cause the compile to fail if these features are used together. I think a reasonable (and currently available) approach for crates with many features is the option that limits the max number of simultaneous feature flags of |
Whoops, just realized that I didn't include the list of features in that comment. quickcheck depends on std in that example. If it didn't, it would naturally be possible to break. |
Ah, I saw what's happening in |
That'll do it. I actually need to ungroup quickcheck regardless, as I recently realized that it means I'm not covering a plausible scenario. |
Given that FYI: filed #97 to track that bug. |
--group-features bug has been fixed in 0.4.8. |
Consider the following case (the actual crate doesn't matter; I just tested it on an empty crate).
I then perform
cargo hack check --feature-powerset --exclude-all-features
(I've inquired about true necessity of the latter flag elsewhere). This is the output:Presumably it would be possible to determine from
Cargo.toml
thatstd
relies onalloc
, and as suchalloc,std
is redundant (becausestd
is already in the powerset).The text was updated successfully, but these errors were encountered: