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

cmake doesn't probe for CHERIOT_{LLVM_BIN,RTOS_SDK} env vars? #101

Closed
nwf opened this issue May 26, 2024 · 2 comments
Closed

cmake doesn't probe for CHERIOT_{LLVM_BIN,RTOS_SDK} env vars? #101

nwf opened this issue May 26, 2024 · 2 comments
Assignees

Comments

@nwf
Copy link

nwf commented May 26, 2024

Despite the instructions saying that we need to run

export CHERIOT_LLVM_BIN=/path/to/cheriot-llvm/bin
export CHERIOT_RTOS_SDK=/path/to/cheriot-rtos/sdk
the key occurrences of those strings in the cmake files herein are...

I am just a caveman and cmake frightens and confuses me, but cmake's documentation suggests that one needs to use DEFINED ENV{...} to probe at environment variables. Assuming one also wants to support the use of -D on the command line for these parameters, I think that means something like this diff should be applied:

diff --git a/sw/cheri/CMakeLists.txt b/sw/cheri/CMakeLists.txt
index f46fc99..f17ab2e 100644
--- a/sw/cheri/CMakeLists.txt
+++ b/sw/cheri/CMakeLists.txt
@@ -11,8 +11,12 @@ if(NOT DEFINED CMAKE_TOOLCHAIN_FILE)
 endif()
 
 if(NOT DEFINED CHERIOT_RTOS_SDK)
-  FetchContent_Populate(CHERIOT_RTOS)
-  set(CHERIOT_RTOS_SDK "${cheriot_rtos_SOURCE_DIR}/sdk")
+  if (NOT DEFINED ENV{CHERIOT_RTOS_SDK})
+    FetchContent_Populate(CHERIOT_RTOS)
+    set(CHERIOT_RTOS_SDK "${cheriot_rtos_SOURCE_DIR}/sdk")
+  else()
+    set(CHERIOT_RTOS_SDK "$ENV{CHERIOT_RTOS_SDK}")
+  endif()
 endif()
 
 project(sonata_system_cheriot_sw LANGUAGES C CXX ASM)
diff --git a/sw/cheri/cheriot_toolchain.cmake b/sw/cheri/cheriot_toolchain.cmake
index 0c3d690..867b1b6 100644
--- a/sw/cheri/cheriot_toolchain.cmake
+++ b/sw/cheri/cheriot_toolchain.cmake
@@ -6,6 +6,10 @@ if (DEFINED CHERIOT_LLVM_BIN)
   set(CMAKE_CXX_COMPILER "${CHERIOT_LLVM_BIN}/clang++")
   set(CMAKE_C_COMPILER   "${CHERIOT_LLVM_BIN}/clang")
   set(CMAKE_ASM_COMPILER "${CHERIOT_LLVM_BIN}/clang")
+elseif (DEFINED ENV{CHERIOT_LLVM_BIN})
+  set(CMAKE_CXX_COMPILER "$ENV{CHERIOT_LLVM_BIN}/clang++")
+  set(CMAKE_C_COMPILER   "$ENV{CHERIOT_LLVM_BIN}/clang")
+  set(CMAKE_ASM_COMPILER "$ENV{CHERIOT_LLVM_BIN}/clang")
 else()
   set(CMAKE_CXX_COMPILER clang++)
   set(CMAKE_C_COMPILER   clang)

I haven't made a PR, tho', because it's possible I'm completely off the mark.

P.S. This is also masking an annoyance that I presume will go away in time: until at least lowRISC/cheriot-rtos@02ed83a is merged to mainline, the instructions at

#### CHERIoT RTOS SDK Installation
You will need a copy of [CHERIoT RTOS](https://github.com/microsoft/cheriot-rtos/tree/main) for this section.
On Windows you may need to set `git config --global core.symlinks true` *before* cloning the repository.
Clone the repository somewhere, *not* into the root of the `sonata-system` directory, but at the same level as `sonata-system`:
```sh
cd ..
git clone --recurse https://github.com/microsoft/cheriot-rtos.git
```
should tell us to use the lowRISC fork's sonata branch, yeah?

@marnovandermaas
Copy link
Contributor

For the documentation part, here's a fix: #103
Thanks for suggesting the cmake changes.

@marnovandermaas
Copy link
Contributor

This should now be fixed.

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

No branches or pull requests

3 participants