Skip to content
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

fix: add module name #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Jappie3
Copy link

@Jappie3 Jappie3 commented Apr 17, 2024

This MR adds the module name in zephyr/module.yml, as is stated in the official documentation:

Each Zephyr module is given a name by which it can be referred to in the build system.
The name should be specified in the zephyr/module.yml file. This will ensure the module name is not changeable through user-defined directory names or west manifest files.

While building ZMK, I ran into trouble with this module because it did not have a name, adding the name fixed the problem.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

While building ZMK, I ran into trouble with this module because it did not have a name, adding the name fixed the problem.

Could you elaborate on this?

When name is not specified, the default behaviour is to use the directory name.

If the name field is not specified then the Zephyr module name will be set to the name of the module folder. As example, the Zephyr module located in /modules/bar will use bar as its module name if nothing is specified in zephyr/module.yml.

@Jappie3
Copy link
Author

Jappie3 commented Apr 23, 2024

Could you elaborate on this?

Of course: I tried packaging my ZMK config without the use of West & provided all the necessary modules (under which cmsis). A few modules acted up, and the common denominator was that they did not specify the module name.

Here is an excerpt from one of the logs I saved:

       > Parsing /build/4dv12abrasy7w8yk45icqffy13mba6yb-source/Kconfig
       > /build/4dv12abrasy7w8yk45icqffy13mba6yb-source/build/zephyr/scripts/kconfig/kconfig.py: /build/4dv12abrasy7w8yk45icqffy13mba6yb-source/build/build/Kconfig/Kconfig.modules:128: Could not open '/build/4dv12abrasy7w8yk45icqffy13mba6yb-source/build/zephyr/' (in 'osource "$(ZEPHYR_C2XYWJYVZ71LFDCNBC6AJLHGVXAD7DBN_SOURCE_KCONFIG)"') (EISDIR: Is a directory)
       > CMake Error at build/zephyr/cmake/modules/kconfig.cmake:355 (message):
       >   command failed with return code: 1
       > Call Stack (most recent call first):
       >   build/zephyr/cmake/modules/zephyr_default.cmake:129 (include)
       >   build/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
       >   build/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:159 (include_boilerplate)
       >   CMakeLists.txt:8 (find_package)

If we take a look at /build/4dv12abrasy7w8yk45icqffy13mba6yb-source/build/build/Kconfig/Kconfig.modules, we see the following (excerpt, not the entire file):

menu "thrift (/nix/store/nc6rqsgzhfp6ikgbl66yv3jhspv2b6m4-source)"
osource "$(ZEPHYR_THRIFT_KCONFIG)"
config ZEPHYR_THRIFT_MODULE
	bool
	default y
endmenu
menu "zscilib (/nix/store/isz10qyclvz7cyrxjw1y4ydwbvccvpcg-source)"
osource "/nix/store/isz10qyclvz7cyrxjw1y4ydwbvccvpcg-source/Kconfig.zscilib"
config ZEPHYR_ZSCILIB_MODULE
	bool
	default y
endmenu
menu "acpica (/nix/store/b8rcgxy2z7bdcws69wglcwwi0srswggk-source)"
osource "$(ZEPHYR_ACPICA_KCONFIG)"
config ZEPHYR_ACPICA_MODULE
	bool
	default y
endmenu
menu "byrm8j6alc6p1x6pw4wxfjccdj770ll1-source (/nix/store/byrm8j6alc6p1x6pw4wxfjccdj770ll1-source)"
osource "$(ZEPHYR_BYRM8J6ALC6P1X6PW4WXFJCCDJ770LL1_SOURCE_KCONFIG)"
config ZEPHYR_BYRM8J6ALC6P1X6PW4WXFJCCDJ770LL1_SOURCE_MODULE
	bool
	default y
endmenu
menu "cmsis-dsp (/nix/store/cz6s7pxbjka4yy3inap9wbwhjl9fnrnf-source)"
osource "$(ZEPHYR_CMSIS_DSP_KCONFIG)"
config ZEPHYR_CMSIS_DSP_MODULE
	bool
	default y
endmenu
menu "cmsis-nn (/nix/store/412shprv7ixbdwmidwqfz2hasha3f13c-source)"
osource "$(ZEPHYR_CMSIS_NN_KCONFIG)"
config ZEPHYR_CMSIS_NN_MODULE
	bool
	default y
endmenu
menu "fatfs (/nix/store/c1bzshix705v2srm0hvzwyiq7r33kpcm-source)"
osource "$(ZEPHYR_FATFS_KCONFIG)"
config ZEPHYR_FATFS_MODULE
	bool
	default y
endmenu

cmsis is clearly the odd one out (all other modules use my patched forks in this example).

Like you said, it just defaulted to the directory name if name is not specified, but unfortunately that causes problems in this scenario (on NixOS when using CMake instead of West) and there's no easy solution to that (other than some very ugly hacks).

@Jappie3

This comment was marked as outdated.

@Zitrax
Copy link

Zitrax commented Aug 19, 2024

I seem to also have hit this issue although with different symptoms, we are using zephyr purely using CMake (no west) so we are setting the ZEPHYR_MODULES variable to point to cmsis among others before calling: find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) I then get this:

kconfiglib._KconfigIOError: <…>/Kconfig/Kconfig.modules:8: Could not open '<…>\_deps\zephyr-rtos-src\' (in 'osource "$(ZEPHYR_ZEPHYR_CMSIS_SRC_KCONFIG)"') (ENOENT: No such file or directory)

Looking at the generated Kconfig.modules it looks like this:

menu "hal_nordic (<…>/_deps/zephyr-hal-nordic-src)"
osource "$(ZEPHYR_HAL_NORDIC_KCONFIG)"
config ZEPHYR_HAL_NORDIC_MODULE
	bool
	default y
endmenu
menu "zephyr-cmsis-src (<…>/_deps/zephyr-cmsis-src)"
osource "$(ZEPHYR_ZEPHYR_CMSIS_SRC_KCONFIG)"
config ZEPHYR_ZEPHYR_CMSIS_SRC_MODULE
	bool
	default y
endmenu
⋮
⋮

As can be seen the cmsis module has it's variable name double-prefixed with ZEPHYR_ZEPHYR_ which only happen for the cmsis module and none of the others in our setup. (I hit this when updating zephyr from 3.3.0 to 3.7.0)

Adding the name just as in this patch the issue goes away and the double prefix change to a single.

(I have replaced my local paths with <…>)

@Jappie3
Copy link
Author

Jappie3 commented Sep 1, 2024

@stephanosio friendly reminder that this MR is still open, feel free to lmk if there are any problems with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants