Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optee_rust_examples_ext: Fix Rust toolchain conflicts #731

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

b49020
Copy link
Contributor

@b49020 b49020 commented Feb 5, 2024

Buildroot provides its own Rust toolchain for various Linux user-space
components. However, that toolchain doesn't support nightly version of
Rust complier which we need for OP-TEE Rust examples for the time being.

Due to two separate Rust toolchains being used for different buildroot
components, there are conflicts [1] [2] observed leading to CI errors.
In order to fix them enable OP-TEE specific Rust toolchain specifically
to build OP-TEE Rust examples rather than enabling it for the entire
buildroot build.

Fixes: [1] #727
Fixes: [2] #728

@DemesneGH fyi..

Copy link
Contributor

@jenswi-linaro jenswi-linaro left a comment

Choose a reason for hiding this comment

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

Acked-by: Jens Wiklander <[email protected]>

@b49020
Copy link
Contributor Author

b49020 commented Feb 6, 2024

Tag applied, thanks.

@jforissier
Copy link
Contributor

jforissier commented Feb 6, 2024

2024-02-06T08:57:39.9105465Z make[5]: cargo: No such file or directory

ci_log.txt

https://github.com/jforissier/optee_os/actions/runs/7796709151/job/21261880050

Tested by applying this patch to ci.yml: jforissier/optee_os@99e65f6 and pushing to my forked optee_os so that CI is run.

@b49020
Copy link
Contributor Author

b49020 commented Feb 6, 2024

@jforissier Thanks for testing. Its embarrassing to see how my local environment tricked me to not catch errors in the first place. BTW, the CI error should be fixed now with the Rust toolchain dependency resolved for Rust examples. Can you please give it a retry?

@jforissier
Copy link
Contributor

Thanks for the update. Still not working (cargo: No such file or directory). I suggest you try running in the Docker image that the CI uses

docker run -it --rm jforissier/optee_os_ci:qemuv8_check2
# cd
# vi get_optee_qemuv8.sh
(Add "git fetch github pull/731/merge && git checkout FETCH_HEAD" before the "make toolchains" line)
# ./get_optee_qemuv8.sh
# cd optee_repo_qemu_v8/build
# make -j$(nproc) buildroot

@b49020
Copy link
Contributor Author

b49020 commented Feb 6, 2024

Still not working (cargo: No such file or directory). I suggest you try running in the Docker image that the CI uses

Sure, that make sense. I will give it a try.

@b49020 b49020 changed the title optee_rust_examples_ext: Use buildroot provided Rust toolchain optee_rust_examples_ext: Fix Rust toolchain conflicts Feb 7, 2024
@b49020
Copy link
Contributor Author

b49020 commented Feb 7, 2024

While digging further the buildroot provided Rust toolchain actually doesn't support nightly version of Rust compiler. So we have to live with OP-TEE specific Rust nightly toolchain for the time being.

Now with this updated PR, it is yet another attempt to fix Rust toolchain conflicts. @jenswi-linaro I dropped your ack since its I switched to a different approach fixing CI problems.

@jforissier Docker image seems to pass for me, can you give this PR a try through CI?

@b49020
Copy link
Contributor Author

b49020 commented Feb 7, 2024

BTW, the IBART error seems unrelated to this PR. @jbech-linaro please have a look.

@jforissier
Copy link
Contributor

jforissier commented Feb 7, 2024

I tried building twice and it failed twice with messages similar to:

error: component download failed for rust-std-aarch64-unknown-linux-gnu: could not rename downloaded file from '/root/.rustup/downloads/6847b6e5e7c628c3d934dbdac2adeb0098364b543159bb158fa5d0aa7b3d1509.partial' to '/root/.rustup/downloads/6847b6e5e7c628c3d934dbdac2adeb0098364b543159bb158fa5d0aa7b3d1509'

Log file here: build.log.zip

It looks like something is happening in parallel which should not.

I am building on a 128-vCPU EC2 host with the following commands:

