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

Explore moving the primary build system to Cargo #945

Closed
daviddrysdale opened this issue May 4, 2020 · 9 comments
Closed

Explore moving the primary build system to Cargo #945

daviddrysdale opened this issue May 4, 2020 · 9 comments
Assignees
Labels
blocked This pull request is blocked by another pull request or issue P0

Comments

@daviddrysdale
Copy link
Contributor

We currently have Bazel as our primary build system, which builds a C++ main, C++ libraries, and includes Rust libraries that are invoked over FFI.

We're expecting to move main over to Rust under #724, and the obvious initial way to do this is with a rust_binary target in Bazel, keeping the rest of the build system roughly as-is1.

However, we could consider moving the primary build system across to be Cargo. That would still require some way of building the C++ code and its dependencies, presumably still Bazel. So:

  • Some kind of build.rs script would invoke Bazel and ensure that a giant everything-cpp.a thing got linked in a known place.
    • We could look at other crates that pull in native code, e.g. ring, to find common ways to do this.
  • Cargo would need setting up to pull that pile of compiled C++ code in at link time. That might include link-time for individual library crates as well as the final binary, because Rust apparently doesn't normally "underlink" 2.

The following issues should all evaporate if we moved away from Bazel:


1: Aside: it's not that simple.

2: Useful phrase from bazelbuild/rules_rust#78 : "underlinking" seems to mean leaving resolution of undefined symbols until final link of the binary.

@daviddrysdale daviddrysdale self-assigned this May 4, 2020
@daviddrysdale
Copy link
Contributor Author

WDYT @tiziano88 @ipetr0v @anghelcovici (as folk who've fought build systems in the past)

@tiziano88
Copy link
Collaborator

Thanks for writing this down @daviddrysdale .

I think part of the complication here is trying to keep all the examples and the tests working while we switch language underneath them.

IIRC:

  • the gRPC server and client pseudo-node are close to have their pure Rust counterparts (@ipetr0v correct me if I am wrong)
  • the roughtime pseudo-node does not currently have a Rust implementation, but I think we will need a pure Rust roughtime anyways, in order to validate certs or anything else that the Rust Runtime needs to do (not necessarily from nodes); there is https://crates.io/crates/roughenough but I have not looked into it; even if this is not good enough, implementing a roughtime client library does not seem to be a massive amount of work anyways, or we could contribute to that one if anything is missing from there
  • the storage pseudo-node does not serve any practical purpose in its current shape; we could probably re-implement something with equivalent functionality in Rust pretty easily, though I think it would not be particularly useful to drag it around; my opinion is that we should just aim for something usable with an external root of trust, which is tracked in Implement storage support via Cloud Spanner #815 . Even for that, I don't think there would be much that can be reused from the current C++ implementation
  • the other use case we mentioned for which some level of C++ support would be useful would be to drive a TPU or some other native accelerator that does not have proper Rust support; I don't think we have immediate plans for this anyways

All in all, I feel more and more strongly that we are not too far off from a pure cargo-buildable binary with no C++ dependencies; if others think that that is at least plausible, I think we should aim for that and cut down the extra complexity of maintaining the dual language split within the same binary.

cc @project-oak/core

@ipetr0v
Copy link
Contributor

ipetr0v commented May 4, 2020

I agree that we may need to move towards pure Rust build, because currently it looks like cargo raze supports only basic features and is still in the early stages of development. Thus ring may not be the last problematic library we encounter.

Also a random thought: C++ Nodes (such as a TPU wrapper, i.e. Nodes that will be much easier to implement in C++) probably don't have to be a part of the Oak process. Instead they may be implemented as separate services that will use gRPC to communicate with Oak and attest to it.
This might be useful, because in this case we will define an external ABI that then will be used to communicate between different Oak instances.

@conradgrobler
Copy link
Collaborator

Regarding roughenough, I had a look at it previously. It seems to have a fully functional client, but most of the client functionality is built directly into the binary rather than a reusable lib. So we would need to vendor it into third_party to change the client to a library. It should not be too much work though.

@tiziano88
Copy link
Collaborator

Indeed there is not much logic to the roughtime client after all, it's just doing requests and parsing responses, it is only a few dozens lines, it's not even worth vendoring IMO, we should be able to re-implement it ourselves: https://github.com/int08h/roughenough/blob/master/src/bin/roughenough-client.rs .

@tiziano88
Copy link
Collaborator

@ipetr0v : that's right, in fact I think when @rbehjati explored the TPU options, we also discussed that the simplest solution would be to just externalize it as a stand-alone gRPC server.

@tiziano88
Copy link
Collaborator

all considered, my proposal is:

  • finish re-implementing gRPC server and client pseudo-nodes in Rust
  • switch the logic in the Runtime so that the gRPC server pseudo-node is created like other nodes and is no longer "magic"
  • also implement roughtime pseudo-node in Rust
  • delete the storage pseudo-node implementation from C++
    • possibly keep a placeholder implementation in Rust with the same interface as the current one, though I also think the interface itself could use some refactoring, so IMO not worth it
  • switch Rust oak_loader to be built only via cargo
  • switch over to the Rust oak_loader to run tests and examples

@tiziano88
Copy link
Collaborator

@daviddrysdale sorry I realize I missed one of the points you were trying to make in this issue, that is, IIUC, having cargo invoke bazel as part of the build process (perhaps via a build.rs file) to build any remaining C++ code that we have, and then convince cargo to link that in the final binary. I think this would be a nice solution, but I still think that we are not too far off from being able to migrate (at least in principle) all the remaining C++ code to Rust, at which point I personally don't think there would be much value in keeping around the cargo to bazel part of the workflow just in case we come up with some other C++ dependency (which I don't think it's likely in the short-medium term, but also definitely not impossible).

@daviddrysdale daviddrysdale added blocked This pull request is blocked by another pull request or issue P0 labels May 6, 2020
ipetr0v added a commit that referenced this issue May 21, 2020
This change:
- Updates examples to use Rust Loader
- Updates macro and scripts for running Rust Loader
- Makes Oak CI to use Rust Loader for running examples
- Builds Rust Oak Loader with Cargo
- Fixes minor issues with gRPC server/client pseudo-Nodes

Fixes #725
Fixes #874
Fixes #901
Ref #945
@daviddrysdale
Copy link
Contributor Author

Done in #1016.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This pull request is blocked by another pull request or issue P0
Projects
None yet
Development

No branches or pull requests

4 participants