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

libC kconfig: default to PICOLIBC for NATIVE_LIBRARY if POSIX_API or GETOPT #63855

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

aescolar
Copy link
Member

@aescolar aescolar commented Oct 12, 2023

When building with the POSIX_API we cannot use the host libC.
When using GETOPT it is similarly quite difficult for users
to use the host libC.
So to make it easier for users, let's just default to PICOBLIC
in those cases,
while we continue defaulting to the host library in other cases
so users can use the Linux APIs for whatever test functionality
they want.

@keith-packard
Copy link
Collaborator

Is there any case for which we prefer the host C library with native_sim? It seems like we should always use a C library that sits atop Zephyr OS APIs instead of the host C library which sits atop the host OS APIs?

@aescolar
Copy link
Member Author

Is there any case for which we prefer the host C library with native_sim? It seems like we should always use a C library that sits atop Zephyr OS APIs instead of the host C library which sits atop the host OS APIs?

It is a trade off: The argument for always using picolibc is the uniformity with other targets, testing in native_sim being more comparable to testing in real targets, and avoiding developers calling by mistake into the host OS.
The arguments against:

  1. Not all native_posix/sim drivers support building with embedded libraries yet: native drivers and backends not yet supported with embedded C libraries #60096 (This is the biggest limiting factor today)
  2. Quite a few users know when they are calling into the host OS, and want to be able to do it easily (for testing/debugging purposes)
  3. Increased build time (picolibc is built from the module)

When 1. is solved, I would still give the option to build with the host library for 2. and 3., but I would be quite inclined to default to PICOLIBC in native_sim.

When building with the POSIX_API we cannot use the host libC.
When using GETOPT it is similarly quite difficult for users
to use the host libC.
So to make it easier for users, let's just default to PICOBLIC
in those cases,
while we continue defaulting to the host library in other cases
so users can use the Linux APIs for whatever test functionality
they want.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
@aescolar aescolar changed the title libC kconfig: default to PICOLIBC for NATIVE_LIBRARY IF POSIX_API libC kconfig: default to PICOLIBC for NATIVE_LIBRARY if POSIX_API or GETOPT Oct 13, 2023
@aescolar
Copy link
Member Author

I just changed the commit to also default to picolibc for native library when getopt is set, as that config is quite tricky to get working with the host libC

Copy link
Collaborator

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable migration plan.

Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

LGTM

@cfriedt
Copy link
Member

cfriedt commented Oct 15, 2023

The nsi_foo() approach is interesting. I was going to approach that problem slightly differently, but that seems to be working well.

@carlescufi carlescufi merged commit 5f8057e into zephyrproject-rtos:main Oct 20, 2023
18 checks passed
@aescolar aescolar deleted the native_deflibc branch October 20, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C Library C Standard Library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants