m4sterchain commented on code in PR #144: URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/144#discussion_r1804322948
########## examples/message_passing_interface-rs/Makefile: ########## @@ -0,0 +1,33 @@ +# 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. + +# If _HOST or _TA specific compiler/target are not specified, then use common +# compiler/target for both +CROSS_COMPILE_HOST ?= aarch64-linux-gnu- +CROSS_COMPILE_TA ?= aarch64-linux-gnu- +TARGET_HOST ?= aarch64-unknown-linux-gnu +TARGET_TA ?= aarch64-unknown-linux-gnu + +all: + $(q)make -C host TARGET_HOST=$(TARGET_HOST) \ + CROSS_COMPILE_HOST=$(CROSS_COMPILE_HOST) + $(q)make -C ta TARGET_TA=$(TARGET_TA) \ + CROSS_COMPILE_TA=$(CROSS_COMPILE_TA) + Review Comment: I see that at the top level, TARGET_HOST and TARGET_TA are used to differentiate between the build parameters for the host and trusted application (TA), which makes sense. However, at the sub-level (host/Makefile and ta/Makefile), it may be unnecessary to keep differentiating these parameters. The sub-level Makefiles could rely on the variables passed down from the top-level Makefile and use more generic variable names, such as TARGET and CROSS_COMPILE, similar as the `@CROSS_COMPILE` usage for building ring. Instead of maintaining the separate TARGET_HOST and TARGET_TA at each level, we could streamline the build process by using: ``` @make -C host TARGET=$(TARGET_HOST) CROSS_COMPILE=$(CROSS_COMPILE_HOST) @make -C ta TARGET=$(TARGET_TA) CROSS_COMPILE=$(CROSS_COMPILE_TA) ``` ########## docs/migrating-to-new-building-env.md: ########## @@ -0,0 +1,57 @@ +--- +permalink: /trustzone-sdk-docs/migrating-to-new-building-env.md +--- + +## Migration Guide: Moving from `master` to `main` Branch (Post-Oct 2024) + +Since the `main` branch (after October 2024) introduces breaking changes +to the build environment, if users of the legacy `master` branch want to +keep upstream or use a new version of the Rust toolchain, they will need +to follow these steps to migrate their TA to the new environment. + +### Step 1: Migrate Old Code to the New Environment + +To migrate an example project (e.g., `tls_server-rs` in `examples/`), you Review Comment: The current migration strategy seems to ask developers to duplicate the hello-world example and manually copy their source code into the corresponding subdirectories. However, this approach doesn't clearly highlight what has actually changed and what developers need to focus on during the migration. The target audience for this section is developers currently using the master branch who have their own code repositories. The key point we need to emphasize is that no code changes are required during the migration—only the build scripts need to be updated. The message we want to convey is simple: developers should be aware that the build process has changed compared to the legacy master branch, and the build scripts will need to be replaced. Could we clearly outline the differences in the build scripts (e.g. Cargo.toml, build.rs, Makefile, etc), step-by-step, and perhaps reference a specific commit for further clarification? This way, developers can focus on adapting their build environment without being concerned about unnecessary code modifications. ########## examples/message_passing_interface-rs/Makefile: ########## @@ -0,0 +1,33 @@ +# 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. + +# If _HOST or _TA specific compiler/target are not specified, then use common +# compiler/target for both +CROSS_COMPILE_HOST ?= aarch64-linux-gnu- +CROSS_COMPILE_TA ?= aarch64-linux-gnu- +TARGET_HOST ?= aarch64-unknown-linux-gnu +TARGET_TA ?= aarch64-unknown-linux-gnu + +all: + $(q)make -C host TARGET_HOST=$(TARGET_HOST) \ + CROSS_COMPILE_HOST=$(CROSS_COMPILE_HOST) + $(q)make -C ta TARGET_TA=$(TARGET_TA) \ + CROSS_COMPILE_TA=$(CROSS_COMPILE_TA) + +clean: + $(q)make -C host clean Review Comment: I cannot find the the definition of `$q` in this commit. Is there any reference or purpose for using $(q) as prefix of the make command? ########## examples/message_passing_interface-rs/ta/ta_static.rs: ########## @@ -0,0 +1,102 @@ +// 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 core::ffi::*; Review Comment: As wildcard imports are discouraged, can we replace it with `use core::ffi;`, and explicit reference `ffi::` for each imported type? -- 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: dev-unsubscr...@teaclave.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org For additional commands, e-mail: dev-h...@teaclave.apache.org
