-
Notifications
You must be signed in to change notification settings - Fork 148
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
[build] - Centralize around a single build system: CMake #606
Comments
I like the idea of centralizing around a single build system a lot. The primary concern I have is that it may appear more complex to the user for their application build files. For example, a current minimal Makefile for an application may look like: # Project Name
TARGET = Blink
# Sources
CPP_SOURCES = main.cpp
LIBDAISY_DIR = ../libs/libDaisy
DAISYSP_DIR = ../libs/DaisySP
# (optional) Includes FatFS source files within project.
#USE_FATFS = 1
# Core location, and generic Makefile.
SYSTEM_FILES_DIR = $(LIBDAISY_DIR)/core
include $(SYSTEM_FILES_DIR)/Makefile A similarly simple project(Blink)
cmake_minimum_required(VERSION 3.19)
set(FIRMWARE_NAME Blink)
set(FIRMWARE_SOURCES
src/main.cpp
)
# include libDaisy
set(LIBDAISY_DIR ../libs/libDaisy)
add_subdirectory(${LIBDAISY_DIR} libDaisy)
# include DaisySP
set(DAISYSP_DIR ../libs/DaisySP)
add_subdirectory(${DAISYSP_DIR} DaisySP)
# linker script
get_filename_component(LIBDAISY_DIR_ABS "${LIBDAISY_DIR}"
REALPATH BASE_DIR "${CMAKE_SOURCE_DIR}")
set(LINKER_SCRIPT ${LIBDAISY_DIR_ABS}/core/STM32H750IB_flash.lds)
# firmware binary image
add_executable(${FIRMWARE_NAME} ${FIRMWARE_SOURCES})
target_link_libraries(${FIRMWARE_NAME}
PRIVATE
daisy
DaisySP
)
set_target_properties(${FIRMWARE_NAME} PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED YES
SUFFIX ".elf"
LINK_DEPENDS ${LINKER_SCRIPT}
)
target_link_options(${FIRMWARE_NAME} PUBLIC
-T ${LINKER_SCRIPT}
-Wl,-Map=${FIRMWARE_NAME}.map,--cref
-Wl,--check-sections
-Wl,--unresolved-symbols=report-all
-Wl,--warn-common
-Wl,--warn-section-align
-Wl,--print-memory-usage
)
add_custom_command(TARGET ${FIRMWARE_NAME} POST_BUILD
COMMAND ${CMAKE_OBJCOPY}
ARGS -O ihex
-S ${FIRMWARE_NAME}.elf
${FIRMWARE_NAME}.hex
BYPRODUCTS
${FIRMWARE_NAME}.hex
COMMENT "Generating HEX image"
VERBATIM)
add_custom_command(TARGET ${FIRMWARE_NAME} POST_BUILD
COMMAND ${CMAKE_OBJCOPY}
ARGS -O binary
-S ${FIRMWARE_NAME}.elf
${FIRMWARE_NAME}.bin
BYPRODUCTS
${FIRMWARE_NAME}.bin
COMMENT "Generating binary image"
VERBATIM) There could be a similar way to the It's also possible that a well commented template project (similar to what you might see when building a project with JUCE (e.g. Definitely open to thoughts, as well as any other things that are missing, worth discussing! |
yeah the core/Makefile style system is what I went with in my cmake/DaisyProject.cmake: set(DAISY_STORAGE flash CACHE STRING "Select the storage destination of the executable.")
if(DEFINED CUSTOM_LINKER_SCRIPT)
set(LINKER_SCRIPT ${CUSTOM_LINKER_SCRIPT})
else()
cmake_path(SET LINKER_SCRIPT NORMALIZE ${CMAKE_CURRENT_LIST_DIR}/../core/STM32H750IB_${DAISY_STORAGE}.lds)
endif()
add_executable(${FIRMWARE_NAME} ${FIRMWARE_SOURCES})
target_link_libraries(${FIRMWARE_NAME}
PRIVATE
daisy
${DAISYSP_LIB}
)
set_target_properties(${FIRMWARE_NAME} PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED YES
SUFFIX ".elf"
LINK_DEPENDS ${LINKER_SCRIPT}
)
target_link_options(${FIRMWARE_NAME} PUBLIC
LINKER:-T,${LINKER_SCRIPT}
$<$<CONFIG:DEBUG>:LINKER:-Map=${FIRMWARE_NAME}.map>
$<$<CONFIG:DEBUG>:LINKER:--cref>
LINKER:--gc-sections
LINKER:--check-sections
LINKER:--unresolved-symbols=report-all
LINKER:--warn-common
$<$<CXX_COMPILER_ID:GNU>:LINKER:--warn-section-align>
# Currently a GSoC project to port this to LLD
$<$<CXX_COMPILER_ID:GNU>:LINKER:--print-memory-usage>
)
add_custom_command(TARGET ${FIRMWARE_NAME} POST_BUILD
COMMAND ${CMAKE_OBJCOPY} -O ihex -S ${FIRMWARE_NAME}.elf ${FIRMWARE_NAME}.hex
BYPRODUCTS ${FIRMWARE_NAME}.hex
COMMENT "Generating HEX image"
VERBATIM
)
option(DAISY_GENERATE_BIN "Sets whether or not to generate a raw binary image using objcopy (warning this is a very large file, as it is a representation of the *full memory space*.)")
if(DAISY_GENERATE_BIN)
add_custom_command(TARGET ${FIRMWARE_NAME} POST_BUILD
COMMAND ${CMAKE_OBJCOPY} -O binary -S ${FIRMWARE_NAME}.elf ${FIRMWARE_NAME}.bin
BYPRODUCTS ${FIRMWARE_NAME}.bin
COMMENT "Generating binary image"
VERBATIM
)
endif(DAISY_GENERATE_BIN) and then subsequent projects can just use this like so: # Only required if not a subdir of another project
#cmake_minimum_required(VERSION 3.20)
#project(PROJECT_NAME_HERE)
set(FIRMWARE_NAME examples_Encoder)
set(FIRMWARE_SOURCES Encoder.cpp)
include(DaisyProject) |
Maintaining two build systems is a hassle and can lead to problems. Make and CMake ultimately accomplish the same thing, but CMake offers far more flexibility in terms of project organization and integration into other projects as a library. From what I understand the CMake system is newer, and is potentially a migration target, but (much like with the GPIO/Pin situation) currently in an awkward middle state.
Submodules are not the greatest way to do dependency management. CMake now has a very nice built-in dependency download and integration system called FetchContent that is well suited for problems like this: downloading weird out-of-source build components and integrating them without polluting the version control system. I've used this in a very similar situation for downloading and integrating AVR Dx devicepacks into a project's AVR-GCC toolchain, for example.
In the long run migrating the current build system to more idiomatic CMake is probably a good path forward, and will reduce a lot of issues with other kinds of integrations such as alternate toolchains (I've had a Clang-support PR waiting in the wings for a while now). However in the short term I think submodules are an acceptable stopgap solution for the dependency version problem, even if it requires an additional VCS step in the form of
git submodule update
.I also kind of just want to see this PR merged so I can work on others, haha 😅
Originally posted by @stellar-aria in #582 (comment)
The text was updated successfully, but these errors were encountered: