Skip to content

RFC: include version.h in the build system #60651

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

RFC: include version.h in the build system #60651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cd506ed
kernel: include version.h
ycsin Jul 20, 2023
657462d
build: Set compiler specific macro inclusion of VERSION_H
ycsin Jul 20, 2023
584c6c0
zephyr: remove version.h
ycsin Jul 20, 2023
56e84ee
samples: an example of supporting multiple version
ycsin Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ endif()

# @Intent: Set compiler specific macro inclusion of AUTOCONF_H
zephyr_compile_options("SHELL: $<TARGET_PROPERTY:compiler,imacros> ${AUTOCONF_H}")
# @Intent: Set compiler specific macro inclusion of VERSION_H
zephyr_compile_options("SHELL: $<TARGET_PROPERTY:compiler,imacros> ${VERSION_H}")
Comment on lines +289 to +290
Copy link
Contributor

@nordicjm nordicjm Jul 21, 2023

Choose a reason for hiding this comment

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

OK with change except for this, should remain an include file which users #include if they need it. Having it in kernel.h I am undecided upon as it seems likely that some codebase or library somewhere will have a conflicting define

Copy link
Member Author

@ycsin ycsin Jul 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This line is the essence of the second way. Do you think name spacing the macros in the version.h (i.e. ZEPHYR_*) would help with mitigating the potential conflict?

Copy link
Contributor

@tejlmand tejlmand Oct 6, 2023

Choose a reason for hiding this comment

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

I don't like this.

Code should include what it needs, so if the code requires version.h, then it should include version.h.
I don't see why version.h is more special than kernel.h, errno.h, or string.h which are includes you find almost everywhere.


if(CONFIG_COMPILER_FREESTANDING)
# @Intent: Set compiler specific flag for bare metal freestanding option
Expand Down Expand Up @@ -552,16 +554,16 @@ if(ZEPHYR_GIT_INDEX)
endif()

add_custom_command(
OUTPUT ${PROJECT_BINARY_DIR}/include/generated/version.h
OUTPUT ${VERSION_H}
COMMAND ${CMAKE_COMMAND} -DZEPHYR_BASE=${ZEPHYR_BASE}
-DOUT_FILE=${PROJECT_BINARY_DIR}/include/generated/version.h
-DOUT_FILE=${VERSION_H}
-DVERSION_TYPE=KERNEL
-DVERSION_FILE=${ZEPHYR_BASE}/VERSION
${build_version_argument}
-P ${ZEPHYR_BASE}/cmake/gen_version_h.cmake
DEPENDS ${ZEPHYR_BASE}/VERSION ${git_dependency}
)
add_custom_target(version_h DEPENDS ${PROJECT_BINARY_DIR}/include/generated/version.h)
add_custom_target(version_h DEPENDS ${VERSION_H})

if(EXISTS ${APPLICATION_SOURCE_DIR}/VERSION)
add_custom_command(
Expand Down Expand Up @@ -1875,7 +1877,7 @@ if(log_dict_db_output)
${ZEPHYR_BASE}/scripts/logging/dictionary/database_gen.py
${KERNEL_ELF_NAME}
${log_dict_db_output}
--build-header ${PROJECT_BINARY_DIR}/include/generated/version.h
--build-header ${VERSION_H}
)
list(APPEND
post_build_byproducts
Expand Down
6 changes: 5 additions & 1 deletion boards/arm/arty/board.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <version.h>
#include <zephyr/arch/arm/aarch32/cortex_m/cmsis.h>
#include <zephyr/device.h>
#include <zephyr/devicetree.h>
Expand Down Expand Up @@ -60,7 +60,11 @@ bool board_daplink_is_fitted(void)
return !NVIC_GetPendingIRQ(DT_IRQN(DAPLINK_QSPI_MUX_NODE));
}

#if ZEPHYR_VERSION_CODE < ZEPHYR_VERSION(3, 4 , 0)
static int board_init(const struct device *arg)
#else
static int board_init(void)
#endif
{

/*
Expand Down
2 changes: 2 additions & 0 deletions cmake/modules/version.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,5 @@ foreach(type file IN ZIP_LISTS VERSION_TYPE VERSION_FILE)
unset(PATCH)
unset(${type}_VERSION_WITHOUT_TWEAK)
endforeach()

set(VERSION_H ${PROJECT_BINARY_DIR}/include/generated/version.h)
Copy link
Contributor

@tejlmand tejlmand Oct 6, 2023

Choose a reason for hiding this comment

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

this is not the right location for this piece of information.

This file is used for CMake and is used both for Zephyr's kernel version, Application version, but can also be used by downstream users.

The intention of this CMake code is to parse a VERSION file (Zephyr, app, downstream project, etc.) and define corresponding CMake variables.

The VERSION_H is a Zephyr code specific header and thus belongs more correctly where it is used today.

Notice also that this call:

zephyr/CMakeLists.txt

Lines 558 to 563 in 657462d

COMMAND ${CMAKE_COMMAND} -DZEPHYR_BASE=${ZEPHYR_BASE}
-DOUT_FILE=${VERSION_H}
-DVERSION_TYPE=KERNEL
-DVERSION_FILE=${ZEPHYR_BASE}/VERSION
${build_version_argument}
-P ${ZEPHYR_BASE}/cmake/gen_version_h.cmake

with your proposal both feeds in VERSION_H as the OUT_FILE argument, but that the script gen_version_h.cmake actually calls on to version.cmake where you just made that piece on info directly available.

zephyr/cmake/gen_version_h.cmake

Line 32 in 9e8d609

include(${ZEPHYR_BASE}/cmake/modules/version.cmake)

Suggested change
set(VERSION_H ${PROJECT_BINARY_DIR}/include/generated/version.h)

1 change: 0 additions & 1 deletion kernel/banner.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <zephyr/kernel.h>
#include <zephyr/init.h>
#include <zephyr/device.h>
#include <version.h>

#if defined(CONFIG_BOOT_DELAY) && (CONFIG_BOOT_DELAY > 0)
#define DELAY_STR STRINGIFY(CONFIG_BOOT_DELAY)
Expand Down
4 changes: 2 additions & 2 deletions kernel/version.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
*/

#include <zephyr/types.h>
#include "version.h" /* generated by MAKE, at compile time */
#include <zephyr/kernel.h>

/**
* @brief Return the kernel version of the present build
*
* The kernel version is a four-byte value, whose format is described in the
* file "kernel_version.h".
* file "version.h".
*
* @return kernel version
*/
Expand Down
1 change: 0 additions & 1 deletion lib/posix/uname.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include "version.h"
#include <string.h>
#include <zephyr/kernel.h>
#include <zephyr/net/hostname.h>
Expand Down
1 change: 0 additions & 1 deletion samples/subsys/shell/shell_module/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <zephyr/kernel.h>
#include <zephyr/shell/shell.h>
#include <version.h>
#include <zephyr/logging/log.h>
#include <stdlib.h>
#include <zephyr/drivers/uart.h>
Expand Down
1 change: 0 additions & 1 deletion subsys/bluetooth/controller/hci/hci.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <stddef.h>
#include <string.h>

#include <version.h>
#include <errno.h>

#include <zephyr/sys/util.h>
Expand Down
1 change: 0 additions & 1 deletion subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

#ifdef CONFIG_MCUMGR_GRP_OS_INFO
#include <stdio.h>
#include <version.h>
#include <os_mgmt_processor.h>
#include <mgmt/mcumgr/util/zcbor_bulk.h>
#if defined(CONFIG_NET_HOSTNAME_ENABLE)
Expand Down
1 change: 0 additions & 1 deletion subsys/net/l2/openthread/openthread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ LOG_MODULE_REGISTER(net_l2_openthread, CONFIG_OPENTHREAD_L2_LOG_LEVEL);
#include <zephyr/sys/slist.h>
#include <zephyr/sys/util.h>
#include <zephyr/sys/__assert.h>
#include <version.h>

#include <openthread/cli.h>
#include <openthread/ip6.h>
Expand Down
1 change: 0 additions & 1 deletion tests/kernel/common/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <zephyr/ztest.h>
#include <zephyr/kernel_version.h>
#include <zephyr/sys/speculation.h>
#include "version.h"

/**
* @defgroup kernel_common_tests Common Tests
Expand Down
1 change: 0 additions & 1 deletion tests/subsys/mgmt/mcumgr/cb_notifications/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <zcbor_decode.h>
#include <zcbor_encode.h>
#include <mgmt/mcumgr/util/zcbor_bulk.h>
#include <version.h>
#include <string.h>
#include <smp_internal.h>
#include "smp_test_util.h"
Expand Down
1 change: 0 additions & 1 deletion tests/subsys/mgmt/mcumgr/os_mgmt_info/src/build_date.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <zcbor_decode.h>
#include <zcbor_encode.h>
#include <mgmt/mcumgr/util/zcbor_bulk.h>
#include <version.h>
#include <smp_internal.h>
#include "smp_test_util.h"

Expand Down
1 change: 0 additions & 1 deletion tests/subsys/mgmt/mcumgr/os_mgmt_info/src/limited.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <zcbor_decode.h>
#include <zcbor_encode.h>
#include <mgmt/mcumgr/util/zcbor_bulk.h>
#include <version.h>
#include <string.h>
#include <smp_internal.h>
#include "smp_test_util.h"
Expand Down
1 change: 0 additions & 1 deletion tests/subsys/mgmt/mcumgr/os_mgmt_info/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <zcbor_decode.h>
#include <zcbor_encode.h>
#include <mgmt/mcumgr/util/zcbor_bulk.h>
#include <version.h>
#include <string.h>
#include <smp_internal.h>
#include "smp_test_util.h"
Expand Down