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

Single Cargo workspace for the entire repository #534

Closed
tiziano88 opened this issue Jan 28, 2020 · 7 comments · Fixed by #613
Closed

Single Cargo workspace for the entire repository #534

tiziano88 opened this issue Jan 28, 2020 · 7 comments · Fixed by #613
Assignees

Comments

@tiziano88
Copy link
Collaborator

We currently have at least 3 Cargo workspaces, and if we keep the current structure we may need to add at least an additional one for things like #533 .

We should consider having a single workspace rooted in the repository root, which will allow us to maintain a single version of our all dependencies. In this model, every single crate will be nested in the global workspace.

If anyone thinks this may be problematic, please follow up here.

@daviddrysdale
Copy link
Contributor

I slightly wonder whether we'll end up needing one workspace for each environment that Rust code runs in – are there any workspace-wide settings that might not work when they're shared between a build-for-Wasm-with-minimal-std and a build-for-x86-with-full-libraries?

@tiziano88
Copy link
Collaborator Author

AFAICT there are no workspace-wide settings, or if there are we don't seem to be using them anyways.

My understanding is that crates in a workspace would share their Cargo.lock file, cache, and output (target) folder. Under the target folder there are already subfolders for each target architecture / platform, so I think it should all be fine.

@tiziano88
Copy link
Collaborator Author

@wildarch do you happen to know if there is any downside in coalescing all our crate in a single top-level Cargo workspace?

My understanding is that we should still be able to individually publish individual crates, and people depending on those would only pull the minimum amount of dependencies (as opposed to all of the workspace dependencies). Anyone let me know if I am missing something though.

@wildarch
Copy link
Contributor

You can definitely publish individual crates in your workspace, there should be no issue there. Like @daviddrysdale I'm a bit skeptical about having code for multiple different targets in one workspace, is there a way to specify for what platform certain crates should be built?

@wildarch
Copy link
Contributor

Looks like that is not yet possible. This would mean that if you make a single workspace you probably couldn't just cargo build it in one go 😓

@ipetr0v
Copy link
Contributor

ipetr0v commented Feb 14, 2020

I think we might have 2 workspaces: examples and oak.
Later, when we will decouple examples in a separate repository, there will be only one workspace remaining in the oak repo.

@tiziano88
Copy link
Collaborator Author

Sure, later when we decouple the SDK from the runtime, it may also make sense to split those, but I still think for now we may be better off with a single workspace. Splitting things off is just a matter of re-creating the workspace file, all the rest is taken care of by cargo. I think we should go ahead with this, I don't expect it will be a large change.

ipetr0v added a commit that referenced this issue Feb 17, 2020
This change:
- Creates a new single Cargo workspace for the entire repository
- Removes old Cargo workspaces:
    - `examples/Cargo.toml`
    - `oak/server/rust/Cargo.toml`
    - `sdk/rust/Cargo.toml`
    - `third_party/grpc-rust/Cargo.toml`

Fixes #534
tiziano88 added a commit to tiziano88/oak that referenced this issue Feb 17, 2020
After project-oak#613 there is now a single Cargo workspace, so the script needs to
be changed to deal with that.

Note that this now also generates docs for example crates.

Ref. project-oak#534
tiziano88 added a commit that referenced this issue Feb 18, 2020
After #613 there is now a single Cargo workspace, so the script needs to
be changed to deal with that.

Note that this now also generates docs for example crates.

Ref. #534
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants