casaroli commented on code in PR #20778: URL: https://github.com/apache/nuttx/pull/20778#discussion_r4307723428
########## libs/libc/unistd/CMakeLists.txt: ########## @@ -129,4 +129,14 @@ if(CONFIG_PIPES) list(APPEND SRCS lib_pipe.c) endif() +# dpopen() needs pipes, posix_spawn() and waitpid(). CONFIG_SYSTEM_POPEN +# selects the copy in apps/system/popen instead. Review Comment: Agreed. libc now also covers the case that apps/system/popen handles without NSH: when no shell can be started, dpopen() runs the command directly with posix_spawnp(). A flat build finds the NSH `sh` builtin through `/bin/sh`, so NSH's in-process mode is not needed either. Since both copies must never be built together, I will do it in this order: (1) this PR keeps the `CONFIG_SYSTEM_POPEN` guard; (2) a PR that removes `CONFIG_SYSTEM_POPEN` from the 15 defconfigs and enables `CONFIG_SCHED_WAITPID` (and `CONFIG_LIBC_EXECFUNCS` where missing); (3) an apps PR that removes apps/system/popen and moves fastboot and the popen tests to the libc conditions; (4) a last PR that removes the guard. ########## libs/libc/unistd/Make.defs: ########## @@ -78,6 +78,15 @@ ifeq ($(CONFIG_PIPES),y) CSRCS += lib_pipe.c endif +# dpopen() needs pipes, posix_spawn() and waitpid(). CONFIG_SYSTEM_POPEN +# selects the copy in apps/system/popen instead. Review Comment: Yes, see the reply on CMakeLists.txt: I will remove it after this PR, in the order described there. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
