search

apache / incubator-teaclave-trustzone-sdk

Teaclave TrustZone SDK enables safe, functional, and ergonomic development of trustlets.
https://teaclave.apache.org
Apache License 2.0
221 stars 62 forks source link

Create overarching Cargo workspace #110

Open tchebb opened 1 year ago

tchebb commented 1 year ago

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!

DemesneGH commented 1 year ago

Thanks for your proposal. I appreciate the commits https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/110/commits/5d0e7d62e9f0f8f56d4807d557d61356f1e319ee and https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/110/commits/56210a5e24326ce2fd65f862ed2b258a4db7cf0c, 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 commented 1 year ago

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 commented 1 year ago

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