Allow tests to filter on availability of full C library #54637
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zephyrbot
requested review from
aasthagr,
cfriedt,
nordic-krch,
stephanosio and
yperess
keith-packard
force-pushed
the
filter-on-full-libc
branch
from
262d0f0
to
ec7b025
Compare
yperess
requested changes
Feb 8, 2023
|
Converted to draft; I don't know how to detect NEWLIB support in Kconfig. |
keith-packard
force-pushed
the
filter-on-full-libc
branch
from
ec7b025
to
32a3b4c
Compare
|
Ok, I've added Kconfig symbol |
keith-packard
force-pushed
the
filter-on-full-libc
branch
from
32a3b4c
to
c5ba059
Compare
nordicjm
previously approved these changes
Feb 9, 2023
dcpleung
previously approved these changes
Feb 9, 2023
galak
previously requested changes
Feb 9, 2023
stephanosio
requested changes
Feb 10, 2023
stephanosio
requested changes
Apr 17, 2023
Switch from using a generator expression to computing the value with a conditional so that it is available during the execution of kconfig instead of only being available while generating the build script. Signed-off-by: Keith Packard <[email protected]>
Restructure the file so that the only elements within 'menu "C Library"' are the library choices. Signed-off-by: Keith Packard <[email protected]>
Add a prompt to the Kconfig symbol so that applications can select this in their configuration to guide C library selection away from the minimal C library. Signed-off-by: Keith Packard <[email protected]>
Clean up libc-related symbols to use a common pattern. Signed-off-by: Keith Packard <[email protected]>
This reflects whether newlib is available in the environment. This symbol should be used in place of TOOLCHAIN_HAS_NEWLIB. Signed-off-by: Keith Packard <[email protected]>
Label espressif, gnuarmemd and xtools toolchains with newlib support using the Kconfig variable rather than relying on the TOOLCHAIN_HAS_NEWLIB value. Signed-off-by: Keith Packard <[email protected]>
Before allowing newlib to be selected as the C library, ensure that it is available for the target. Signed-off-by: Keith Packard <[email protected]>
keith-packard
force-pushed
the
filter-on-full-libc
branch
from
8f5fb9a
to
123dd3c
Compare
|
I added one more commit to this PR which will select picolibc as the default C library when a full C library is required and newlib isn't available. Now that crosstool-ng has configurations which provide only picolibc, this will be useful there. |
stephanosio
reviewed
Apr 18, 2023
This symbol is selected when the target has any full libc available. This allows tests to filter on this condition. It doesn't depend on whether the application actually selects that library, only whether requesting a full C library would work. Signed-off-by: Keith Packard <[email protected]>
This symbol detects whether there is any available full libc for a particular target, allowing tests to filter on this condition. Signed-off-by: Keith Packard <[email protected]>
When the only C library available is the minimal one, this test cannot work. Accept only platforms with full C library support to prevent build failures. Signed-off-by: Keith Packard <[email protected]>
These tests all require a full C library to run. Replace the condition checking for either newlib or picolibc with a check for CONFIG_FULL_LIBC_SUPPORTED instead, then add CONFIG_REQUIRES_FULL_LIBC=y to ensure a full libc is used. Signed-off-by: Keith Packard <[email protected]>
Select FULL_LIBC_SUPPORTED when picolibc is available. Add picolibc as a secondary default C library when REQUIRES_FULL_LIBC is selected. This is necessary as tests gated on FULL_LIBC_SUPPORTED need to be sure that a full C library will be selected -- if only picolibc is available, those tests will need to select that. This should permit use of a picolibc-only crosstool-ng toolchain in testing. Signed-off-by: Keith Packard <[email protected]>
keith-packard
force-pushed
the
filter-on-full-libc
branch
from
123dd3c
to
a7a5269
Compare
yperess
approved these changes
Apr 19, 2023
cfriedt
approved these changes
Apr 20, 2023
|
@galak ping |
|
Dismissing @galak's change request since his concern has been addressed (see #54637 (comment)). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some tests are filtering on
not CONFIG_MINIMAL_LIBCand others onTOOLCHAIN_HAS_NEWLIB || CONFIG_PICOLIBC_SUPPORTED, both of which have issues in correctly identifying the desired targets. Instead, create a new hidden Kconfig value,SUPPORT_FULL_LIBC, which currently depends onNATIVE_APPLICATION || TOOLCHAIN_HAS_NEWLIB.When picolibc is added as a default in
LIBC_IMPLEMENTATION,SUPPORT_FULL_LIBCcan be updated which will automagically allow those tests to run using picolibc if available.