Add pipeline support for nsh commandline

[JianyuWang0623]

Summary

Add pipeline support for nsh commandline through pipe2() and posix_spawn_file_actions_adddup2()

1. Pack parameters of nsh_execute() as struct nsh_exec_param_s
   • Input redirect flags currently is hardcode, passing by arguments maybe better.
   • Only support redirect to path_name, redirect to fd is needed for pipeline.
2. Add pipeline support for commandline

And nesting pipeline supported.

Impact

nsh: nsh_parse_command()

Testing

General

# Redirectin
cat < /etc/init.d/rc.sysinit

# Pipe
ls | cat

# Pipe (nested)
ls | cat | cat | cat

# FIFO
mkfifo /dev/testfifo
cat /dev/testfifo &
ls > /dev/testfifo

# CMDPARAMS
set name `uname`
echo $name

# binfmt/sotest
sotest

With dd

Depends: #2751

# Env
# sim:nsh with `PIPES`, `NSH_PIPELINE_NEST_DEPTH=5`, `FS_HOSTFS` and `SIM_HOSTFS` enabled.

# Read from stdin and write to "of" 
mount -t hostfs -o fs=. /data 
ls | cat | dd | cat | dd of=/data/test_dd_stdin

#Read from stdin and write to stdout
ls | cat | dd | cat | dd

• patch

$ git diff
diff --git a/nshlib/nsh_ddcmd.c b/nshlib/nsh_ddcmd.c
index 601e64408d..812d51c103 100644
--- a/nshlib/nsh_ddcmd.c
+++ b/nshlib/nsh_ddcmd.c
@@ -107,7 +107,9 @@ static int dd_write(FAR struct dd_s *dd)
   written = 0;
   do
     {
+      write(dd->outfd, "<dd ", 4);
       nbytes = write(dd->outfd, buffer, dd->nbytes - written);
+      write(dd->outfd, " dd>", 4);
       if (nbytes < 0)
         { 
           FAR struct nsh_vtbl_s *vtbl = dd->vtbl;
diff --git a/nshlib/nsh_fscmds.c b/nshlib/nsh_fscmds.c
index 413ed24634..e6a453ab5b 100644
--- a/nshlib/nsh_fscmds.c
+++ b/nshlib/nsh_fscmds.c
@@ -806,7 +806,9 @@ int cmd_cat(FAR struct nsh_vtbl_s *vtbl, int argc, FAR char **argv)
           if (n == 0)
             break;

+          nsh_write(vtbl, "<cat ", 5);
           nsh_write(vtbl, buf, n);
+          nsh_write(vtbl, " cat>", 5);
         }

       free(buf);

• runtime

nsh> ls | cat | cat | dd | dd
<dd <dd <cat <cat /:
 bin/
 data/
 dev/
 etc/
 proc/
 tmp/
 var/
 cat> cat> dd> dd>nsh>
nsh> ls | dd | cat | cat | dd
<dd <cat <cat <dd /:
 bin/
 data/
 dev/
 etc/
 proc/
 tmp/
 var/
 dd> cat cat><cat > cat> dd>nsh>
nsh>

Apache:NuttX CI

[nuttxpr]

[Experimental Bot, please feedback here]

The PR partially meets the NuttX requirements.

Strengths:

• Summary provides a clear explanation of the changes and their purpose.
• Testing section includes specific examples and mentions CI, which is good.

Areas for improvement:

• Impact:
  • The impact section is incomplete. It only mentions nsh: nsh_parse_command() without explaining the impact of the change on users, builds, hardware, documentation, security, or compatibility.
  • You need to thoroughly address all impact points, stating "NO" or "YES" with a clear and concise description if applicable. For instance:
    • Impact on the user: Will users need to change how they use the nsh command line due to pipeline support?
    • Impact on build: Will any build system changes be needed for this feature?
    • Impact on documentation: Does this change require documentation updates? If yes, have you updated the documentation?
• Testing:
  • While you mention "Selftest" and "Apache:NuttX CI," providing snippets of the actual logs (before and after the change) would significantly strengthen this section.
  • Detailing the build host and target environments used for testing would also be beneficial. For example:
    • Build Host: Ubuntu 20.04, GCC 9.4
    • Target: SIM, ARM Cortex-M4

Recommendations:

1. Expand the Impact section. Provide a "NO" or "YES" answer and a brief explanation for each impact point.
2. Include detailed testing logs. Add snippets of the actual testing logs before and after your changes for both the selftest and CI environments.
3. Specify testing environments. Clearly state the build host operating system, compiler, and target architecture/board used for testing.
4. Consider future work: Briefly mention your plans to upstream nested pipeline support. This demonstrates ongoing commitment and a roadmap for future improvements.

By addressing these points, your PR will be more comprehensive and easier for the NuttX maintainers to review.

[JianyuWang0623]

Link #2469

[cederom]

Thank you @JianyuWang0623 ! :-)

[tmedicci]

Hi @JianyuWang0623 !

This PR broke esp32s3-devkit:smp's ostest just like reported at apache/nuttx#14611 (comment) (it is the same behavior). Could you please take a look?

[tmedicci]

Perhaps @hujun260 may have a clue here too...

[JianyuWang0623]

Hi @JianyuWang0623 !

This PR broke esp32s3-devkit:smp's ostest just like reported at apache/nuttx#14611 (comment) (it is the same behavior). Could you please take a look?

@tmedicci OK, I'll prepare the environment and try to reproduce.

[JianyuWang0623]

Hi @JianyuWang0623 !
This PR broke esp32s3-devkit:smp's ostest just like reported at apache/nuttx#14611 (comment) (it is the same behavior). Could you please take a look?

@tmedicci OK, I'll prepare the environment and try to reproduce.

@tmedicci

1. A simple test with qemu-armv8a:nsh_smp was passed:

user_main: Exiting
ostest_main: Exiting with status 0
nsh>

2. But having problems when executing esp32s3-devkit:smp.

Ref to @masayuki2009 `s comment apache/nuttx#14656 (comment)

/home/ishikawa/opensource/QEMU/qemu-esp-develop-8.2.0-20240122/build/qemu-system-xtensa -nographic -M esp32 -smp 2 -drive file=/home/ishikawa/qemu_efuse.bin,if=none,format=raw,id=efuse -global driver=nvram.esp32.efuse,property=drive,value=efuse -drive file=./nuttx/nuttx.merged.bin,if=mtd,format=raw

• Pulling ghcr.io/apache/nuttx/apache-nuttx-ci-linux for build
• Download "qemu-system-xtensa" from https://github.com/espressif/qemu/releases

But don`t know how to generate the "qemu_efuse.bin". @masayuki2009 @tmedicci Do you know where has the quickstart doc? thank you;-)

[masayuki2009]

