Skip to content

scripts: kconfig: Replace conf with Python #5569

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

Merged
merged 6 commits into from
Jan 12, 2018
Merged

scripts: kconfig: Replace conf with Python #5569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
812c8a7
scripts: kconfig: Import Kconfiglib
carlescufi Jan 7, 2018
e7426e3
scripts: kconfig: Add support for wildcards and globbing
carlescufi Jan 7, 2018
e1b4229
scripts: kconfig: Prefer later defaults
carlescufi Jan 7, 2018
0a96841
scripts: kconfig: Replace Kconfig exectuables with Python
carlescufi Jan 3, 2018
c067079
cmake: kconfig: Run Kconfig on every reconfiguration
SebastianBoe Jan 8, 2018
13547f8
kconfig: Prune kconfig files that don't match the ARCH or BOARD
SebastianBoe Jan 9, 2018
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
12 changes: 10 additions & 2 deletions Kconfig.zephyr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ config KERNELVERSION
string
option env="KERNELVERSION"

config ENV_VAR_SYM_BOARD_DIR
string
option env="ENV_VAR_BOARD_DIR"

config ENV_VAR_SYM_ARCH
string
option env="ENV_VAR_ARCH"

source "arch/Kconfig"

source "kernel/Kconfig"
Expand Down Expand Up @@ -37,5 +45,5 @@ source "tests/Kconfig"
# Board defaults should be parsed after SoC defaults
# because board usually overrides SoC values.
#
source "arch/*/soc/*/Kconfig.defconfig"
source "boards/*/*/Kconfig.defconfig"
source "arch/$ENV_VAR_SYM_ARCH/soc/*/Kconfig.defconfig"
source "$ENV_VAR_SYM_BOARD_DIR/Kconfig.defconfig"
2 changes: 1 addition & 1 deletion arch/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,6 @@ config BOARD
arch/<arch>/soc/<family>/<series>


source "arch/*/Kconfig"
source "arch/$ENV_VAR_SYM_ARCH/Kconfig"

source "boards/Kconfig"
4 changes: 2 additions & 2 deletions boards/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ config QEMU_TARGET

choice
prompt "Board Selection"
source "boards/*/*/Kconfig.board"
source "$ENV_VAR_SYM_BOARD_DIR/Kconfig.board"
endchoice


menu "Board Options"
source "boards/*/*/Kconfig"
source "$ENV_VAR_SYM_BOARD_DIR/Kconfig"
endmenu
14 changes: 8 additions & 6 deletions cmake/host-tools.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ if(${DTC} STREQUAL DTC-NOTFOUND)
message(FATAL_ERROR "Unable to find dtc")
endif()

find_program(
KCONFIG_CONF
conf
)
if(${KCONFIG_CONF} STREQUAL KCONFIG_CONF-NOTFOUND)
message(FATAL_ERROR "Unable to find the Kconfig program 'conf'")
if (NOT WIN32)
Copy link
Member Author

Choose a reason for hiding this comment

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

Keep looking for conf in non-Windows builds, including MSYS2.

find_program(
KCONFIG_CONF
conf
)
if(${KCONFIG_CONF} STREQUAL KCONFIG_CONF-NOTFOUND)
message(FATAL_ERROR "Unable to find the Kconfig program 'conf'")
endif()
endif()

