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

Create overarching Cargo workspace #110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tchebb
Copy link
Contributor

@tchebb tchebb commented Apr 21, 2023

This makes it easier for engineers unfamiliar with the project to see where all the crates live and prevents lots of nested target/ and Cargo.lock files from cluttering up the tree.

It's perhaps not as useful as for other projects, since optee-utee still needs to be built via Xargo, which means cargo build from the root can't build everything. But the other benefits still apply.

I had to rename the "systest" crates inside optee-teec and optee-utee so their names don't conflict. I also didn't include examples in the workspace yet, since every example currently has the same three crate names, meaning multiple can't coexist in one workspace. Also, the example Makefiles and tests/ scripts hardcode per-example target/ dirs.

If we did add the examples to the workspace, we could build all of them much faster since they'd share a dependency graph. Doing that is just out of scope of this PR.

There are also a couple of cleanup commits tossed in here for .gitignore and some Cargo.locks, so if you merge please don't squash. Thanks!

@mssun mssun requested a review from DemesneGH April 22, 2023 21:24
@DemesneGH
Copy link
Contributor

Thanks for your proposal. I appreciate the commits 5d0e7d6 and 56210a5, and they seem good to go.

Regarding the "create overarching Cargo workspace" commit:

If we did add the examples to the workspace, we could build all of them much faster since they'd share a dependency graph.

I agree with you. The host and ta both share the proto crate, and they are built with different toolchains. While this organization allows for building individual examples separately, it does indeed require more time to build all examples.

I think adding a Cargo workspace for optee-teec alone would not have a significant impact unless we include the examples in the workspace. However, since the host and TA rely on different toolchains, and Cargo does not support two workspaces in one root folder, we would need to reorganize the examples to separate the host and ta. Considering the shared proto crate, this approach would likely complicate the examples' structure.

So I would suggest merging the first two commits and revisiting the last commit at a later time if necessary. Any ideas about the ways of workspace organization would be appreciated. Thanks!

@tchebb
Copy link
Contributor Author

tchebb commented Apr 23, 2023

I mostly agree with that feedback. I do think having a workspace at the root is nice even if only for discoverability, but I do have a couple ideas to make the examples work. TBD if I'll actually have time to work on those, though. In the meantime, I've split the first two commits into a separate PR, #111.

@DemesneGH
Copy link
Contributor

Sounds good. The other commits have been merged, thanks for your contribution!

@DemesneGH DemesneGH added the enhancement New feature or request label Apr 24, 2023
This makes it easier for engineers unfamiliar with the project to see
where all the crates live and prevents lots of nested target/ and
Cargo.lock files from cluttering up the tree.

It's perhaps not as useful as for other projects, since optee-utee still
needs to be built via Xargo, which means "cargo build" from the root
can't build everything. But the other benefits still apply.

I had to rename the "systest" crates inside optee-teec and optee-utee so
their names don't conflict. I also didn't include examples in the
workspace yet, since every example currently has the same three crate
names, meaning multiple can't coexist in one workspace. Also, the
example Makefiles and tests/ scripts hardcode per-example target/ dirs.

If we did add the examples to the workspace, we could build all of them
much faster since they'd share a dependency graph. Doing that is just
out of scope of this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants