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<PathBuf>, + + #[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> \ [--path <PATH>] \ [--arch aarch64|arm] \ [--std] \ [--signing_key <PATH>] \ [--uuid_path <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. ### Learn from Existing Design Lastly, as I have suggested multiple times, please take a deeper look at the cargo-sgx tool and compare our current design against it. Many of the challenges we are encountering have already been addressed there — such as providing a developer-friendly interface, automatic build support for customized std environments, and seamless integration of loadable trusted components with untrusted applications. There may be additional insights and techniques we can learn from those design patterns. I strongly recommend gaining a solid understanding of what is already available before reinventing the wheel. -- 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]