find_program(
Expand Down
120 changes: 42 additions & 78 deletions cmake/kconfig.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,34 @@ set(COMMAND_FOR_menuconfig ${KCONFIG_MCONF} ${KCONFIG_ROOT})
set(COMMAND_FOR_oldconfig ${KCONFIG_CONF} --oldconfig ${KCONFIG_ROOT})
set(COMMAND_FOR_xconfig qconf ${KCONFIG_ROOT})

# Set environment variables so that Kconfig can prune Kconfig source
# files for other architectures
set(ENV{ENV_VAR_ARCH} ${ARCH})
set(ENV{ENV_VAR_BOARD_DIR} ${BOARD_DIR})

foreach(kconfig_target ${kconfig_target_list})
add_custom_target(
${kconfig_target}
${CMAKE_COMMAND} -E env
srctree=${PROJECT_SOURCE_DIR}
KERNELVERSION=${PROJECT_VERSION}
KCONFIG_CONFIG=${DOTCONFIG}
${COMMAND_FOR_${kconfig_target}}
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/kconfig
USES_TERMINAL
)
if (NOT WIN32)
Copy link
Member Author

Choose a reason for hiding this comment

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

@SebastianBoe does this look OK to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

add_custom_target(
${kconfig_target}
${CMAKE_COMMAND} -E env
srctree=${PROJECT_SOURCE_DIR}
KERNELVERSION=${PROJECT_VERSION}
KCONFIG_CONFIG=${DOTCONFIG}
ENV_VAR_ARCH=$ENV{ENV_VAR_ARCH}
ENV_VAR_BOARD_DIR=$ENV{ENV_VAR_BOARD_DIR}
${COMMAND_FOR_${kconfig_target}}
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/kconfig
USES_TERMINAL
)
else()
add_custom_target(
${kconfig_target}
${CMAKE_COMMAND} -E echo
"========================================="
"Reconfiguration not supported on Windows."
"========================================="
)
endif()
endforeach()

# Bring in extra configuration files dropped in by the user or anyone else;
Expand Down Expand Up @@ -81,80 +98,27 @@ foreach(f ${merge_config_files_with_absolute_paths})
endif()
endforeach()

# Calculate a checksum of merge_config_files to determine if we need
# to re-generate .config
set(merge_config_files_checksum "")
foreach(f ${merge_config_files_with_absolute_paths})
file(MD5 ${f} checksum)
set(merge_config_files_checksum "${merge_config_files_checksum}${checksum}")
endforeach()

# Create a new .config if it does not exists, or if the checksum of
# the dependencies has changed
set(merge_config_files_checksum_file ${PROJECT_BINARY_DIR}/.cmake.dotconfig.checksum)
set(CREATE_NEW_DOTCONFIG "")
if(NOT EXISTS ${DOTCONFIG})
set(CREATE_NEW_DOTCONFIG 1)
else()
# Read out what the checksum was previously
file(READ
${merge_config_files_checksum_file}
merge_config_files_checksum_prev
)
set(CREATE_NEW_DOTCONFIG 1)
if(
${merge_config_files_checksum} STREQUAL
${merge_config_files_checksum_prev}
)
set(CREATE_NEW_DOTCONFIG 0)
endif()
endif()
if(CREATE_NEW_DOTCONFIG)
execute_process(
COMMAND
${PROJECT_SOURCE_DIR}/scripts/kconfig/merge_config.sh
-m
-q
-O ${PROJECT_BINARY_DIR}
${merge_config_files}
WORKING_DIRECTORY ${APPLICATION_SOURCE_DIR}
# The working directory is set to the app dir such that the user
# can use relative paths in CONF_FILE, e.g. CONF_FILE=nrf5.conf
RESULT_VARIABLE ret
)
if(NOT "${ret}" STREQUAL "0")
message(FATAL_ERROR "command failed with return code: ${ret}")
endif()

execute_process(
COMMAND ${KCONFIG_CONF}
--olddefconfig
${KCONFIG_ROOT}
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/kconfig
RESULT_VARIABLE ret
)
if(NOT "${ret}" STREQUAL "0")
message(FATAL_ERROR "command failed with return code: ${ret}")
endif()

file(WRITE
${merge_config_files_checksum_file}
${merge_config_files_checksum}
)
endif()

# Force CMAKE configure when the configuration files changes.
foreach(merge_config_input ${merge_config_files} ${DOTCONFIG})
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${merge_config_input})
endforeach()

message(STATUS "Generating zephyr/include/generated/autoconf.h")
execute_process(
COMMAND ${KCONFIG_CONF}
--silentoldconfig
${KCONFIG_ROOT}
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/kconfig
)
COMMAND
${PYTHON_EXECUTABLE}
${PROJECT_SOURCE_DIR}/scripts/kconfig/kconfig.py
${KCONFIG_ROOT}
${PROJECT_BINARY_DIR}/.config
${PROJECT_BINARY_DIR}/include/generated/autoconf.h
${merge_config_files}
WORKING_DIRECTORY ${APPLICATION_SOURCE_DIR}
# The working directory is set to the app dir such that the user
# can use relative paths in CONF_FILE, e.g. CONF_FILE=nrf5.conf
RESULT_VARIABLE ret
)
if(NOT "${ret}" STREQUAL "0")
message(FATAL_ERROR "command failed with return code: ${ret}")
endif()

add_custom_target(config-sanitycheck DEPENDS ${DOTCONFIG})

Expand Down
50 changes: 50 additions & 0 deletions scripts/kconfig/kconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env python3
# Modified from: https://github.com/ulfalizer/Kconfiglib/blob/master/examples/merge_config.py
Copy link
Contributor

Choose a reason for hiding this comment

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

And thus ISC license as well? Very small script so perhaps less of a concern?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, ISC as well, so that is not an issue.

from kconfiglib import Kconfig, Symbol, BOOL, STRING, TRISTATE, TRI_TO_STR
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to keep things tidy and reviewable since this is not minimal, I think putting kconfiglib.py in ext/ (one patch to add the upstream copy, another with your changes) would be appropriate.