But don`t know how to generate the "qemu_efuse.bin". @masayuki2009 @tmedicci Do you know where has the quickstart doc? thank you;-)

apache/nuttx#11563 (comment)

[JianyuWang0623]

@tmedicci Is there a full steps/configs to build/run esp32s3-devkit:smp(QEMU)?

1. nuttx.merged.bin: no such file - Enable ESP32S3_MERGE_BINS to gen
2. only 2, 4, 8, 16 MB flash images are supported - Enable ESP32S3_QEMU_IMAGE to fix

$ ../../qemu-xtensa-softmmu-esp_develop_8.2.0_20240122-x86_64-linux-gnu/bin/qemu-system-xtensa -nographic -M esp32 -smp 2 -drive file=../../qemu_efuse.bin,if=none,format=raw,id=efuse -global driver=nvram.esp32.efuse,property=drive,value=efuse -drive file=./nuttx.merged.bin,if=mtd,format=raw
Adding SPI flash device
qemu-system-xtensa: Error: only 2, 4, 8, 16 MB flash images are supported
qemu-system-xtensa: -drive file=./nuttx.merged.bin,if=mtd,format=raw: machine type does not support if=mtd,bus=0,unit=0

3. invalid header

Adding SPI flash device
ets Jul 29 2019 12:21:46

rst:0x1 (POWERON_RESET),boot:0x12 (SPI_FAST_FLASH_BOOT)
invalid header: 0x03610250
invalid header: 0x03610250
invalid header: 0x03610250
invalid header: 0x03610250
invalid header: 0x03610250
invalid header: 0x03610250

Currently ostest in qemu-armv8a:nsh_smp passed.

[JianyuWang0623]

@tmedicci esp32s3-devkit:smp(QEMU) ostest passed

./tools/configure.sh -l esp32s3-devkit:smp
# Enable ESP32S3_MERGE_BINS
# Enable ESP32S3_QEMU_IMAGE

qemu-system-xtensa -nographic -machine esp32s3 -drive file=nuttx.merged.bin,if=mtd,format=raw

nsh> ostest
... ...
user_main: Exiting
ostest_main: Exiting with status 0
nsh> uname -a
NuttX 12.7.0 f41b0d00fdc-dirty Nov 12 2024 12:58:43 xtensa esp32s3-devkit
nsh>

[tmedicci]

@tmedicci esp32s3-devkit:smp(QEMU) ostest passed

./tools/configure.sh -l esp32s3-devkit:smp
# Enable ESP32S3_MERGE_BINS
# Enable ESP32S3_QEMU_IMAGE

qemu-system-xtensa -nographic -machine esp32s3 -drive file=nuttx.merged.bin,if=mtd,format=raw

nsh> ostest
... ...
user_main: Exiting
ostest_main: Exiting with status 0
nsh> uname -a
NuttX 12.7.0 f41b0d00fdc-dirty Nov 12 2024 12:58:43 xtensa esp32s3-devkit
nsh>

I tested with real hardware. Don't you have one at your disposal? I can try on QEMU too later.

[JianyuWang0623]

I tested with real hardware. Don't you have one at your disposal? I can try on QEMU too later.

ok, will try on hardware later

[tmedicci]

Just to add some details: I forgot to mention that our internal CI enables CONFIG_DEBUG_ASSERTIONS. This makes total difference to the test. I tried with and without applying the following patch to the esp32s3-devkit:smp.

The patch:

diff --git a/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig b/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig
index a72faf9b2d..bf6fbad093 100644
--- a/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig
+++ b/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig
@@ -21,8 +21,12 @@ CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARCH_XTENSA=y
 CONFIG_BOARD_LOOPSPERMSEC=16717
 CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_ASSERTIONS_EXPRESSION=y
+CONFIG_DEBUG_FEATURES=y
 CONFIG_DEBUG_FULLOPT=y
 CONFIG_DEBUG_SYMBOLS=y
+CONFIG_ESP32S3_MERGE_BINS=y
 CONFIG_ESP32S3_UART0=y
 CONFIG_FS_PROCFS=y
 CONFIG_HAVE_CXX=y

Applying this patch, the ostest hangs:

user_main: pthread_rwlock test
pthread_rwlock: Initializing rwlock
pthread_exit_thread 30: Exiting

I attached the firmware generated by applying this patch with the following revisions:

• nuttx: f41b0d0
• nuttx-apps: c052bd8 (the commit from this PR)
  nuttx.zip

[tmedicci]

Just to add some details: I forgot to mention that our internal CI enables CONFIG_DEBUG_ASSERTIONS. This makes total difference to the test. I tried with and without applying the following patch to the esp32s3-devkit:smp.

The patch:

diff --git a/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig b/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig
index a72faf9b2d..bf6fbad093 100644
--- a/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig
+++ b/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig
@@ -21,8 +21,12 @@ CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARCH_XTENSA=y
 CONFIG_BOARD_LOOPSPERMSEC=16717
 CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_ASSERTIONS_EXPRESSION=y
+CONFIG_DEBUG_FEATURES=y
 CONFIG_DEBUG_FULLOPT=y
 CONFIG_DEBUG_SYMBOLS=y
+CONFIG_ESP32S3_MERGE_BINS=y
 CONFIG_ESP32S3_UART0=y
 CONFIG_FS_PROCFS=y
 CONFIG_HAVE_CXX=y

Applying this patch, the ostest hangs:

user_main: pthread_rwlock test
pthread_rwlock: Initializing rwlock
pthread_exit_thread 30: Exiting

I attached the firmware generated by applying this patch with the following revisions:

• nuttx: f41b0d0
• nuttx-apps: c052bd8 (the commit from this PR)
  nuttx.zip

Hi @JianyuWang0623 , although the issue happens on those references, I've just on the latest master and it seems to be fixed:
Just for the records, just tried with:

• nuttx: 3d90079
• nuttx-apps: 1c7a7f7

So, don't mind trying to find a solution for outdated code. We will keep testing it in our internal CI. Thanks!