Skip to content

lib: libc: Kconfig: Correct dependencies for MINIMAL_LIBC_SUPPORTED #81410

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
anangl wants to merge 1 commit into from
Closed

lib: libc: Kconfig: Correct dependencies for MINIMAL_LIBC_SUPPORTED #81410

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/libc/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ config FULL_LIBC_SUPPORTED
config MINIMAL_LIBC_SUPPORTED
bool
depends on !NATIVE_APPLICATION
depends on !REQUIRES_FULL_LIBC
Copy link
Member

@aescolar aescolar Nov 14, 2024

Choose a reason for hiding this comment

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

I don't think this is correct. MINIMAL_LIBC_SUPPORTED indicates the target itself supports the library.
REQUIRES_FULL_LIBC indicates an application or feature requires a full libc. Which has nothing to do with the target supporting or not minimallibc.

Copy link
Member Author

@anangl anangl Nov 14, 2024

Choose a reason for hiding this comment

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

Well, nrf9131ek/nrf9131/ns is an example of a target that selects REQUIRES_FULL_LIBC (because of NPM1300_CHARGER) and this generates a build warning when the libraries.libc.common.minimal scenario is executed for the tests/lib/c_lib/common test.

Copy link
Member

@aescolar aescolar Nov 14, 2024

Choose a reason for hiding this comment

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

should that board kconfig.defconfig have a

config MINIMAL_LIBC_SUPPORTED
	default n

?

Copy link
Member Author

@anangl anangl Nov 15, 2024

Choose a reason for hiding this comment

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

This would only solve the problem for this one board, but similar scenario may occur for others. I wanted to provide a generic solution and I thought that such dependency would be fine, as I saw that PICOLIBC_SUPPORTED has dependency on !REQUIRES_FULL_LIBCPP:

zephyr/lib/libc/Kconfig

Lines 45 to 48 in 5f418f5

config PICOLIBC_SUPPORTED
bool
depends on !NATIVE_APPLICATION
depends on ("$(TOOLCHAIN_HAS_PICOLIBC)" = "y") || (ZEPHYR_PICOLIBC_MODULE && !REQUIRES_FULL_LIBCPP)

Copy link
Member

@aescolar aescolar Nov 15, 2024

Choose a reason for hiding this comment

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

That pico one is different, as you can't build from the source module today if you need the c++ std lib.

Anyhow, to me it seems we should just set MINIMAL_LIBC_SUPPORTED to n for that board, and others which may just not be able to be built with minimal.
But if others think otherwise I'm not going to oppose this change.

default y
help
Selected when the target has support for the minimal C library
Expand Down Expand Up @@ -68,7 +69,6 @@ choice LIBC_IMPLEMENTATION

config MINIMAL_LIBC
bool "Minimal C library"
depends on !REQUIRES_FULL_LIBC
depends on MINIMAL_LIBC_SUPPORTED
imply COMPILER_FREESTANDING
select COMMON_LIBC_ABORT
Expand Down
Loading