You can explicitly add os.path.join(os.path.abspath(os.environ['ZEPHYR_BASE']), 'ext', 'subdir', 'containing', 'kconfiglib') to sys.path before performing the import to ensure it's visible as done here:

https://github.com/zephyrproject-rtos/zephyr/blob/master/doc/conf.py#L21

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure about that because ext/ is supposed to hold unmodified copies of external repos. Instead I suggest we fix our Kconfig structure and then use pip to install Kconfiglib at a later date.

Copy link
Member Author

Choose a reason for hiding this comment

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

@nashif can you weigh in?

Copy link
Member

@nashif nashif Jan 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

ext/ is for stuff that ends up in the binary, external components and middleware, we never put original kconfig in ext/ and we also never put kbuild, kconfig is part of the build and configuration system and IMO should be in scripts/ just like it used to be before.

btw, the issue here: if I get some random script that was not originally written for zephyr and start maintaining such scripts in different locations such as ext/scriptA, ext/scriptB this will become ugly really fast. We lose the basic structure we somehow have now and bits and pieces will be all over the place because of some license difference. Makes sense for some c code that ends up in the binary, not for scripts.

Copy link
Member

Choose a reason for hiding this comment

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

and the other thing, we want to get rid of this eventually anyways and use the upstream version, so it will go away sooner than later.

Copy link
Contributor

Choose a reason for hiding this comment

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

ext/ is for stuff that ends up in the binary, external components and middleware, we never put original kconfig in ext/ and we also never put kbuild, kconfig is part of the build and configuration system and IMO should be in scripts/ just like it used to be before.

Okay. Should I send patches to the documentation to make this more clear?

Copy link
Member Author

@carlescufi carlescufi Jan 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think you raise good points @mbolivar, but I still think kconfiglib.py should live in scripts/kconfig.

That's not correct across the board.

You are right, and the Nordic ones are definitely incorrect oversights that will be fixed soon once we replace what we have now with nrfx.

Commit template for external components:

Yes, I did not follow that strictly because I assumed that was only valid for ext/ :) bit of a catch-22 here.

Requirement that non-Apache lives in ext/:

I think that this applies only to firmware, i.e. source code that once compiled ends up in the image to flash.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW @mbolivar we even have GPL code in scripts/kconfig! https://github.com/zephyrproject-rtos/zephyr/blob/master/scripts/kconfig/gconf.c#L5

Copy link
Member

Choose a reason for hiding this comment

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

kconfiglib.py ALREADY exists in the tree, this is just an update to an existing library we already have. We did not remove the old one because there is another script that depend on it for generating docs, but this should be done in parallel and be ready before this PR is merged. I started looking at that.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK OK, my mistake :).

I'm just going to send a docs patch to try to clear this up, we can discuss that there

import sys

if len(sys.argv) < 5:
print('usage: {} Kconfig dotconfig autoconf conf1 [conf2 ...]'
.format(sys.argv[0]))
sys.exit(1)

print("Parsing Kconfig tree in {}".format(sys.argv[1]))
kconf = Kconfig(sys.argv[1])

# Enable warnings for assignments to undefined symbols
kconf.enable_undef_warnings()

# (This script uses alldefconfig as the base. Other starting states could be
# set up here as well. The approach in examples/allnoconfig_simpler.py could
# provide an allnoconfig starting state for example.)

print("Using {} as base".format(sys.argv[4]))
for config in sys.argv[5:]:
print("Merging {}".format(config))
# Create a merged configuration by loading the fragments with replace=False
for config in sys.argv[4:]:
kconf.load_config(config, replace=False)

# Write the merged configuration
kconf.write_config(sys.argv[2])
Copy link
Member Author

Choose a reason for hiding this comment

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

We can replace the header of the .config file if we want to


# Output autoconf
kconf.write_autoconf(sys.argv[3])
Copy link
Member Author

Choose a reason for hiding this comment

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

We can replace the header of the autoconf.h file if we want to


# Print warnings for symbols whose actual value doesn't match the assigned
# value
for sym in kconf.defined_syms:
# Was the symbol assigned to?
#print('name: {} value: {}'.format(sym.name, sym.str_value))
if sym.user_value is not None:
# Tristate values are represented as 0, 1, 2. Having them as
# "n", "m", "y" is more convenient here, so convert.
if sym.type in (BOOL, TRISTATE):
user_value = TRI_TO_STR[sym.user_value]
else:
user_value = sym.user_value
if user_value != sym.str_value:
print('warning: {} was assigned the value "{}" but got the '
'value "{}" -- check dependencies'
.format(sym.name, user_value, sym.str_value))

Loading