asomers / mockall

A powerful mock object library for Rust
Apache License 2.0
1.5k stars 62 forks source link

Switch to downcast-rs instead of downcast? #521

Closed mgeisler closed 11 months ago

mgeisler commented 11 months ago

Hi @asomers,

I'm looking at vendoring mockall to AOSP so that it becomes available for Android Platform development. As part of that, I would need to vendor the dependencies to our external/rust/crates/ directory.

These are the current dependencies, annotated to indicate crates that are not yet in AOSP:

% cargo tree -e normal
mockall v0.11.2
├── cfg-if v1.0.0
├── downcast v0.11.0  <-- missing
├── fragile v2.0.0  <-- missing
├── lazy_static v1.4.0
├── mockall_derive v0.11.2 (proc-macro)  <-- missing
│   ├── cfg-if v1.0.0
│   ├── proc-macro2 v1.0.67
│   │   └── unicode-ident v1.0.12
│   ├── quote v1.0.33
│   │   └── proc-macro2 v1.0.67 (*)
│   └── syn v2.0.37
│       ├── proc-macro2 v1.0.67 (*)
│       ├── quote v1.0.33 (*)
│       └── unicode-ident v1.0.12
├── predicates v3.0.4  <-- missing
│   ├── anstyle v1.0.3  <-- missing
│   ├── itertools v0.11.0
│   │   └── either v1.9.0
│   └── predicates-core v1.0.6  <-- missing
└── predicates-tree v1.0.9  <-- missing
    ├── predicates-core v1.0.6  <-- missing
    └── termtree v0.4.1  <-- missing

mockall_derive v0.11.2 (proc-macro)  <-- missing

We try to avoid duplicate dependencies in our tree, and I noticed that we have downcast-rs vendored alrady. From looking at the FAQ of downcast, it seems like the two crates are mostly doing the same job.

Would you be up for a PR which switches the dependency to downcast-rs instead?

asomers commented 11 months ago

Maybe. According to downcast's README, "the other one is more actively maintained". However, downcast actually has a more recent release than downcast-rs. Also, I previously attempted to switch to downcast-rs without success. So I don't want to try again absent a compelling reason. And I don't think that reducing Android's vendor branch is compelling enough. After all, a switch like this might increase somebody else's vendor branch.

mgeisler commented 11 months ago

Maybe. According to downcast's README, "the other one is more actively maintained". However, downcast actually has a more recent release than downcast-rs. Also, I previously attempted to switch to downcast-rs without success. So I don't want to try again absent a compelling reason.

That makes a lot of sense!

And I don't think that reducing Android's vendor branch is compelling enough. After all, a switch like this might increase somebody else's vendor branch.

True :slightly_smiling_face: I will have to look into this myself in case we import mockall: we can carry our own patches to remove the extra dependency if needed.

mgeisler commented 11 months ago

Hi again, we can close this — I'm going ahead with the import as-is! We're only using downcast-rs a few times in AOSP, so it might be simpler to switch those to use downcast instead.