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

Make examples a separate workspace #1045

Merged
merged 2 commits into from
Jun 1, 2020

Conversation

rbehjati
Copy link
Contributor

@rbehjati rbehjati commented May 29, 2020

Ref #971 and #765

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have written tests that cover the code changes.
    • I have checked that these tests are run by Cloudbuild
    • I have updated documentation accordingly.
    • I have raised an issue to
      cover any TODOs and/or unfinished work.
  • Pull request includes prototype/experimental work that is under
    construction.

@rbehjati rbehjati added the WIP Work in progress label May 29, 2020
@rbehjati rbehjati force-pushed the oak-971-examples branch 7 times, most recently from 7a516af to be95e67 Compare June 1, 2020 14:25
@@ -5,6 +5,7 @@ readonly SCRIPTS_DIR="$(dirname "$(readlink -f "$0")")"
# shellcheck source=scripts/common
source "${GLOBAL_SCRIPTS_DIR}/common"

cd "examples"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it needed here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't cd in the middle of scripts, unless necessary, and even so, please use a subshell instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently it is not needed, but the script does not quite work even on master.

Comment on lines 28 to 48
oak = { path = "../sdk/rust/oak" }
oak_abi = { path = "../oak_abi" }
oak_runtime = { path = "../oak/server/rust/oak_runtime" }
oak_tests = { path = "../sdk/rust/oak_tests" }
oak_utils = { path = "../oak_utils" }
# Examples.
abitest_common = { path = "abitest/abitest_common" }
abitest_0_frontend = { path = "abitest/module_0/rust" }
abitest_1_backend = { path = "abitest/module_1/rust" }
abitest_tests = { path = "abitest/tests" }
aggregator = { path = "aggregator/module/rust" }
aggregator_backend = { path = "aggregator/backend" }
aggregator_common = { path = "aggregator/common" }
chat = { path = "chat/module/rust" }
hello_world = { path = "hello_world/module/rust" }
machine_learning = { path = "machine_learning/module/rust" }
private_set_intersection = { path = "private_set_intersection/module/rust" }
running_average = { path = "running_average/module/rust" }
rustfmt = { path = "rustfmt/module/rust" }
translator = { path = "translator/module/rust" }
translator_common = { path = "translator/common" }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are all these patches needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess Cargo needs these to find the dependencies. Previously, they were in the top-level Cargo.toml.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware they were there, but I don't think they are actually relevant any more. I think they were just a convenience so that we could use --package=translator, but now that they are in a separate workspace we probably need to specify the entire manifest path anyways, so I don't think they buy us anything. Or it would be good to know if they are actually used for anything else :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! I removed them except from translator_common = { path = "translator/common" }. That one is still needed, since it is a dependency for translator and hello-world. Specifying the manifest-path does not seem to bring translator_common into scope.

@@ -5,6 +5,7 @@ readonly SCRIPTS_DIR="$(dirname "$(readlink -f "$0")")"
# shellcheck source=scripts/common
source "${GLOBAL_SCRIPTS_DIR}/common"

cd "examples"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't cd in the middle of scripts, unless necessary, and even so, please use a subshell instead.

* Added a top-level `Cargo.toml` file in the `examples` directory to
  make the examples a separate workspace.
* Reduced parallelism in `cloudbuild` to avoid conflicting build steps.
@rbehjati rbehjati merged commit a8a9902 into project-oak:master Jun 1, 2020
@rbehjati rbehjati deleted the oak-971-examples branch June 1, 2020 18:08
@github-actions
Copy link

github-actions bot commented Jun 1, 2020

Reproducibility Index:

95ec42fa4c0007805d047b07d942c860121c83a2e2e97390c0e3ba1140bcfef7  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
46353864578416c3a47692dcb9e1b508ab803e7f88138d333f351499e0674849  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
61b479957a2ed92c7ecb10feee982839f941fd6cf4d1a8c585d9362c2b5aab53  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
6ec5830ce3beedb20788488a984b6e260f4eb5069a7b1dd3d92791f04123d527  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
479f7e7cf1b157da5d9098878bbe3e1a8c7615e46288f6ad02b0526e04533b50  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
8d71b9c3587c5b73ab80d5f14bac6aae5fc4b36b990971a0482fc549efb8a964  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
c9f51da5c57023acd4cb2f62619b4b25843ac61fccecd1ea7bd47de58cd90a72  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
51dd042a18dc51a767c54db46c49ad7e4a7af0e042c41f43f43d76a5171abe6f  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
ce6ce387935e09a55681f48516215af08bc344b500ad6342e3280cabc7b5da89  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
a4c456e4da50d72d91ed55d7cc2e689af8cd6104173bb478bcb48643c8e12b31  ./target/x86_64-unknown-linux-musl/release/oak_loader

Reproducibility Index diff:

diff --git a/reproducibility_index b/reproducibility_index
index 76ac4ff..cd0b5d4 100644
--- a/reproducibility_index
+++ b/reproducibility_index
@@ -1,10 +1,10 @@
-4bfb94cd7dd5b2809f0afdebfb5e77c5fbffbbcab5ea8dd7d257e202e80518b4  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
-a76f8259ff7200f4c459dd9e071914b2d874cd42677fa964f47f2f1367b80f00  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
-93c63ff47b47ead838ca5144547b29463d01d699759b44cc599a3cf70adb9f53  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
-5c468b6295f481af8b4b5758e3c67ec5523ecbac99f72a88cf9d43a4a7747274  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
-dda50d2185ae1e9c7e9c5cded711823b6952d277ac8f38290eee4093763a67de  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
-cb2de9fc1d3af6d4d80b2441409a8b6888b892654e8769253de535cceea78135  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
-ffacb1b30bb721d235df5fb648017c8dfaacbb7e318993d16f14893b07293845  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
-7ceb8a30ec9bafb671ba15f886a684ec0776c1e00c3bf3ba269a627e938feba3  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
-47b81f9c85ad5dfeacb389a6a4a1beb0baaeac828ac002ccd0b04dba8fa07c45  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
-905dd5854a9d2634f93186ae65e28fb85f14d16b65f74d345631f71f9b2d2048  ./target/x86_64-unknown-linux-musl/release/oak_loader
+95ec42fa4c0007805d047b07d942c860121c83a2e2e97390c0e3ba1140bcfef7  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
+46353864578416c3a47692dcb9e1b508ab803e7f88138d333f351499e0674849  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
+61b479957a2ed92c7ecb10feee982839f941fd6cf4d1a8c585d9362c2b5aab53  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
+6ec5830ce3beedb20788488a984b6e260f4eb5069a7b1dd3d92791f04123d527  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
+479f7e7cf1b157da5d9098878bbe3e1a8c7615e46288f6ad02b0526e04533b50  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
+8d71b9c3587c5b73ab80d5f14bac6aae5fc4b36b990971a0482fc549efb8a964  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
+c9f51da5c57023acd4cb2f62619b4b25843ac61fccecd1ea7bd47de58cd90a72  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
+51dd042a18dc51a767c54db46c49ad7e4a7af0e042c41f43f43d76a5171abe6f  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
+ce6ce387935e09a55681f48516215af08bc344b500ad6342e3280cabc7b5da89  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
+a4c456e4da50d72d91ed55d7cc2e689af8cd6104173bb478bcb48643c8e12b31  ./target/x86_64-unknown-linux-musl/release/oak_loader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants