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

Build scripts: Name client variants by their build system for clarity #1170

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

jul-sh
Copy link
Contributor

@jul-sh jul-sh commented Jun 18, 2020

TL;DR

cpp -> bazel
rust -> cargo
nodejs -> npm


Addresses @daviddrysdale's earlier comment: #1169 (comment)

This renames the issue of the somewhat confusing cpp variant.

This makes sense, since bazel is the build system the scripts will use for this variant. And other non cpp clients (Go) that use bazel shouldn't use the cpp flag.

Similarly this renames rust to cargo, and nodejs to npm, so we always refer to the build system.

This is quite similar to the naming used in the rust runner.

enum Target {

Imho it might be worth to think about doing the same for the server/application variants.

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.

Comment on lines 126 to 127
bazel)
cpp_client="${SCRIPTS_DIR}/../examples/${EXAMPLE}/client/BUILD"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also means we now explicitly check for the presence of a BUILD file.

@jul-sh jul-sh force-pushed the bash_scripts_naming branch from e8619e1 to 7193d30 Compare June 18, 2020 15:20
@jul-sh jul-sh changed the title Build scripts: Name variants by their build system for clarity Build scripts: Name client variants by their build system for clarity Jun 18, 2020
@jul-sh jul-sh force-pushed the bash_scripts_naming branch 3 times, most recently from 41e2a13 to 7230c33 Compare June 18, 2020 16:35
@jul-sh jul-sh marked this pull request as ready for review June 18, 2020 17:32
@jul-sh jul-sh requested a review from daviddrysdale June 18, 2020 17:32
@jul-sh jul-sh force-pushed the bash_scripts_naming branch from 7230c33 to 9a8de65 Compare June 19, 2020 10:12
@jul-sh jul-sh merged commit 16cbf88 into project-oak:main Jun 19, 2020
@github-actions
Copy link

Reproducibility Index:

c7e447de8c9dc9456f7a8ba83959474a39970eb4967998fe0b908f92f0a4d89b  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
7197a5c68eaf0d2d5ae226ea1cb8dbf1a5a3b499c8a205e5d7a0537faa7dcd1f  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
20c9b09fd04da0a06a69f6343eaf0f3afd7bbf97304e3a08464e13f185feb969  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
0bdbe9ec58af36a1c50311a598bdc0bb09327ba4ea9aa4abd0d5c4ce6426bf02  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
c0f0cfc1d370b0adfe7907d79f8afa0548f2f77dc7ef8aa5a17be3405f30cd76  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
97f2f2eff51281290bd836ed86cffc8999718659431d0f5f8468d1afcaa24cdf  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
a3e0afddba46224ca81f1a43a163e9f0c47fe106bd5f670ead206610939db449  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
2ed84ec712fbfc733e204a6ed1af7651f7b9b1bc241878adb17c6ea301a233d1  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
f42dae0e0c31c6dea9766ee2441327d95de2242990d024d68e99b431a414c562  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
f2f4a2ced4d1af2c4fde34361742d3bceae636c6bfcc468af92e9b4be5427ad7  ./examples/target/wasm32-unknown-unknown/release/trusted_information_retrieval.wasm
75bb864495cbb7278f8fdc8d25f050323d9dc281f9528f3f789835def25e4e7f  ./oak/server/target/x86_64-unknown-linux-musl/release/oak_loader

Reproducibility Index diff:

@jul-sh jul-sh deleted the bash_scripts_naming branch July 16, 2020 14:07
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