Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH closed pull request #245: experimental: add cargo-optee URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3673407205 Closing this draft PR and continuing the work in https://github.com/apache/teaclave-trustzone-sdk/pull/263 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3587748877 > could you clarify whether we should use host or ca in this project? @asir66 Let's use `host` which also aligns with optee C examples https://github.com/linaro-swg/optee_examples/tree/master/acipher -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
asir66 commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3585624878 > Hi @asir66 thanks for the effort. Please take my previous comment ([#245 (comment)](https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2473250351) compare the current `cargo optee` subcommand with existing `cargo` conventions. If possible, could you help list all the areas where the current implementation violates or diverges from standard cargo behavior? Once we have this list, we can decide whether to fix them within this PR or address them in a series of follow-up PRs before publishing to crates.io. > > A few highlights from my earlier comment for reference: > > * Project configuration should stay in each project’s Cargo.toml. Toolchain configuration (e.g., ta-dev-kit-dir) should be managed by a dedicated subcommand such as `cargo optee setup`, instead of being mixed into project config. > * std / no_std selection should be inferred automatically from the active feature set, consistent with how existing cargo workflows behave. > > Thanks in advance. I’ll do my best to review these areas and put together a comprehensive list of the differences from standard cargo conventions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
asir66 commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3585612877 > @asir66 Thanks for your detailed and careful testing! > > > I encountered an issue related to path resolution in `cargo-optee`. The behavior is inconsistent between `mnist-rs` and other examples such as `hello_world-rs` and `acipher-rs`. Since the root cause is not yet clear to me, I would like to ask for guidance or insights from the community. > > The `mnist-rs` have the workspace in `mnist/ta/Cargo.toml`, could you help to check if the issue related with this? > > > I would like to ask whether the project would benefit from having a Makefile under the examples/ directory to manage or build all examples together. Is this something we might need? Like [examples/Makefile](https://github.com/apache/teaclave-trustzone-sdk/blob/23a041fd658c99f542e3ba83331784fbe8453819/examples/Makefile) > > The `ci/build.sh` performs as what original Makefile does, it reads the `examples/metadata.json` to get all app path of examples and build. Please also help to check if it enough for our use case? > > For other recommendations: > > 1. I'd prefer: >Introduce explicit CLI flags: >--std >--no-std > > 3 and 4: LGTM. Please help to fix the 1. 3. 4. thanks. It’s my pleasure — I’d be glad to work on that. By the way, could you clarify whether we should use host or ca in this project? > 5. The cargo-optee tool isn’t tied to the dev Docker image, so we can keep the current dev image unchanged and consider optimizing it later. >However, if we merge this PR, our documentation and CI will need a full update. >How about we add cargo-optee in this PR while keeping both the old Makefile system and the new tool? After this merged, we can update the docs and CI step by step, and remove the Makefiles once the new tool is stable and widely used. What do you think? LGTM! I think this approach is great — it allows users to transition smoothly from the old Makefile workflow to the new tool. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3585190298
> # cargo-optee Testing Report and Findings
> Below is a consolidated report of issues, observations, and suggestions
identified during testing of `cargo-optee` in PR #245. I include reproducible
examples, analysis, and potential directions for improvement.
>
> ## 1. Issue: `std` Flag Logic Bug
> ### Description
> The CLI parameter `--std` is parsed as a bool, and later converted into
`Option` (`cmd_std`). However, the current logic contains an unintended
behavior:
>
> * When the user does not pass `--std`, the default value `std = false`
becomes `Some(false)`
> * As a result, `cmd_std` never becomes `None`
> * Therefore, `BuildConfig::resolve()` never falls back to the
configuration defined in `Cargo.toml`
>
> This causes the `std` parameter to never fall back to the configuration
provided in `Cargo.toml`.
>
> ### Quick Fix (tested and working)
> Changing the following argument inside `BuildConfig::resolve()`:
>
> ```rust
> # cargo-optee/main.rs:79
> if std { Some(true) } else { None }
> ```
>
> This restores correct fallback behavior, but it is only a temporary
workaround.
>
> ### Suggested Solutions
> 1. Introduce explicit CLI flags:
>`--std`
>`--no-std`
> 2. Or, treat `std`/`no-std` as features selection:
>`--features std`
>`--features no-std`
>
> Feedback from maintainers would be appreciated; I am willing to contribute
a proper solution.
>
> ## 2. Issue: `--manifest-path` Relative Path Resolution in mnist-rs
> ### Description
> I encountered an issue related to path resolution in `cargo-optee`. The
behavior is inconsistent between `mnist-rs` and other examples such as
`hello_world-rs` and `acipher-rs`. Since the root cause is not yet clear to me,
I would like to ask for guidance or insights from the community.
>
> Below are the reproducible cases.
>
> 1. Case 1 — Running inside the Cargo.toml directory (`hello_world-rs`)
>
> `Cargo.toml` is
>
> ```toml
> ...
> [package.metadata.optee.ta]
> arch = "arm"
> std = true
> uuid-path = "../uuid.txt"
> ta-dev-kit-dir = { aarch64 =
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64", arm =
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm32" }
> ```
>
> In `hello_world-rs/ta/`, running with:
>
> ```
> root@05d55135aefe:~/teaclave_sdk_src/examples/hello_world-rs/ta#
cargo-optee build ta --manifest-path ./Cargo.toml
> Building TA with:
> Arch: Arm
> Debug: false
> Std: false
> TA dev kit dir:
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm32"
> UUID path: "/root/teaclave_sdk_src/examples/hello_world-rs/uuid.txt"
> Building TA in directory: /root/teaclave_sdk_src/examples/hello_world-rs/ta
> Running cargo fmt and clippy...
> ^C
> root@05d55135aefe:~/teaclave_sdk_src/examples/hello_world-rs/ta#
cargo-optee build ta --manifest-path Cargo.toml
> Building TA with:
> Arch: Arm
> Debug: false
> Std: false
> TA dev kit dir:
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm32"
> UUID path: "/root/teaclave_sdk_src/examples/hello_world-rs/uuid.txt"
> Building TA in directory:
> Running cargo fmt and clippy...
> Error: No such file or directory (os error 2)
> ```
>
> The metadata in `Cargo.toml` is parsed correctly (TA dev kit dir, UUID
path are correct). But there is Error with `No such file or directory (os error
2)`
>
> 2. There is diffierent between `mnist-rs` and `hello_world-rs`.
>
> When `ta-dev-kit-dir` is correctly configured in the corresponding
`Cargo.toml`, running the following command under `mnist-rs` produces the
expected output:
>
> ```
> root@05d55135aefe:~/teaclave_sdk_src/examples# cargo-optee build ta \
> > --manifest-path mnist-rs/ta/train/Cargo.toml
> Building TA with:
> Arch: Aarch64
> Debug: false
> Std: false
> TA dev kit dir:
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64"
> UUID path: "/root/teaclave_sdk_src/examples/mnist-rs/ta/train/uuid.txt"
> Building TA in directory: /root/teaclave_sdk_src/examples/mnist-rs/ta/train
> Running cargo fmt and clippy...
> ^C
> root@05d55135aefe:~/teaclave_sdk_src/examples# cargo-optee build ta
--manifest-path ./mnist-rs/ta/train/Cargo.toml
> Building TA with:
> Arch: Aarch64
> Debug: false
> Std: false
> UUID path: "./mnist-rs/ta/train/../uuid.txt"
> Error: ta-dev-kit-dir is MANDATORY but not configured.
> Please set it via:
> 1. Command line: --ta-dev-kit-dir
> 4. Cargo.toml metadata: [package.metadata.optee.ta] section
>
> Example Cargo.toml:
> [package.metadata.optee.t
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3585064704 Thanks for your detailed and careful testing! > I encountered an issue related to path resolution in `cargo-optee`. The behavior is inconsistent between `mnist-rs` and other examples such as `hello_world-rs` and `acipher-rs`. Since the root cause is not yet clear to me, I would like to ask for guidance or insights from the community. The `mnist-rs` have the workspace in `mnist/ta/Cargo.toml`, could you help to check if the issue related with this? > I would like to ask whether the project would benefit from having a Makefile under the examples/ directory to manage or build all examples together. Is this something we might need? Like [examples/Makefile](https://github.com/apache/teaclave-trustzone-sdk/blob/23a041fd658c99f542e3ba83331784fbe8453819/examples/Makefile) The `ci/build.sh` performs as what original Makefile does, it reads the `examples/metadata.json` to get all app path of examples and build. Please also help to check if it enough for our use case? --- For other recommendations: 1. I'd prefer: Introduce explicit CLI flags: --std --no-std 3 and 4: LGTM. Please help to fix the 1. 3. 4. thanks. 5. The cargo-optee tool isn’t tied to the dev Docker image, so we can keep the current dev image unchanged and consider optimizing it later. However, if we merge this PR, our documentation and CI will need a full update. How about we add cargo-optee in this PR while keeping both the old Makefile system and the new tool? After this merged, we can update the docs and CI step by step, and remove the Makefiles once the new tool is stable and widely used. What do you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
asir66 commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2567841622
##
cargo-optee/src/config.rs:
##
@@ -0,0 +1,637 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use anyhow::{bail, Result};
+use cargo_metadata::MetadataCommand;
+use serde_json::Value;
+use std::path::{Path, PathBuf};
+
+use crate::common::Arch;
+
+/// Build configuration that can be discovered from proto metadata
+#[derive(Debug, Clone)]
+pub struct BuildConfig {
+pub arch: Arch,
+pub debug: bool,
+pub std: bool,
Review Comment:
There is an issue here: when --std is not provided, std defaults to false,
so cmd_std becomes Some(false) instead of None.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
asir66 commented on PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3584945539
# cargo-optee Testing Report and Findings
Below is a consolidated report of issues, observations, and suggestions
identified during testing of `cargo-optee` in PR #245. I include reproducible
examples, analysis, and potential directions for improvement.
## 1. Issue: `std` Flag Logic Bug
### Description
The CLI parameter `--std` is parsed as a bool, and later converted into
`Option` (`cmd_std`). However, the current logic contains an unintended
behavior:
- When the user does not pass `--std`, the default value `std = false`
becomes `Some(false)`
- As a result, `cmd_std` never becomes `None`
- Therefore, `BuildConfig::resolve()` never falls back to the configuration
defined in `Cargo.toml`
This causes the `std` parameter to never fall back to the configuration
provided in `Cargo.toml`.
### Quick Fix (tested and working)
Changing the following argument inside `BuildConfig::resolve()`:
```rust
# cargo-optee/main.rs:79
if std { Some(true) } else { None }
```
This restores correct fallback behavior, but it is only a temporary
workaround.
### Suggested Solutions
1. Introduce explicit CLI flags:
`--std`
`--no-std`
2. Or, treat `std`/`no-std` as features selection:
`--features std`
`--features no-std`
Feedback from maintainers would be appreciated; I am willing to contribute a
proper solution.
## 2. Issue: `--manifest-path` Relative Path Resolution in mnist-rs
### Description
I encountered an issue related to path resolution in `cargo-optee`. The
behavior is inconsistent between `mnist-rs` and other examples such as
`hello_world-rs` and `acipher-rs`. Since the root cause is not yet clear to me,
I would like to ask for guidance or insights from the community.
Below are the reproducible cases.
1. Case 1 — Running inside the Cargo.toml directory (`hello_world-rs`)
`Cargo.toml` is
```toml
...
[package.metadata.optee.ta]
arch = "arm"
std = true
uuid-path = "../uuid.txt"
ta-dev-kit-dir = { aarch64 =
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64", arm =
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm32" }
```
In `hello_world-rs/ta/`, running with:
```
root@05d55135aefe:~/teaclave_sdk_src/examples/hello_world-rs/ta# cargo-optee
build ta --manifest-path ./Cargo.toml
Building TA with:
Arch: Arm
Debug: false
Std: false
TA dev kit dir:
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm32"
UUID path: "/root/teaclave_sdk_src/examples/hello_world-rs/uuid.txt"
Building TA in directory: /root/teaclave_sdk_src/examples/hello_world-rs/ta
Running cargo fmt and clippy...
^C
root@05d55135aefe:~/teaclave_sdk_src/examples/hello_world-rs/ta# cargo-optee
build ta --manifest-path Cargo.toml
Building TA with:
Arch: Arm
Debug: false
Std: false
TA dev kit dir:
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm32"
UUID path: "/root/teaclave_sdk_src/examples/hello_world-rs/uuid.txt"
Building TA in directory:
Running cargo fmt and clippy...
Error: No such file or directory (os error 2)
```
The metadata in `Cargo.toml` is parsed correctly (TA dev kit dir, UUID path
are correct). But there is Error with `No such file or directory (os error 2)`
2. There is diffierent between `mnist-rs` and `hello_world-rs`.
When `ta-dev-kit-dir` is correctly configured in the corresponding
`Cargo.toml`, running the following command under `mnist-rs` produces the
expected output:
```
root@05d55135aefe:~/teaclave_sdk_src/examples# cargo-optee build ta \
> --manifest-path mnist-rs/ta/train/Cargo.toml
Building TA with:
Arch: Aarch64
Debug: false
Std: false
TA dev kit dir:
"/opt/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64"
UUID path: "/root/teaclave_sdk_src/examples/mnist-rs/ta/train/uuid.txt"
Building TA in directory: /root/teaclave_sdk_src/examples/mnist-rs/ta/train
Running cargo fmt and clippy...
^C
root@05d55135aefe:~/teaclave_sdk_src/examples# cargo-optee build ta
--manifest-path ./mnist-rs/ta/train/Cargo.toml
Building TA with:
Arch: Aarch64
Debug: false
Std: false
UUID path: "./mnist-rs/ta/train/../uuid.txt"
Error: ta-dev-kit-dir is MANDATORY but not configured.
Please set it via:
1. Command line: --ta-dev-kit-dir
4. Cargo.toml metadata: [package.metadata.optee.ta] section
Example Cargo.toml:
[package.metadata.optee.ta]
ta-dev-kit-dir = { aarch64 =
"/path/to/optee_os/out/arm-plat-vexpress/export-ta_arm64" }
# arm architecture omitted (defaults to null)
For help with available options, run: cargo-optee buil
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
asir66 commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3561241080 > @asir66 Could you help to try out the tool and share your feedback? Thanks! It's my pleasure. I'll test it and share my feedback. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3561177298 @asir66 Could you help to try out the tool and share your feedback? Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3561174592 Updates: The build and runtime tests now pass in the OP-TEE repo. The `OP-TEE/build` project requires the changes in this commit: https://github.com/DemesneGH/build/commit/806fafdeedde8a9c9d6eed7bd0aa058e65fb164f This confirms that the tool can be integrated into OP-TEE. I will open PRs in the OP-TEE repositories after our tool is merged. Todos: * CI: add tests on ARM host * Documentation * Code review for inner logic * Functional testing and bug fixes Pending: *std: cargo-optee sets up the std environments and remove the Xargo.toml files from each example. This is not urgent and can likely be addressed in the next stage. -- Planned to merge the initial version within the next two weeks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3491735288 The new commit: 1. Read from Cargo.toml metadata: - Enable reading build parameters from `[package.metadata.optee.*]` sections - Implement priority-based configuration: CLI arguments > metadata > default value - Update `supp_plugin-rs/*/Cargo.toml` files for demonstration - See updated `cargo-optee/README.md` for detailed usage instructions 2. Custom build env var and features: - Allow users to specify custom environment variables and features - These options are automatically appended to `cargo build` commands The CI build script (`ci/build.sh`) passes successfully for the default aarch64 no-std configuration. How to use this tool: 1. clone the code; 2. build the cargo-optee: ``` (cd cargo-optee && cargo build --release) # Optional: add to PATH for easier usage export PATH=/path/to/cargo-optee/target/release:$PATH ``` 4. run `cargo-optee --help` to show the help messages 5. build the app: e.g. ``` # with metadata the app can be built without params cd examples/supp_plugin-rs/plugin && cargo-optee build plugin # without metadata, (if you don't want to change the directory, you can specify `--manifest-path` like the original cargo) cargo-optee build ta --manifest-path /teaclave/examples/hello_world-rs/ta/Cargo.toml --ta-dev-kit-dir /teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64 ``` At this stage, we’ll focus on user-facing functionality and bug fixes. Please feel free to try out the tool and share your feedback, thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2477590499
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
`cargo_metadata` retrieves meta info based on `cargo.toml` and command-line
options in a programatic way, which does not mean convert to `cargo build
--features std`. It is designated to used from within a cargo-* executable.
https://crates.io/crates/cargo_metadata
Here is a more concrete example to illustrate the difference.
- `cargo-optee build ta --std` means no matter what default feature
developer defines in the Cargo.toml, the customized toolchain will build in
using std library (new convention is introduced by cargo-optee).
- `cargo optee build` (with other cargo compatible flags) means the the
customized toolchain will respect what feature the developer enables in the
Cargo.toml (std/no-std) and automatically handles internally (select xargo to
build stdlib or not) based on the active feature.
`Any reasons on why we should follow seamless interface principle?`
There’s no strict requirement to maintain full consistency with Cargo’s
original command-line parameters. However, I am just wondering if our goal
should be to make OP-TEE development feel like a natural extension of the
standard Cargo experience — a design I believe is achievable for our current
use cases. To realize this, we can learn from existing customized toolchains,
e.g. both tools you mentioned also leverage `cargo_metadata` to retrieve
project setup information.
I will end this conversation here, as I believe I’ve explained my points
clearly and don’t want to spend more time on further debate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2477422262
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
I think `cargo-optee` provides the convenience for building TAs, not
mandatory to keep consistency with the original cargo cmd parameters.
For example, the cargo-optee cmd:
```
cargo-optee build ta \
--ta_dev_kit_dir /opt/optee/export-ta_arm64 \
--path ./ta \
--arch aarch64
```
will be converted to:
```
TA_DEV_KIT_DIR=/opt/optee/export-ta_arm64 \
RUSTFLAGS="-C panic=abort" \
cargo build --target aarch64-unknown-linux-gnu --release \
--config target.aarch64-unknown-linux-gnu.linker="aarch64-linux-gnu-gcc"
```
for build.
`--arch` is not consistent with cargo's cmdline but it will configure the
target for developer's convenience. Do you mean we should use the `--target` in
`cargo-optee`?
If so, why not directly use cargo command, we don't need a wrapper.
`feature std` is similar. It's not only converted to the `cargo build
--features std`, but also decide the `target` and whether we use `xargo`. I can
follow the cargo's cmd format, like `cargo-optee build ta --features std`, but
it doesn't mean the same as in cargo.
> From my perspective, **the cargo-optee build command should provide a
seamless interface aligned with the existing cargo build workflow**,
I've checked other tools `cargo-sgx` `cargo-near` which also defines their
cmd lines different with original cargo. Any reasons on why we should follow
`seamless interface` principle?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2477152119
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
Before we move further, could you clarify what kind of user experience you
envision for TA developers using the cargo-optee toolchain?
From my perspective, **the cargo-optee build command should provide a
seamless interface aligned with the existing cargo build workflow**, either
through transparent parameter passing or through complementary extensions. The
key principle is to stay consistent with Cargo’s CLI conventions and user
expectations.
It’s perfectly fine for the internal implementation to have a different
interface, but here I’m mainly referring to the **user-facing design**.
Here I just expand one point for illustration.
Q: “But what should we write in the common examples’ Cargo.toml? If we set
no-std, how would we build it in std mode for CI? By modifying Cargo.toml each
time? I don't have the answer.”
My view: when designing the interface, we should ask — how do Rust
developers normally address this problem (e.g., switching between no_std and
std, or specifying architectures)? We should adopt the same mechanism.
For std / no_std switching, Rust developers use Cargo features declared in
Cargo.toml, together with `--no-default-features` or `--features xxx` or
nothing for `cargo build`. This convention can also be applied for our
customized tooling.
The underlying build logic can follow Cargo’s native feature resolution
model (via `cargo_metadata`), for example:
```
cargo (optee) build
cargo (optee) build --no-default-features
cargo (optee) build --no-default-features --features no_std // or std
```
The key concept here is transparent delegation to Cargo, cargo-optee does
not need to reimplement or reinterpret feature logic, or use a reinvented
`--std` parameter, but instead query and respect Cargo’s native feature
resolution. This ensures consistent developer experience and predictable build
behavior.
A similar comparison and concept mapping approach to existing Cargo
mechanisms can be applied to other aspects as well, such as toolchain
convention
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2477152119
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
Before we move further, could you clarify what kind of user experience you
envision for TA developers using the cargo-optee toolchain?
From my perspective, **the cargo-optee build command should provide a
seamless interface aligned with the existing cargo build workflow**, either
through transparent parameter passing or through complementary extensions. The
key principle is to stay consistent with Cargo’s CLI conventions and user
expectations.
It’s perfectly fine for the internal implementation to have a different
interface, but here I’m mainly referring to the **user-facing design**.
Here I just expand one point for illustration.
Q: “But what should we write in the common examples’ Cargo.toml? If we set
no-std, how would we build it in std mode for CI? By modifying Cargo.toml each
time? I don't have the answer.”
My view: when designing the interface, we should ask — how do Rust
developers normally address this problem (e.g., switching between no_std and
std, or specifying architectures)? We should adopt the same mechanism.
For std / no_std switching, Rust developers use Cargo features declared in
Cargo.toml, together with `--no-default-features or --features xxx. This
convention can also be applied for our customized tooling.
The underlying build logic can follow Cargo’s native feature resolution
model (via `cargo_metadata`), for example:
```
cargo (optee) build
cargo (optee) build --no-default-features
cargo (optee) build --no-default-features --features no_std // or std
```
The key concept here is transparent delegation to Cargo, cargo-optee does
not need to reimplement or reinterpret feature logic, or use a reinvented
`--std` parameter, but instead query and respect Cargo’s native feature
resolution. This ensures consistent developer experience and predictable build
behavior.
A similar comparison and concept mapping approach to existing Cargo
mechanisms can be applied to other aspects as well, such as toolchain
conventions and project conventions, to ach
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3466775215 Refer to the OP-TEE issue: https://github.com/OP-TEE/build/issues/849 Adding `install` and `clean` subcmds helps us to cleanup the large building artifacts which save the disk space, it's useful for CI and production build. This also mentioned in `cargo-optee/README.md` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476331683
##
optee-utee-build/src/linker.rs:
##
@@ -126,10 +126,11 @@ impl Linker {
out_dir: PathBuf,
ta_dev_kit_dir: PathBuf,
) -> Result<(), Error> {
-const ENV_TARGET_TA: &str = "TARGET_TA";
-println!("cargo:rerun-if-env-changed={}", ENV_TARGET_TA);
+// cargo passes TARGET as env to the build scripts
+const ENV_TARGET: &str = "TARGET";
+println!("cargo:rerun-if-env-changed={}", ENV_TARGET);
let mut aarch64_flag = true;
-match env::var(ENV_TARGET_TA) {
+match env::var(ENV_TARGET) {
Ok(ref v) if v == "arm-unknown-linux-gnueabihf" || v ==
"arm-unknown-optee" => {
Review Comment:
tracked in https://github.com/apache/teaclave-trustzone-sdk/issues/250
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476294732
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
I've reviewed the `cargo-sgx` code snippet — although it reads metadata, it
still accepts parameters via command line with default values. I assume you're
suggesting we should move some params to `[package.metadata]` in `Cargo.toml`
instead of passing them as command-line arguments?
I actually mentioned the idea in the initial message of this PR, but there
are a few considerations. Let's go through each:
`arch` / `std`
We should allow building the same app for different arch and modes.
If these values are hardcoded in `Cargo.toml`, we lose the flexibility. For
example, in CI:
> `cargo-trustzone` should automatically infer whether to build with `std`
based on the feature declared in `Cargo.toml`.
But what should we write in the common examples’ `Cargo.toml`?
If we set `"no-std"`, how would we build it in `std` mode for CI? by
modifying `Cargo.toml` each time? I don't have the answer.
Therefore, I believe **`arch`** and **`std`** should remain as
**command-line parameters**, each with default values.
`ta_dev_kit_dir`
Seems this might suitable for `Cargo.toml`, but it’s different for `arch` —
for example:
```
optee_os/out/arm-plat-vexpress/export-ta_arm64
optee_os/out/arm-plat-vexpress/export-ta_arm32
```
If we put it in `Cargo.toml`, we’d have to edit that file each time we
change the build architecture.
Maybe try this?
```
[metadata]
ta_dev_kit.64 = "_export-ta_arm64"
ta_dev_kit.32 = "_export-ta_arm32"
```
use the different entry decided by cmd param `arch`.
`uuid_path` (or `uuid`)
Instead of storing a `uuid_path`, it may make more sense to store a `uuid`
string directly in the metadata. Since `uuid.txt` is both used by TA build
tools and read by the proto (and imported by the CA when invoking the TA). We
could try to write into the Cargo.toml, but to which Cargo.toml? Maybe we
should have a workspace?
I’m still thinking about it.
signing_key
It makes sense to write i
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476294732
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
I've reviewed the `cargo-sgx` code snippet — although it reads metadata, it
still accepts parameters via command line with default values. I assume you're
suggesting we should move some params to `[package.metadata]` in `Cargo.toml`
instead of passing them as command-line arguments?
I actually mentioned the idea in the initial message of this PR, but there
are a few considerations. Let's go through each:
`arch` / `std`
We should allow building the same app for different arch and modes.
If these values are hardcoded in `Cargo.toml`, we lose the flexibility. For
example, in CI:
> `cargo-trustzone` should automatically infer whether to build with `std`
based on the feature declared in `Cargo.toml`.
But what should we write in the common examples’ `Cargo.toml`?
If we set `"no-std"`, how would we build it in `std` mode for CI? by
modifying `Cargo.toml` each time? I don't have the answer.
Therefore, I believe **`arch`** and **`std`** should remain as
**command-line parameters**, each with default values.
`ta_dev_kit_dir`
Seems this might suitable for `Cargo.toml`, but it’s different for `arch` —
for example:
```
optee_os/out/arm-plat-vexpress/export-ta_arm64
optee_os/out/arm-plat-vexpress/export-ta_arm32
```
If we put it in `Cargo.toml`, we’d have to edit that file each time we
change the build architecture.
Maybe try this?
```
[metadata]
ta_dev_kit.64 = "_export-ta_arm64"
ta_dev_kit.32 = "_export-ta_arm32"
```
use the different entry decided by cmd param `arch`.
`uuid_path` (or `uuid`)
Instead of storing a `uuid_path`, it may make more sense to store a `uuid`
string directly in the metadata. Since `uuid.txt` is both used by TA build
tools and read by the proto (and imported by the CA when invoking the TA). We
could try to write into the Cargo.toml, but to which Cargo.toml? Maybe we
should have a workspace?
I’m still thinking about it.
signing_key
It makes sense to write i
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476294732
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
I've reviewed the `cargo-sgx` code snippet — although it reads metadata, it
still accepts parameters via command line with default values. I assume you're
suggesting we should move some params to `[package.metadata]` in `Cargo.toml`
instead of passing them as command-line arguments?
I actually mentioned the idea in the initial message of this PR, but there
are a few considerations. Let's go through each:
`arch` / `std`
We should allow building the same app for different arch and modes.
If these values are hardcoded in `Cargo.toml`, we lose the flexibility. For
example, in CI:
> `cargo-trustzone` should automatically infer whether to build with `std`
based on the feature declared in `Cargo.toml`.
But what should we write in the common examples’ `Cargo.toml`?
If we set `"no-std"`, how would we build it in `std` mode for CI? by
modifying `Cargo.toml` each time? I don't have the answer.
Therefore, I believe **`arch`** and **`std`** should remain as
**command-line parameters**, each with default values.
`ta_dev_kit_dir`
Seems this might suitable for `Cargo.toml`, but it’s different for `arch` —
for example:
```
optee_os/out/arm-plat-vexpress/export-ta_arm64
optee_os/out/arm-plat-vexpress/export-ta_arm32
```
If we put it in `Cargo.toml`, we’d have to edit that file each time we
change the build architecture.
Maybe try this?
```
[metadata]
default = "ta_dev_kit.64" // provide a cmd param to overwrite this?
ta_dev_kit.64 = "_export-ta_arm64"
ta_dev_kit.32 = "_export-ta_arm32"
```
`uuid_path` (or `uuid`)
Instead of storing a `uuid_path`, it may make more sense to store a `uuid`
string directly in the metadata. Since `uuid.txt` is both used by TA build
tools and read by the proto (and imported by the CA when invoking the TA). We
could try to write into the Cargo.toml, but to which Cargo.toml? Maybe we
should have a workspace?
I’m still thinking about it.
signing_key
It makes s
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476294732
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
I've reviewed the `cargo-sgx` code snippet — although it reads metadata, it
still accepts parameters via command line with default values. I assume you're
suggesting we should move some params to `[package.metadata]` in `Cargo.toml`
instead of passing them as command-line arguments?
I actually mentioned the idea in the initial message of this PR, but there
are a few considerations. Let's go through each:
`arch` / `std`
We should allow building the same app for different arch and modes.
If these values are hardcoded in `Cargo.toml`, we lose the flexibility. For
example, in CI:
> `cargo-trustzone` should automatically infer whether to build with `std`
based on the feature declared in `Cargo.toml`.
But what should we write in the common examples’ `Cargo.toml`?
If we set `"no-std"`, how would we build it in `std` mode for CI? by
modifying `Cargo.toml` each time? I don't have the answer.
Therefore, I believe **`arch`** and **`std`** should remain as
**command-line parameters**, each with default values.
`ta_dev_kit_dir`
Seems this might suitable for `Cargo.toml`, but it’s different for `arch` —
for example:
```
optee_os/out/arm-plat-vexpress/export-ta_arm64
optee_os/out/arm-plat-vexpress/export-ta_arm32
```
If we put it in `Cargo.toml`, we’d have to edit that file each time we
change the build architecture.
Maybe try this?
```
[metadata]
default = "ta_dev_kit.64" // ?
ta_dev_kit.64 = "_export-ta_arm64"
ta_dev_kit.32 = "_export-ta_arm32"
```
`uuid_path` (or `uuid`)
Instead of storing a `uuid_path`, it may make more sense to store a `uuid`
string directly in the metadata. Since `uuid.txt` is both used by TA build
tools and read by the proto (and imported by the CA when invoking the TA). We
could try to write into the Cargo.toml, but to which Cargo.toml? Maybe we
should have a workspace?
I’m still thinking about it.
signing_key
It makes sense to write into the Cargo.toml.
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476294732
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
I've reviewed the `cargo-sgx` code snippet — although it reads metadata, it
still accepts parameters via command line with default values. I assume you're
suggesting we should move some params to `[package.metadata]` in `Cargo.toml`
instead of passing them as command-line arguments?
I actually mentioned the idea in the initial message of this PR, but there
are a few considerations. Let's go through each:
`arch` / `std`
We should allow building the same app for different arch and modes.
If these values are hardcoded in `Cargo.toml`, we lose the flexibility. For
example, in CI:
> `cargo-trustzone` should automatically infer whether to build with `std`
based on the feature declared in `Cargo.toml`.
But what should we write in the common examples’ `Cargo.toml`?
If we set `"no-std"`, how would we build it in `std` mode for CI? by
modifying `Cargo.toml` each time? I don't have the answer.
Therefore, I believe **`arch`** and **`std`** should remain as
**command-line parameters**, each with default values.
`ta_dev_kit_dir`
Seems this might suitable for `Cargo.toml`, but it’s different for `arch` —
for example:
```
optee_os/out/arm-plat-vexpress/export-ta_arm64
optee_os/out/arm-plat-vexpress/export-ta_arm32
```
If we put it in `Cargo.toml`, we’d have to edit that file each time we
change the build architecture.
Since different hardware platforms may also have distinct export paths,
automatic detection isn’t always feasible.
So, I keep **`ta_dev_kit_dir`** as a cmd param for now.
`uuid_path` (or `uuid`)
Instead of storing a `uuid_path`, it may make more sense to store a `uuid`
string directly in the metadata. Since `uuid.txt` is both used by TA build
tools and read by the proto (and imported by the CA when invoking the TA). We
could try to write into the Cargo.toml, but to which Cargo.toml? Maybe we
should have a workspace?
I’m still thinking about it.
signing_key
It makes sense to write into t
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2473306038
##
optee-utee-build/src/linker.rs:
##
@@ -126,10 +126,11 @@ impl Linker {
out_dir: PathBuf,
ta_dev_kit_dir: PathBuf,
) -> Result<(), Error> {
-const ENV_TARGET_TA: &str = "TARGET_TA";
-println!("cargo:rerun-if-env-changed={}", ENV_TARGET_TA);
+// cargo passes TARGET as env to the build scripts
+const ENV_TARGET: &str = "TARGET";
+println!("cargo:rerun-if-env-changed={}", ENV_TARGET);
let mut aarch64_flag = true;
-match env::var(ENV_TARGET_TA) {
+match env::var(ENV_TARGET) {
Ok(ref v) if v == "arm-unknown-linux-gnueabihf" || v ==
"arm-unknown-optee" => {
Review Comment:
Agree. We can open an issue to track this inner logic refinement, which
seems like a good starting point for new committers to contribute.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2473250351
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
Thanks for the effort for showing the current design.
```bash
cargo-optee build ta \
--ta_dev_kit_dir \
[--path ] \
[--arch aarch64|arm] \
[--std] \
[--signing_key ] \
[--uuid_path ] \
[--debug]
```
However, I don’t think this should be the ideal interface for TA developers.
I suggest we align and compare the design with existing cargo-based
workflows such as cargo and cargo-sgx to ensure consistency and ease of use.
### Project-related configurations (for both TA and CA) should remain in
Cargo.toml.
1. For example, `cargo-trustzone` should automatically infer whether to
build with std based on the std feature declared in `Cargo.toml`, as you have
proposed using the `std` feature for the std build in previous PR.
2. For other project-specific parameters, we can follow the convention used
by `cargo-sgx`, which leverages `[package.metadata]` as the bridge between
developers and the custom cargo toolchain.
### Toolchain configurations & Zero-argument usability
By default, running `cargo-trustzone build` should work without requiring
additional parameters.
Since project-specific configurations are defined in `Cargo.toml`, the
toolchain should have clear default discovery rules for common paths (e.g., how
`ta_dev_kit_dir` is located, instead of a required parameter for each
invocation of the build cli).
For example, the standard `cargo` toolchain automatically discovers the Rust
sysroot and standard library paths by resolving from `rustc --print sysroot`,
without needing users to specify them manually. Its behavior might also be
influenced by environment variables such as RUSTUP_HOME and CARGO_HOME, which
allow users to override toolchain locations in a standardized way.
Similarly, `cargo-trustzone` should support automatic discovery of the
default `ta_dev_kit_dir` (e.g., from an environment variable, configuration
file, or standard installation path) to provide a smooth and familiar developer
experience consistent with the normal cargo wo
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2473250351
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
Thanks for the effort for showing the current design.
```bash
cargo-optee build ta \
--ta_dev_kit_dir \
[--path ] \
[--arch aarch64|arm] \
[--std] \
[--signing_key ] \
[--uuid_path ] \
[--debug]
```
However, I don’t think this should be the ideal interface for TA developers.
I suggest we align and compare the design with existing cargo-based
workflows such as cargo and cargo-sgx to ensure consistency and ease of use.
### Project-related configurations (for both TA and CA) should remain in
Cargo.toml.
1. For example, `cargo-trustzone` should automatically infer whether to
build with std based on the std feature declared in `Cargo.toml`, as you have
proposed use `std` feature for the std build in previous PR.
2. For other project-specific parameters, we can follow the convention used
by `cargo-sgx`, which leverages `[package.metadata]` as the bridge between
developers and the custom cargo toolchain.
### Toolchain configurations & Zero-argument usability
By default, running `cargo-trustzone` build should work without requiring
additional parameters.
Since project-specific configurations are defined in `Cargo.toml`, the
toolchain should have clear default discovery rules for common paths (e.g., how
`ta_dev_kit_dir` is located, instead of a required parameter for each
invocation of the build cli).
For example, the standard `cargo` toolchain automatically discovers the Rust
sysroot and standard library paths by resolving from `rustc --print sysroot`,
without needing users to specify them manually. Its behavior might also be
influenced by environment variables such as RUSTUP_HOME and CARGO_HOME, which
allow users to override toolchain locations in a standardized way.
Similarly, `cargo-trustzone` should support automatic discovery of the
default `ta_dev_kit_dir` (e.g., from an environment variable, configuration
file, or standard installation path) to provide a smooth and familiar developer
experience consistent with the normal cargo workflow
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2473114474
##
optee-utee-build/src/linker.rs:
##
@@ -126,10 +126,11 @@ impl Linker {
out_dir: PathBuf,
ta_dev_kit_dir: PathBuf,
) -> Result<(), Error> {
-const ENV_TARGET_TA: &str = "TARGET_TA";
-println!("cargo:rerun-if-env-changed={}", ENV_TARGET_TA);
+// cargo passes TARGET as env to the build scripts
+const ENV_TARGET: &str = "TARGET";
+println!("cargo:rerun-if-env-changed={}", ENV_TARGET);
let mut aarch64_flag = true;
-match env::var(ENV_TARGET_TA) {
+match env::var(ENV_TARGET) {
Ok(ref v) if v == "arm-unknown-linux-gnueabihf" || v ==
"arm-unknown-optee" => {
Review Comment:
I’ve checked whether the interfaces of `optee-utee-build` crate need any
adjustments for the new building system.
After reviewing the interface layer (from cargo to the build script
parameter passing), the build script mainly reads Cargo auto-set environment
variables and the `TA_DEV_KIT` set by developer. I think this interface looks
clear and does not require changes (see the appendix in `cargo-optee/README.md`
for details).
Therefore, this is indeed an inner logic refinement issue. Let's create a
separate issue to track this refactoring and resolve it in another PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2473033400
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
Updated in the new commit and `cargo-optee/README.md`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3461489138 Summary of the new commit: ### Users & Scenarios **For developers (dev/emulation environments):** They only need to build and run a few examples or their own projects. After installing the toolchain, they can simply use `cargo-optee` to build CA, TA and Plugins with the source code. **For CI:** We need to build all examples. The workflow is: 1. Setup toolchains and OP-TEE libraries 2. Run: `TA_DEV_KIT_DIR=xxx OPTEE_CLIENT_EXPORT=xxx ./ci/build.sh` to build all examples using `cargo-optee`, the script reads `examples/metadata.json` to get info of each example I've tested the `build.sh` and all examples build passed for `aarch64` TA and `aarch64` CA: - No-std mode: `source ~/.cargo/env && TA_DEV_KIT_DIR=/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64 OPTEE_CLIENT_EXPORT=/teaclave/optee/optee_client/export_arm64 ./ci/build.sh` - Std mode: `source ~/.cargo/env && TA_DEV_KIT_DIR=/teaclave/optee/optee_os/out/arm-plat-vexpress/export-ta_arm64 OPTEE_CLIENT_EXPORT=/teaclave/optee/optee_client/export_arm64 ./ci/build.sh --std` We can remove all Makefiles from the SDK now. For design, user interface and other details, please refer to `cargo-optee/README.md`. -- ### Changes - **cargo-optee**: - Add `build plugin` support - Add `README.md` - **examples**: Add `metadata.json` for recording all application info - **ci**: Add `build.sh` for building all examples defined in `metadata.json` - **Fix special cases**: - **mnist-rs**: Move `-Z build-std` parameters into `.cargo/config.toml` for self-maintenance - **mnist-rs**: `cargo-optee` now searches workspace `Cargo.toml` (needs polish) - **std-only TAs** (message_passing_interface, serde, secure_db_abstraction, tls_client, tls_server): Require mandatory `std` feature for consistency. After this change: common TAs and `std-only` TAs explicitly require `--std` flag for build with `std`. This ensures `cargo-optee` handles `std` feature in a unified manner for all examples. ### Notes - For building CAs, `proto` includes relative UUID path `../../uuid.txt`, should be revised - CI: requires `apt-get install jq` for json parsing in `ci/build.sh` ### Next steps - remove all Makefiles - enable the Github Action CI - remove all Xargo.toml from ta - polish the code -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
m4sterchain commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2462214183
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,158 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ca_builder;
+mod common;
+mod ta_builder;
+
+use common::Arch;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Parser)]
+struct BuildTypeCommonOptions {
+/// Path to the app directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: Arch,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+}
+
+#[derive(Debug, Subcommand)]
+enum BuildCommand {
+/// Build a Trusted Application
+TA {
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+/// Build a Client Application (Host)
+CA {
+/// Path to the OP-TEE client export directory (mandatory)
+#[arg(long = "optee_client_export", required = true)]
+optee_client_export: PathBuf,
+
+#[command(flatten)]
+common: BuildTypeCommonOptions,
+},
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
Review Comment:
Do we have a high level design about the cargo-optee?
What is the convention | requirement between different roles?
- cargo-optee and underlining build system
- cargo-optee and TA developer
Please refer to the following link as a reference.
`https://github.com/automata-network/automata-sgx-sdk?tab=readme-ov-file#getting-started`
##
optee-utee-build/src/linker.rs:
##
@@ -126,10 +126,11 @@ impl Linker {
out_dir: PathBuf,
ta_dev_kit_dir: PathBuf,
) -> Result<(), Error> {
-const ENV_TARGET_TA: &str = "TARGET_TA";
-println!("cargo:rerun-if-env-changed={}", ENV_TARGET_TA);
+// cargo passes TARGET as env to the build scripts
+const ENV_TARGET: &str = "TARGET";
+println!("cargo:rerun-if-env-changed={}", ENV_TARGET);
let mut aarch64_flag = true;
-match env::var(ENV_TARGET_TA) {
+match env::var(ENV_TARGET) {
Ok(ref v) if v == "arm-unknown-linux-gnueabihf" || v ==
"arm-unknown-optee" => {
Review Comment:
This function & inner logic is not straight forward and difficult to
maintain.
Please leverage the Rust type system an use linear logic, instead of the
boolean flag and nested if/else.
The convention here between src/ta.ld.S and created ta.lds is not straight
forward.
It seems we can make it better by clearly handling Aarch64 and Arm32
separately and better leveraging the Type system to check the supported
configurations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
DemesneGH commented on PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#issuecomment-3442967683 new commit: - enable build ca - add sub-command: ta and ca - revise param "arch" from string to enum - fix building error for 32bit TAs, modified optee-utee-build/src/linker.rs - fix license -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
ivila commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2458315205
##
cargo-optee/src/ta_builder.rs:
##
@@ -0,0 +1,329 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use anyhow::{bail, Result};
+use std::env;
+use std::fs;
+use std::path::{Path, PathBuf};
+use std::process::{Command, Output};
+use tempfile::TempDir;
+
+// Embed the target JSON files at compile time
+const AARCH64_TARGET_JSON: &str =
include_str!("../aarch64-unknown-optee.json");
+const ARM_TARGET_JSON: &str = include_str!("../arm-unknown-optee.json");
+
+// Helper function to print command output and return error
+fn print_output_and_bail(cmd_name: &str, output: &Output) -> Result<()> {
+eprintln!(
+"{} stdout: {}",
+cmd_name,
+String::from_utf8_lossy(&output.stdout)
+);
+eprintln!(
+"{} stderr: {}",
+cmd_name,
+String::from_utf8_lossy(&output.stderr)
+);
+bail!(
+"{} failed with exit code: {:?}",
+cmd_name,
+output.status.code()
+)
+}
+
+pub struct TaBuildConfig {
+pub arch: String,// "aarch64" or "arm"
Review Comment:
Suggest using enum instead of String for `arch`, and derive `ValueEnum` for
usage in clap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
ivila commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2458304431
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,138 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use anyhow::bail;
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ta_builder;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
+/// Build a Trusted Application (TA)
+#[clap(name = "build")]
+Build {
+/// Type of build target (currently only 'ta' is supported)
+#[arg(value_name = "TYPE")]
+build_type: String,
+
+/// Path to the TA directory (default: current directory)
+#[arg(long = "path", default_value = ".")]
+path: PathBuf,
+
+/// Target architecture: aarch64 or arm (default: aarch64)
+#[arg(long = "arch", default_value = "aarch64")]
+arch: String,
+
+/// Enable std feature for the TA
+#[arg(long = "std")]
+std: bool,
+
+/// Path to the TA dev kit directory (mandatory)
+#[arg(long = "ta_dev_kit_dir", required = true)]
+ta_dev_kit_dir: PathBuf,
+
+/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
+#[arg(long = "signing_key")]
+signing_key: Option,
+
+/// Path to the UUID file (default: ../uuid.txt)
+#[arg(long = "uuid_path", default_value = "../uuid.txt")]
+uuid_path: PathBuf,
+
+/// Build in debug mode (default is release)
+#[arg(long = "debug")]
+debug: bool,
+},
+}
+
+fn main() {
+
env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info"))
+.format_timestamp_millis()
+.init();
+
+// Setup cargo environment if needed
+if let Ok(home) = env::var("HOME") {
+let cargo_env = format!("{}/.cargo/env", home);
+if std::path::Path::new(&cargo_env).exists() {
+// Add cargo bin to PATH
+let cargo_bin = format!("{}/.cargo/bin", home);
+if let Ok(current_path) = env::var("PATH") {
+let new_path = format!("{}:{}", cargo_bin, current_path);
+env::set_var("PATH", new_path);
+}
+} else {
+eprintln!("Error: Cargo environment file not found at: {}. Please
ensure Rust and Cargo are installed.", cargo_env);
+abort();
+}
+}
+
+let cli = Cli::parse();
+let result = execute_command(cli.cmd);
+
+if let Err(e) = result {
+eprintln!("Error: {}", e);
+abort();
+}
+}
+
+fn execute_command(cmd: Command) -> anyhow::Result<()> {
+match cmd {
+Command::Build {
+build_type,
+path,
+arch,
+std,
+ta_dev_kit_dir,
+signing_key,
+uuid_path,
+debug,
+} => {
+// Validate build type
+if build_type != "ta" {
Review Comment:
Turning build_type into a subcommand helps eliminate the need for such
comparisons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] experimental: add cargo-optee [teaclave-trustzone-sdk]
ivila commented on code in PR #245:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2458298878
##
cargo-optee/src/main.rs:
##
@@ -0,0 +1,138 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use anyhow::bail;
+use clap::{Parser, Subcommand};
+use std::env;
+use std::path::PathBuf;
+use std::process::abort;
+
+mod ta_builder;
+
+#[derive(Debug, Parser)]
+#[clap(version = env!("CARGO_PKG_VERSION"))]
+#[clap(about = "Build tool for OP-TEE Rust projects")]
+pub(crate) struct Cli {
+#[clap(subcommand)]
+cmd: Command,
+}
+
+#[derive(Debug, Subcommand)]
+enum Command {
+/// Build a Trusted Application (TA)
+#[clap(name = "build")]
+Build {
Review Comment:
Each build type uses its own set of options, with some overlap.
Making build_type a subcommand would help developers avoid irrelevant
options.
For demo:
```rust
use clap::{Parser, Subcommand};
use std::path::PathBuf;
#[derive(Debug, Parser)]
struct BuildTypeCommonOptions {
/// Path to the TA directory (default: current directory)
#[arg(long = "path", default_value = ".")]
path: PathBuf,
/// Target architecture: aarch64 or arm (default: aarch64)
#[arg(long = "arch", default_value = "aarch64")]
arch: String,
/// Path to the UUID file (default: ../uuid.txt)
#[arg(long = "uuid_path", default_value = "../uuid.txt")]
uuid_path: PathBuf,
/// Build in debug mode (default is release)
#[arg(long = "debug")]
debug: bool,
}
#[derive(Debug, Subcommand)]
enum BuildCommand {
TA {
/// Enable std feature for the TA
#[arg(long = "std")]
std: bool,
/// Path to the TA dev kit directory (mandatory)
#[arg(long = "ta_dev_kit_dir", required = true)]
ta_dev_kit_dir: PathBuf,
/// Path to the TA signing key (default:
$(TA_DEV_KIT_DIR)/keys/default_ta.pem)
#[arg(long = "signing_key")]
signing_key: Option,
#[command(flatten)]
common: BuildTypeCommonOptions,
},
}
#[derive(Debug, Subcommand)]
enum Command {
/// Build a Trusted Application (TA)
#[clap(name = "build")]
#[command(subcommand)]
Build(BuildCommand),
}
#[derive(Debug, Parser)]
#[clap(version = env!("CARGO_PKG_VERSION"))]
#[clap(about = "Build tool for OP-TEE Rust projects")]
pub(crate) struct Cli {
#[clap(subcommand)]
cmd: Command,
}
fn main() {
Cli::parse();
}
```
Developers will run `${cmd} build ta --${options}` instead of `${cmd} build
--build_ta=ta --${options}`, and they will get clearer help messages(only the
relevant options involve)
https://github.com/user-attachments/assets/bbb6f353-e947-4e55-869c-29f8a718237b";
/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
