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

samples: http_get: move 'not CONFIG_NATIVE_LIBC' filter to POSIX build #69242

Merged

Conversation

mniestroj
Copy link
Member

not CONFIG_NATIVE_LIBC filter is just to prevent building for
native_posix platform with additional options selected by
sample.net.sockets.http_get.posix testcase.

'not CONFIG_NATIVE_LIBC' filter is just to prevent building for
'native_posix' platform with additional options selected by
'sample.net.sockets.http_get.posix' testcase.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

To the best of my understanding CONFIG_NET_SOCKETS_POSIX_NAMES works with the host libC. So the change is ok.
But, it may be nicer to set CONFIG_PICOLIBC=y in general so we avoid the possible confusion that would follow due to having several symbols with those names around.
We could either do it globally (@rlubos @jukkar would you like this?):

diff --git a/lib/libc/Kconfig b/lib/libc/Kconfig
index 8b71808ff5..e31f04ad33 100644
--- a/lib/libc/Kconfig
+++ b/lib/libc/Kconfig
@@ -61,7 +61,7 @@ menu "C Library"
 
 choice LIBC_IMPLEMENTATION
        prompt "C Library Implementation"
-       default EXTERNAL_LIBC if NATIVE_BUILD && !(NATIVE_LIBRARY && POSIX_API)
+       default EXTERNAL_LIBC if NATIVE_BUILD && !(NATIVE_LIBRARY && (POSIX_API || NET_SOCKETS_POSIX_NAMES))
        default PICOLIBC
        default NEWLIB_LIBC if REQUIRES_FULL_LIBC
        default MINIMAL_LIBC

Or just for this test, (and filter out native_posix, which does not support building with PICO, and is not really adding much anymore):

diff --git a/samples/net/sockets/http_get/sample.yaml b/samples/net/sockets/http_get/sample.yaml
index 999c7ba97d..68a13dac49 100644
--- a/samples/net/sockets/http_get/sample.yaml
+++ b/samples/net/sockets/http_get/sample.yaml
@@ -2,7 +2,12 @@ sample:
   description: BSD Sockets API HTTP GET example
   name: socket_http_get
 common:
-  filter: CONFIG_FULL_LIBC_SUPPORTED and not CONFIG_NATIVE_LIBC
+  filter: CONFIG_FULL_LIBC_SUPPORTED
+  extra_configs:
+    - arch:posix:CONFIG_PICOLIBC=y
+  platform_exclude:
+    - native_posix
+    - native_posix_64
   harness: net
   min_ram: 32
   min_flash: 80

@aescolar aescolar added the hwmv2-likely-conflict DNM until collab-hwmv2 has been merged label Feb 26, 2024
@jukkar
Copy link
Member

jukkar commented Feb 26, 2024

We could either do it globally (@rlubos @jukkar would you like this?):

I am fine with both, but perhaps the global setting makes more sense if it works properly in all cases.

@carlescufi carlescufi merged commit 4666d60 into zephyrproject-rtos:main Mar 3, 2024
21 checks passed
@mniestroj mniestroj deleted the samples-http-get-filter branch March 3, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Samples Samples area: Sockets Networking sockets hwmv2-likely-conflict DNM until collab-hwmv2 has been merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants