microsoft / mu_devops

Project Mu Developer Operations
https://microsoft.github.io/mu/
Other
26 stars 22 forks source link

Conditionalize RustSetupSteps.yml to Non-Self-Hosted Agents #246

Closed kuqin12 closed 1 year ago

kuqin12 commented 1 year ago

The current rust setup script embedded for package CI build will cause failures on selfhosted agent pipelines.

On Windows AARCH64 agents, the failure is due to the installation of toolchain is currently specified to be x86_64, which will not succeed on AARCH64 systems in the self-host pool. In addition, the update will apply to installed cargo, which could cause conflict when multiple agents run on the same host system.

On Linux agents, the failure is due to modifications made to container permissions. Self-hosted Linux agents do not use containers.

The change here removes the toolchain installation steps for selfhost agent matrix builds as it assumes these systems preset the needed environment properly.

makubacki commented 1 year ago

Ultimately, self-hosted agents will need to become a part of the test process for changes here to prevent future breaks.

While more changes will be needed to comprehensively support aarch64 hosts (like pulling arch-specific cargo extensions), I put up a PR here (https://github.com/microsoft/mu_devops/pull/247) to determine the target triple used in the rustup step and I imagine it could be helpful in the download steps as well.

kuqin12 commented 1 year ago

Ultimately, self-hosted agents will need to become a part of the test process for changes here to prevent future breaks.

While more changes will be needed to comprehensively support aarch64 hosts (like pulling arch-specific cargo extensions), I put up a PR here (#247) to determine the target triple used in the rustup step and I imagine it could be helpful in the download steps as well.

It definitely does resolve the downloading process. But there is another issue when we initiate multiple agents on the same host system. It could run into some conflicts, if we directly operate on the rust toolchain? I worked around the python pip modules with python virtual environment for each agent. But I am not sure if the same exist for cargo/rustup?

kuqin12 commented 1 year ago

Btw, @makubacki, not sure if you noticed, but there has been some silent failures on MsCorePkg when it comes to rust test: https://dev.azure.com/projectmu/mu/_build/results?buildId=54419&view=results (find the Core package log and search for cargo make, and the failure was about "error: invalid character , in pkgid: HelloWorldRustDxe,RustBootServicesAllocatorDxe, characters must be Unicode XID characters (numbers, -, _, or most letters)").

Not sure why pipeline did not fail, but I think the above error was due to cargo make version changed from 0.36.13 to 0.37.1.

makubacki commented 1 year ago

Ultimately, self-hosted agents will need to become a part of the test process for changes here to prevent future breaks. While more changes will be needed to comprehensively support aarch64 hosts (like pulling arch-specific cargo extensions), I put up a PR here (#247) to determine the target triple used in the rustup step and I imagine it could be helpful in the download steps as well.

It definitely does resolve the downloading process. But there is another issue when we initiate multiple agents on the same host system. It could run into some conflicts, if we directly operate on the rust toolchain? I worked around the python pip modules with python virtual environment for each agent. But I am not sure if the same exist for cargo/rustup?

You can have multiple toolchains installed simultaneously. There's a concept of a "default" toolchain, additionally explicit toolchains can be given to most commands with --toolchain. Are you seeing specific issues related to it?

makubacki commented 1 year ago

Btw, @makubacki, not sure if you noticed, but there has been some silent failures on MsCorePkg when it comes to rust test: https://dev.azure.com/projectmu/mu/_build/results?buildId=54419&view=results (find the Core package log and search for cargo make, and the failure was about "error: invalid character , in pkgid: HelloWorldRustDxe,RustBootServicesAllocatorDxe, characters must be Unicode XID characters (numbers, -, _, or most letters)").

Not sure why pipeline did not fail, but I think the above error was due to cargo make version changed from 0.36.13 to 0.37.1.

Thanks, I'll look into it.

makubacki commented 1 year ago

Btw, @makubacki, not sure if you noticed, but there has been some silent failures on MsCorePkg when it comes to rust test: https://dev.azure.com/projectmu/mu/_build/results?buildId=54419&view=results (find the Core package log and search for cargo make, and the failure was about "error: invalid character , in pkgid: HelloWorldRustDxe,RustBootServicesAllocatorDxe, characters must be Unicode XID characters (numbers, -, _, or most letters)"). Not sure why pipeline did not fail, but I think the above error was due to cargo make version changed from 0.36.13 to 0.37.1.

Thanks, I'll look into it.

Issue is due to the cargo-tarpaulin 0.27 release updating clap v2 to v4:

"Upgraded from clap v2 to v4. This has a few changes, notably any arguments which can be specified more
than once require multiple entries so --run-types doc test needs to be turned into --run-types doc --run-types test"

Will send a change for it later.

kuqin12 commented 1 year ago

Verified in https://dev.azure.com/projectmu/mu/_build/results?buildId=54649&view=results and https://github.com/microsoft/mu_tiano_platforms/pull/710, as well as selfhosted pipelines.

makubacki commented 1 year ago

Fyi, in case you'd like to double check the changes.