docker run -it --rm jforissier/optee_os_ci:qemuv8_check2
vi /root/get_optee_qemuv8.sh
# Before the "make toolchains" line, add:
git fetch github pull/731/merge && git checkout FETCH_HEAD
# Then run:
yes "" | /root/get_optee_qemuv8.sh && make -j$(nproc) -C /root/optee_repo_qemu_v8/build buildroot 2>&1 | tee build.log

I can give you access temporarily if you wish. Just give me your SSH key (~/.ssh/id_rsa.pub or similar) and I will give you the current DNS name of the VM.

@b49020
Copy link
Contributor Author

b49020 commented Feb 7, 2024

@jforissier Thanks for providing your build machine access to me. It turned out to be following fix in order to get the build to pass:

diff --git a/br-ext/package/optee_rust_examples_ext/optee_rust_examples_ext.mk b/br-ext/package/optee_rust_examples_ext/optee_rust_examples_ext.mk
index 64b2a5f..0b538b3 100644
--- a/br-ext/package/optee_rust_examples_ext/optee_rust_examples_ext.mk
+++ b/br-ext/package/optee_rust_examples_ext/optee_rust_examples_ext.mk
@@ -10,7 +10,7 @@ EXAMPLE = $(wildcard examples/*)
 
 define OPTEE_RUST_EXAMPLES_EXT_BUILD_CMDS
        echo Building OP-TEE rust examples && \
-       source $(BR2_PACKAGE_OPTEE_RUST_EXAMPLES_EXT_TC_PATH)/.cargo/env && \
+       source $(subst $\",,$(BR2_PACKAGE_OPTEE_RUST_EXAMPLES_EXT_TC_PATH))/.cargo/env && \
        TARGET_HOST=$(BR2_PACKAGE_OPTEE_RUST_EXAMPLES_EXT_TARGET_HOST) \
        TARGET_TA=$(BR2_PACKAGE_OPTEE_RUST_EXAMPLES_EXT_TARGET_TA) \
        CROSS_COMPILE_HOST=$(BR2_PACKAGE_OPTEE_RUST_EXAMPLES_EXT_CROSS_COMPILE_HOST) \

The PR has been updated, hopefully it should pass the CI now.

@jforissier
Copy link
Contributor

Still those "could not rename" errors...

@b49020 b49020 force-pushed the master branch 5 times, most recently from 746252e to 60363f5 Compare February 9, 2024 06:57
@b49020
Copy link
Contributor Author

b49020 commented Feb 9, 2024

Further digging led me to discover that externally installed OP-TEE specific toolchain can't be discovered within the buildroot environment. This was leading to Rust toolchain installation while building Rust examples which doesn't support parallelism. Given that I instead switched Rust toolchain installation to configure stage for Rust examples. With that I no longer observe any "could not rename" errors. Feel free to give it a retry.

@jforissier
Copy link
Contributor

Tested-by: Jerome Forissier <[email protected]>

@b49020 many thanks for spending time on this nasty bug 😉 it will be good to have Rust checked in all the QEMUv8 CI jobs!

@b49020
Copy link
Contributor Author

b49020 commented Feb 9, 2024

Thanks @jforissier for your support with testing, tag applied.

Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

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

One comment below.

Acked-by: Jerome Forissier <[email protected]>

Buildroot provides its own Rust toolchain for various Linux user-space
components. However, that toolchain doesn't support nightly version of
Rust complier which we need for OP-TEE Rust examples for the time being.

Due to two separate Rust toolchains being used for different buildroot
components, there are conflicts [1] [2] observed leading to CI errors.
In order to fix them enable OP-TEE specific Rust toolchain specifically
to build OP-TEE Rust examples rather than enabling it for the entire
buildroot build.

Fixes: [1] OP-TEE#727
Fixes: [2] OP-TEE#728
Acked-by: Jerome Forissier <[email protected]>
Tested-by: Jerome Forissier <[email protected]>
Signed-off-by: Sumit Garg <[email protected]>
@b49020
Copy link
Contributor Author

b49020 commented Feb 9, 2024

Added proper comments, tag applied.

@jforissier jforissier merged commit f0a2eef into OP-TEE:master Feb 9, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants