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

Add pipeline support for nsh commandline #18

Conversation

JianyuWang0623
Copy link
Member

@JianyuWang0623 JianyuWang0623 commented Dec 24, 2024

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
  3. Concat variable arguments failed - nshlib/pipeline: Concat variable arguments failed
    • Without this patch
    nsh> set var_test `uname`
    nsh> echo $var_test
    NuttX
    nsh> echo $var_test | cat
    sh [5:100]
    
    nsh>
    
    • With this patch
    nsh> set var_test `uname`
    nsh> echo $var_test
    NuttX
    nsh> echo $var_test | cat
    sh [4:100]
    NuttX
    nsh>
    
    Reported by @jasonbu and @[email protected], thank you ;-)

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: apache/nuttx-apps#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

@JianyuWang0623 JianyuWang0623 force-pushed the br_wjy_nsh_cli_pipeline_1015_openvela branch 5 times, most recently from da18edf to 2b77256 Compare December 31, 2024 03:58
@xiaoxiang781216
Copy link
Collaborator

@JianyuWang0623 ltp fail with this patchset.

@JianyuWang0623 JianyuWang0623 force-pushed the br_wjy_nsh_cli_pipeline_1015_openvela branch from 2b77256 to 1965972 Compare January 3, 2025 06:47
@JianyuWang0623
Copy link
Member Author

Polling docker ghcr.io/open-vela/openvela-ci-linux and try locally.
The test_framework, test_example and test_libuv are also failed:

test_framework/test_cmocka.py::test_cmocka FAILED                        [  0%]
test_example/test_example.py::test_hello FAILED                          [  0%]
test_libuv/test_libuv.py::TestLibuv::test_test_macros FAILED             [  0%]
test_open_posix/test_openposix_.py::test_ltp_interfaces_aio_write_9_1 FAILED [  6%]

1. Input redirect flags currently is hardcode, passing by arguments maybe better.
2. Only support redirect to path_name, redirect to fd is needed for pipeline.

Test
  1. Redirect in
    cat < /etc/init.d/rc.sysinit

  2. Redirect with FIFO
    mkfifo /dev/testfifo
    cat /dev/testfifo &
    ls > /dev/testfifo

  3. NSH Params
    set name `uname`
    echo $name

Signed-off-by: wangjianyu3 <[email protected]>
And nested pipeline supported.

Test
  1. Redirect in
    cat < /etc/init.d/rc.sysinit

  2. Simple pipeline
    ls | cat

  3. Nested pipeline
    ls | dd | cat | dd | cat

Signed-off-by: wangjianyu3 <[email protected]>
Changed from calling `strlen()` at runtime to using `sizeof()` at compile time

Signed-off-by: wangjianyu3 <[email protected]>
@JianyuWang0623 JianyuWang0623 force-pushed the br_wjy_nsh_cli_pipeline_1015_openvela branch from 1965972 to 3d4078e Compare January 10, 2025 06:42
@xiaoxiang781216 xiaoxiang781216 merged commit 9524d91 into open-vela:dev Jan 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants