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

Problem: sputnikvm users don't want to download large gaslighter dependencies #79

Closed
sorpaas opened this issue May 6, 2017 · 15 comments

Comments

@sorpaas
Copy link
Contributor

sorpaas commented May 6, 2017

Cargo still doesn't support bin dependencies (rust-lang/cargo#1982). However, several dependencies we currently have (like JSON parser) are only for the bin target.

Until that, I suggest we separate sputnikvm and gaslighter into two different crates. This basically creates a structure like below:

sputnikvm/Cargo.toml
sputnikvm/src/mod.rs
sputnikvm/src/vm/..
gaslighter/Cargo.toml
gaslighter/src/mod.rs

The sputnikvm will then be used to build the library, and the gaslighter will be used to build the executable.

@sjmackenzie
Copy link
Contributor

We can make gaslighter deps as dev dependencies? Would that work? But your way is equally viable. Go for it!

@sorpaas
Copy link
Contributor Author

sorpaas commented May 7, 2017

I'm planning to do this after testing has been finished.

@sjmackenzie
Copy link
Contributor

sjmackenzie commented May 8, 2017

So you're proposing to split gaslighter and sputnikvm into different git repositories?

@sorpaas
Copy link
Contributor Author

sorpaas commented May 8, 2017

We can just split them into different subfolders under https://github.com/ethereumproject/sputnikvm
There's no need to create multiple repositories.

@sjmackenzie
Copy link
Contributor

I see, okay great

@sjmackenzie
Copy link
Contributor

I'll do this now, though I'm keeping the src folder in situ as the lack of an src directory is one of the things that breaks nixcrates :-) I'll add a separate bin folder on the same mezzanine as src.

@sorpaas
Copy link
Contributor Author

sorpaas commented May 8, 2017

Thanks!

@sjmackenzie
Copy link
Contributor

Actually gaslighter dependencies are not present in sputnikvm.

The src/bin/gaslighter/mod.rs makes a number of calls to extern crate ... this indicates that.
It would also seem that having a bin directory in the src directory is standard rust conventions.

@sorpaas
Copy link
Contributor Author

sorpaas commented May 8, 2017

But Cargo indeed pulls all those dependencies in when building the library. See rust-lang/cargo#1982

@sjmackenzie
Copy link
Contributor

This seems to be current convention. You can't get cargo to 1) build multiple libraries 2) selectively build a binary (I'm quite sure on number 1, but not entirely sure of number 2). This is standard operating procedure for carge.

@sorpaas
Copy link
Contributor Author

sorpaas commented May 8, 2017

So until this Cargo issue is fixed, let's just separate sputnikvm and gaslighter to different crates.

@sjmackenzie
Copy link
Contributor

sjmackenzie commented May 8, 2017

I still fail to see why this is a problem.

To make different crates you need two "top level" Cargo.toml files. This means a few things: the crate sputnikvm library likes to have a single point of entry (you can only have one [lib] per Cargo.toml), this ideally should be in top level directory. To satisfy your wish the [[bin]]\nname = "gaslighter" should not be included into $ROOT/Cargo.toml. It also means you need to include a relative path to sputnikvm in $ROOT/gaslighter/Cargo.toml.

Cargo will download all deps anyway and put them in $HOME/.cargo/ thereafter it'll compile all deps to build all artefacts (sputnikvm and gaslighter). If you make a change to gaslighter only that change will be compiled. Downloading dependencies is negligible.

Dependencies that are present in gaslighter are not linked into sputnikvm. Only if src/lib.rs uses the same extern crate ... are the dependencies of the two artefacts the same.

In other words the artefacts are already completely separated and the current state of the repository already works with expected behaviour. There is only one crate sputnikvm, gaslighter isn't actually a crate, or more to the point cannot be a crate.

@sorpaas
Copy link
Contributor Author

sorpaas commented May 8, 2017

But if a library user wants to pull sputnikvm, he or she would not want to pull some large dependencies required by gaslighter, right?

Many projects like servo, pumpkindb, parity, redox (and rux!) uses multiple Cargo.toml files for separation of different libaries and binaries. Some optionally use the Cargo's workspace feature to group multiple cargo libraries and binaries together.

@sjmackenzie sjmackenzie changed the title Problem: some dependencies are only for gaslighter Problem: sputnikvm users don't want to download large gaslighter dependencies May 8, 2017
@sjmackenzie
Copy link
Contributor

okay I see your problem. Yeah okay I'll do that now.

@sjmackenzie
Copy link
Contributor

solved by #91

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

2 participants