sel4test icon indicating copy to clipboard operation
sel4test copied to clipboard

CMake: allow skipping include of settings.cmake

Open axel-h opened this issue 4 years ago • 8 comments

My build system currently sets up everything and then uses add_subdirectory() to include sel4test. This patch allow building sel4test then. Maybe there is a smarter solution here by impersonating settings.cmake somehow to this does nothing?

axel-h avatar Jun 23 '21 15:06 axel-h

What is your build system setting up before including sel4test? Could you instead include add_subdirectory(apps/sel4test-driver) instead of the toplevel CMakeLists.txt that currently expects to setup the elfloader and kernel.elf targets?

kent-mcleod avatar Jun 23 '21 22:06 kent-mcleod

Our build system basically does what settings.cmake does and that conflicts. The goal is being able to use existing seL4 projects like this without modification, so we don't have to create another wrapper project. Having the switch SEL4TEST_DONT_USE_SEL4_DEFAULT_BUILD_SYSTEM nicely solves this.

axel-h avatar Dec 01 '21 13:12 axel-h

The goal is being able to use existing seL4 projects like this without modification, so we don't have to create another wrapper project.

But isn't this already a modification? The coupling between settings.cmake and CMakeLists.txt isn't a stable public interface. settings.cmake contains many sel4test specific options and options can be moved between CMakeLists.txt and settings.cmake without causing external breakages.

add_subdirectory(apps/sel4test-driver) or even using https://cmake.org/cmake/help/latest/variable/CMAKE_PROJECT_PROJECT-NAME_INCLUDE_BEFORE.html#variable:CMAKE_PROJECT_%3CPROJECT-NAME%3E_INCLUDE_BEFORE will let you include the sel4test applications into another project without requiring special modifications upstream.

kent-mcleod avatar Dec 08 '21 22:12 kent-mcleod

Thanks, I will give this a try.

axel-h avatar Dec 08 '21 22:12 axel-h

@axel-h, feel free to merge this if you still need the change

kent-mcleod avatar Feb 21 '23 01:02 kent-mcleod

I have not found a better solution yet for our use case with a different/custom build system. I will check our internal CI one more tomorrow and merge this then. Thanks.

axel-h avatar Mar 09 '23 22:03 axel-h

I think this can be merged.

Indanz avatar Jun 25 '23 08:06 Indanz

@axel-h this PR should either be closed or merged — happy to merge it if it’s useful for you, otherwise I’d close it.

lsf37 avatar Jun 18 '24 11:06 lsf37