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

Loom should be dev dependency #400

Closed
JordiPolo opened this issue Jun 19, 2020 · 14 comments
Closed

Loom should be dev dependency #400

JordiPolo opened this issue Jun 19, 2020 · 14 comments

Comments

@JordiPolo
Copy link

Bytes brings as dependency loom which itself brings a lot of stuff. This is very surprising because loom is a testing utility.

@seanmonstar
Copy link
Member

It's listed only for cfg(loom).

@Ralith
Copy link

Ralith commented Jun 19, 2020

Does that actually work?

@JordiPolo
Copy link
Author

JordiPolo commented Jun 19, 2020

I'm getting bytes via reqwest and tokio, no extra configuration or anything and I do get loom

@jplatte
Copy link
Member

jplatte commented Jun 19, 2020

@JordiPolo Can't reproduce. Creating a dummy project and adding tokio with features = ["full"] + reqwest, then running cargo tree, does not turn up loom.

@JordiPolo
Copy link
Author

JordiPolo commented Jun 19, 2020

Maybe is my ignorance of how Cargo works then.
Indeed cargo tree does not show loom but it is in the Cargo.lock file as dependency of bytes. I thought that was enough for the project to compile it.

[[package]]
name = "bytes"
version = "0.5.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
 "loom 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)",
]

@jplatte
Copy link
Member

jplatte commented Jun 19, 2020

With my dummy project it also wasn't compiled. I'm pretty sure the lockfile is target-independent, so compiling the same project for different targets doesn't change the lockfile back and forth (if it would that would be very annoying for applications that keep the lockfile in source control).

@jplatte
Copy link
Member

jplatte commented Jun 19, 2020

crates.io could really use some UI for target-specific dependencies though...

@JordiPolo
Copy link
Author

Yeah, just looking at the files in my repo I do not have enough information to know if I am going to compile this into my production application which can be scary.
Anyways, this turned out to not be an issue, thank you for looking into it!

@benma
Copy link

benma commented Jul 2, 2020

Please reopen this issue. It is also a problem when using cargo vendor, which pulls in a ton of crates that should not be there.

@jplatte
Copy link
Member

jplatte commented Jul 2, 2020

Seems like something that should be handled as an option in cargo vendor, like cargo metadatas --filter-platform or cargo trees --target.

@benma
Copy link

benma commented Jul 2, 2020

Seems like something that should be handled as an option in cargo vendor, like cargo metadatas --filter-platform or cargo trees --target.

Would be nice, but I don't think it exists? See also https://www.reddit.com/r/rust/comments/8plauq/dont_vendor_deps_i_dont_need/

@jplatte
Copy link
Member

jplatte commented Jul 2, 2020

Doesn't look like it but still, if the extra stuff being downloaded is an issue for you, you should probably try to get cargo fixed rather than having bytes work around it. The main issue seems to be rust-lang/cargo#4544.

@benma
Copy link

benma commented Jul 2, 2020

I agree in principle, but in practice it could take years to fix it in cargo. Maybe there is a simple workaround in bytes that could help in the meantime.

In any case, the solution I went for in the end is to fork bytes, remove the loom dep, and patch it in with a [patch] section in my Cargo.toml. It's not great, especially as I need to maintain the fork and keep it in sync with new releases.

@carllerche
Copy link
Member

Switching it to a dev-dependency should be a trivial PR. Anyone here should feel free to submit it.

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

No branches or pull requests

6 participants