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

Exclude oak_abi and oak_utils from the workspace #1039

Merged
merged 2 commits into from
May 28, 2020

Conversation

rbehjati
Copy link
Contributor

@rbehjati rbehjati commented May 28, 2020

Fixes the problem described in this comment.

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 requested a review from tiziano88 May 28, 2020 09:54
@rbehjati rbehjati requested a review from daviddrysdale May 28, 2020 09:54
@tiziano88
Copy link
Collaborator

I guess this will have to be undone as part of #971 ? The whole point of that is to end up with 2/3 non-overlapping workspaces

@rbehjati
Copy link
Contributor Author

I think this fixes the problem with the build, and the docs. Since oak_abi is in the same folder as Cargo.toml it has to be a member of the workspace. We should be able to make oak_abi a standalone crate after the rest of #971 is completed.

@tiziano88
Copy link
Collaborator

Since oak_abi is in the same folder as Cargo.toml it has to be a member of the workspace.

I thought the error message I posted earlier (#971 (comment)) actually mentions a couple of ways of excluding it from the workspace, doesn't it?

@rbehjati
Copy link
Contributor Author

I thought the error message I posted earlier (#971 (comment)) actually mentions a couple of ways of excluding it from the workspace, doesn't it?

I am using workspace.exclude now. This required excluding oak_utils from the workspace too, to avoid circular dependencies. I tried to make it a module in the oak_abi package, but I am not sure how to make it work, since oak_utils is used in build.rs. Any suggestions?

@tiziano88
Copy link
Collaborator

Do you mean that if moved under oak_abi::utils, then it cannot be used from the build.rs file?

@rbehjati
Copy link
Contributor Author

Do you mean that if moved under oak_abi::utils, then it cannot be used from the build.rs file?

Yes.

@tiziano88
Copy link
Collaborator

If that's the case, it does sound like oak_utils needs to be a top-level crate too, unfortunately.

@rbehjati rbehjati changed the title Make oak_abi a workspace member Exclude oak_abi and oak_utils from the workspace May 28, 2020
@rbehjati
Copy link
Contributor Author

If that's the case, it does sound like oak_utils needs to be a top-level crate too, unfortunately.

Yes. It is unfortunate.
This change does not fix the problem with the docs (#1037). I'll fix that in a different PR.

@rbehjati rbehjati merged commit 0b133c0 into project-oak:master May 28, 2020
@rbehjati rbehjati deleted the abi-fix branch May 28, 2020 15:53
@github-actions
Copy link

Reproducibility Index:

165306babbdaf0d7c8cddfbae0ef95a8bee9090b1664d036a39df69299682b7f  ./target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
a76f8259ff7200f4c459dd9e071914b2d874cd42677fa964f47f2f1367b80f00  ./target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
bc01acbfb3f9ef2fa8ad962f9dd5ace34c37e9afd30b6b6b9ba4e92a5512f7f9  ./target/wasm32-unknown-unknown/release/aggregator.wasm
e2124e9da19d018e783ade27569392627804d781c0e8f44c46dd0dfc43ba48ca  ./target/wasm32-unknown-unknown/release/chat.wasm
b3d1f58ffcb4ccdb10f4d7d2914e24952ae44d5259520ba949167bdc459435b4  ./target/wasm32-unknown-unknown/release/hello_world.wasm
cb2de9fc1d3af6d4d80b2441409a8b6888b892654e8769253de535cceea78135  ./target/wasm32-unknown-unknown/release/machine_learning.wasm
c10517881a33f4b631c31f0e511c1944b946b2ad133dbccee6f1dddba36c7705  ./target/wasm32-unknown-unknown/release/private_set_intersection.wasm
7b1ff17fc70489273da8b55b01dd4cfe496b96c56faefe0fe535e027d27ea7dc  ./target/wasm32-unknown-unknown/release/running_average.wasm
20194a33031ff9e3dcce7b6e2289a50377f1418aee48bd6463f5118640ce6140  ./target/wasm32-unknown-unknown/release/translator.wasm
141f915fb198d9ce7a2c1e97115d9810c9253d3d2b7d34eddaad0fb32b77c9d6  ./target/x86_64-unknown-linux-musl/release/oak_loader

Reproducibility Index diff:

diff --git a/reproducibility_index b/reproducibility_index
index f21af69..039b26b 100644
--- a/reproducibility_index
+++ b/reproducibility_index
@@ -1,10 +1,10 @@
-be42bdb972d9af908c9f0acd6f8c3b2433868ee4a21ddc4e48d1b1544f0d922f  ./target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
-62a7c845f382090c43597ba9acdc200e8c188d99d1b0496baeb4ec3e5fed6c57  ./target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
-7be1f95a8b1bf112cd5234bbc530a9cde16d87121efc6b229bd3d123badbfb8f  ./target/wasm32-unknown-unknown/release/aggregator.wasm
-9427abf7d8b7adb32de93d23f49feb3c0c923660f915f19babb7db8ec438031f  ./target/wasm32-unknown-unknown/release/chat.wasm
-ab7c1b08acda1b5ebc0e0340903e4aad766890a348a35d7d99f7e5df1a4ecfd5  ./target/wasm32-unknown-unknown/release/hello_world.wasm
-90f24086f4dacc3803507bc5724ccc2d26017f3958b4cbac1e02ca5b0a66da5c  ./target/wasm32-unknown-unknown/release/machine_learning.wasm
-57da5ae2bab03a4cef6554939b4b8eeabe179759f94f2e0e0b28b24aed46475d  ./target/wasm32-unknown-unknown/release/private_set_intersection.wasm
-ed408f320722c23336ad47e7b3daa709ebde2cf17f6558a01da5a9b6ee6df78c  ./target/wasm32-unknown-unknown/release/running_average.wasm
-3dcf9e3baba5446283c067c69bc081a6e7fb212d04d2ff774701969e0b738066  ./target/wasm32-unknown-unknown/release/translator.wasm
-9fcfe67e52f589a18f149ffd8b3355dd3943e8431acb2889813a38640ae4ec29  ./target/x86_64-unknown-linux-musl/release/oak_loader
+165306babbdaf0d7c8cddfbae0ef95a8bee9090b1664d036a39df69299682b7f  ./target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
+a76f8259ff7200f4c459dd9e071914b2d874cd42677fa964f47f2f1367b80f00  ./target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
+bc01acbfb3f9ef2fa8ad962f9dd5ace34c37e9afd30b6b6b9ba4e92a5512f7f9  ./target/wasm32-unknown-unknown/release/aggregator.wasm
+e2124e9da19d018e783ade27569392627804d781c0e8f44c46dd0dfc43ba48ca  ./target/wasm32-unknown-unknown/release/chat.wasm
+b3d1f58ffcb4ccdb10f4d7d2914e24952ae44d5259520ba949167bdc459435b4  ./target/wasm32-unknown-unknown/release/hello_world.wasm
+cb2de9fc1d3af6d4d80b2441409a8b6888b892654e8769253de535cceea78135  ./target/wasm32-unknown-unknown/release/machine_learning.wasm
+c10517881a33f4b631c31f0e511c1944b946b2ad133dbccee6f1dddba36c7705  ./target/wasm32-unknown-unknown/release/private_set_intersection.wasm
+7b1ff17fc70489273da8b55b01dd4cfe496b96c56faefe0fe535e027d27ea7dc  ./target/wasm32-unknown-unknown/release/running_average.wasm
+20194a33031ff9e3dcce7b6e2289a50377f1418aee48bd6463f5118640ce6140  ./target/wasm32-unknown-unknown/release/translator.wasm
+141f915fb198d9ce7a2c1e97115d9810c9253d3d2b7d34eddaad0fb32b77c9d6  ./target/x86_64-unknown-linux-musl/release/oak_loader

@rbehjati rbehjati mentioned this pull request Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants