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 1 commit
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
1 change: 1 addition & 0 deletions include/zephyr/kernel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#define ZEPHYR_INCLUDE_KERNEL_H_

#if !defined(_ASMLANGUAGE)
#include "version.h" /* generated by MAKE, at compile time */
Copy link
Contributor

@tejlmand tejlmand Oct 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not fond of this.

kernel.h itself doesn't require anything / provide anything related to version, so I fail to see why it should suddenly start to include a header so that other parts of code can save a single include line.

Doing this will have a severe impact on incremental builds.
On incremental builds, then only code that contains actual changes (or headers they include) will rebuild if there is a need.
But including version.h in kernel.h means that everyone will depend on version.h, and thus commiting a foo.txt, unrelated to other files will cause everything to rebuild.

Today:

$ cmake -GNinja -DBOARD=nrf52840dk_nrf52840 -Bbuild samples/hello_world
$ ninja -C build
$ ninja: Entering directory `build'
...
[134/134] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       18152 B         1 MB      1.73%
             RAM:        5580 B       256 KB      2.13%
        IDT_LIST:          0 GB         2 KB      0.00%
$ ninja -C build
ninja: Entering directory `build'
ninja: no work to do.
$ touch foo.txt; git add foo.txt; git commit -s -m"File added"
$ ninja -C build
ninja: Entering directory `build'
[1/9] Generating include/generated/version.h
-- Zephyr version: 3.5.0-rc1 (/projects/github/ncs/zephyr), build: v3.5.0-rc1-45-gb59168b4833f
[9/9] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       18152 B         1 MB      1.73%
             RAM:        5580 B       256 KB      2.13%
        IDT_LIST:          0 GB         2 KB      0.00%

Note, that the new file causes a new commit, which generates a new version.h, but only files actually using version.h needs to be recompiled, and thus linked.

Just 9 build step.

If kernel.h includes version.h, then almost everything will rebuild.
Result of doing the #include "version.h" change in my repo,

$ ninja -C build
ninja: Entering directory `build'
ninja: no work to do.
$ touch foo.txt; git add foo.txt; git commit -s -m"File added"
$ ninja -C build;
ninja: Entering directory `build'
[1/89] Generating include/generated/version.h
-- Zephyr version: 3.5.0-rc1 (/projects/github/ncs/zephyr), build: v3.5.0-rc1-45-gdf07a813c711
[89/89] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       18152 B         1 MB      1.73%
             RAM:        5580 B       256 KB      2.13%
        IDT_LIST:          0 GB         2 KB      0.00%

#include <zephyr/kernel_includes.h>
#include <errno.h>
#include <limits.h>
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