Skip to content

makefiles/kconfig: Include configuration symbols to build system#12913

Merged
cgundogan merged 1 commit intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_include_config
Dec 19, 2019
Merged

makefiles/kconfig: Include configuration symbols to build system#12913
cgundogan merged 1 commit intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_include_config

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

Contribution description

Besides the autoconf.h header file, which contains the configurations
synbols in the form of C macros, Kconfig generates a .config file. This file has Makefile syntax and contains the same configuration symbols in the form of variables.

This is useful to check if a certain option is being set. E.g. to decide if a CFLAG should be used to configure something:

ifndef CONFIG_FOO
  CFLAGS += -DCONFIG_FOO=BAR
endif

Note on how this is working

When make all is called, make starts reading all Makefiles. At some point it finds the -include directive for the merged.config file. If the file is there (i.e. it's not a clean build) it will jump to the file and read, if not, it will continue reading kconfig.mk. Once make has read all the files it will try to remake all the included files. For that, it will look for implicit and explicit rules to remake the files. In the particular case of merged.config it will find this:

# Generates a merged configuration file from the given sources. If the config
# file has been edited a '.editedconfig' file will be present.
# This is used to decide if the sources have to be merged or not.
$(KCONFIG_MERGED_CONFIG): $(MERGECONFIG) $(KCONFIG_GENERATED_DEPENDENCIES) FORCE
$(Q)\
if ! test -f $(KCONFIG_EDITED_CONFIG); then \
if ! test -z "$(strip $(MERGE_SOURCES))"; then \
$(MERGECONFIG) $(KCONFIG) $@ $(MERGE_SOURCES); \
else \
rm -f $@; \
fi \
fi

If the file is updated (either because it was not there or because of a change on a source file) make will start from scratch reading all the (now updated) makefiles. This way, the user can check for Kconfig symbols in the application's Makefile (after including Makefile.include).

The problem with clean

In the case make clean all is called merged.config is not included.

Why? The reason is that it depends on the generated folder being created, which in turn depends on $(CLEAN) (which is actually clean if it was called). So if we always included the file, make would try to remake it, it would run clean, and create it again. The it would start over and try to remake it, it would run clean again, updating the file and re-triggering this mechanism forever.

Not including the file on clean has the side effect of not being able to include Kconfig symbols on build, when running make clean all. So far, if users want to use Kconfig they will have to call make clean && make all. Any proposal to solve this is appreciated :-)

Testing procedure

You can add these messages to the tests/kconfig application:

diff --git a/tests/kconfig/Makefile b/tests/kconfig/Makefile
index e386d876b..eb8fcdab8 100644
--- a/tests/kconfig/Makefile
+++ b/tests/kconfig/Makefile
@@ -1,2 +1,8 @@
 include ../Makefile.tests_common
 include $(RIOTBASE)/Makefile.include
+
+ifdef CONFIG_APP_MSG_2
+  $(info "MSG 2 is active")
+else
+  $(info "MSG 2 is not active")
+endif

app.config sets the MSG 2 as active by default.

  • Run make clean && make all, you should see that first MSG 2 is not active and then make starts over with MSG 2 is active.
  • Run make all again, you should only see MSG 2 is active once.
  • Run make menuconfig and disable the message. You should see MSG 2 is not active (once).
  • Run make clean all, as the file is not included you will see MSG 2 is not active.

Issues/PRs references

None

@leandrolanzieri leandrolanzieri added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system TF: Config Marks issues and PRs related to the work of the Configuration Task Force labels Dec 10, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 12, 2019
Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

ACK. Please squash!

@cgundogan cgundogan added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Dec 19, 2019
If the generated configuration file is present include it. That way one
can check if certain symbols are being configured using Kconfig.
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_include_config branch from 21c76a0 to 707ad8d Compare December 19, 2019 14:39
@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 19, 2019
@cgundogan cgundogan merged commit 0d9d496 into RIOT-OS:master Dec 19, 2019
@leandrolanzieri leandrolanzieri deleted the pr/kconfig_include_config branch December 19, 2019 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines TF: Config Marks issues and PRs related to the work of the Configuration Task Force Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants