Re: [PATCH rtems-tools] rtemstoolkit: Fix shlex.split to use posix mode and add unit test for pipe operation

[Chris Johns]

On 21/6/2023 2:22 pm, Muhammad Sulthan Mazaya wrote:
>> On 21/6/2023 11:55 am, Muhammad Sulthan Mazaya wrote:
>> > Turns out subprocess.Popen operates on posix mode.
>>
>> Is this true for non-POSIX systems?
>>
>> Where is this documented?
> 
> It's not, turns out it is also dependent on the system too. It is documented
> here https://docs.python.org/3/library/subprocess.html#subprocess.Popen
> . Should I
> add a check like this instead?

I think this is more important:

https://docs.python.org/3/library/shlex.html

I think removing `posix=` from the argument as in your first patch makes our
command parsing conform to POSIX and that seems like a good thing for us.

I will apply the first patch.

Thanks
Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH rtems-tools] rtemstoolkit: Fix shlex.split to use posix mode and add unit test for pipe operation

[Muhammad Sulthan Mazaya]

> On 21/6/2023 11:55 am, Muhammad Sulthan Mazaya wrote:
> > Turns out subprocess.Popen operates on posix mode.
>
> Is this true for non-POSIX systems?
>
> Where is this documented?

It's not, turns out it is also dependent on the system too. It is
documented here
https://docs.python.org/3/library/subprocess.html#subprocess.Popen. Should
I add a check like this instead?

---
 rtemstoolkit/execute.py | 21 ++---
 tester/rt/config.py |  2 +-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/rtemstoolkit/execute.py b/rtemstoolkit/execute.py
index e47aa24..5f0d94b 100755
--- a/rtemstoolkit/execute.py
+++ b/rtemstoolkit/execute.py
@@ -392,7 +392,7 @@ class execute(object):
 command[0] = command[0] + '.exe'
 # If a string split
 if isinstance(command, str):
-command = shlex.split(command, posix=False)
+command = shlex.split(command, posix=(os.name == 'posix'))
 # See if there is a pipe operator in the command. If present
 # split the commands by the pipe and run them with the pipe.
 # if no pipe it is the normal stdin and stdout
@@ -623,10 +623,11 @@ if __name__ == "__main__":
 proc.stdin.close()
 e.capture(proc)
 del proc
-ec, proc = e.open(commands['open'])
-if ec == 0:
-e.capture(proc)
-del proc
+for c in commands['open']:
+ec, proc = e.open(c)
+if ec == 0:
+e.capture(proc)
+del proc

 def capture_output(text):
 print(text, end = '')
@@ -645,7 +646,10 @@ if __name__ == "__main__":
 commands['windows']['csubsts'] = [('netstat %0', ['-a']),
   ('netstat %0 %1', ['-a', '-n'])]
 commands['windows']['pipe'] = ('ftp', None, 'help\nquit')
-commands['windows']['open'] = ["echo",  "hello rtems", "|", "findstr",
"rtems"]
+commands['windows']['open'] = [
+["echo",  "hello rtems", "|", "findstr", "rtems"],
+" ".join(["echo",  "hello rtems", "|", "findstr", "rtems"])
+]
 commands['unix']['shell'] = ['pwd', 'ls -las', './xyz', sh_shell_test]
 commands['unix']['spawn'] = ['ls', 'execute.pyc', ['ls', '-i']]
 commands['unix']['cmd'] = [('date'), ('date', '-R'), ('date', ['-u',
'+%d %D']),
@@ -653,7 +657,10 @@ if __name__ == "__main__":
 commands['unix']['csubsts'] = [('date %0 "+%d %D %S"', ['-u']),
('date %0 %1', ['-u', '+%d %D %S'])]
 commands['unix']['pipe'] = ('grep', 'hello', 'hello world')
-commands['unix']['open'] = ["echo", "hello world", "|", "cut", "-d ",
"-f2"]
+commands['unix']['open'] = [
+["echo", "hello world", "|", "cut", "-d ", "-f2"],
+" ".join(["echo", "hello world", "|", "cut", "-d\" \"", "-f2"])
+]

 print(arg_list('cmd a1 a2 "a3 is a string" a4'))
 print(arg_list('cmd b1 b2 "b3 is a string a4'))
diff --git a/tester/rt/config.py b/tester/rt/config.py
index c0c31de..ceb044e 100644
--- a/tester/rt/config.py
+++ b/tester/rt/config.py
@@ -327,7 +327,7 @@ class file(config.file):
 if len(_data):
 ds = [_data[0]]
 if len(_data) > 1:
-ds += shlex.split(_data[1], posix=False)
+ds += shlex.split(_data[1], posix=(os.name == 'posix'))
 ds = self.expand(ds)

 if _directive == '%console':
-- 
2.34.1


On Wed, Jun 21, 2023 at 1:27 PM Chris Johns  wrote:

> On 21/6/2023 11:55 am, Muhammad Sulthan Mazaya wrote:
> > Turns out subprocess.Popen operates on posix mode.
>
> Is this true for non-POSIX systems?
>
> Where is this documented?
>
> Chris
>
> Also, there is an
> > issue with previous implementation of pipe command that is fixed by
> > Chris. Now, it can also accepts command in form of a string. The unit
> > test for that is added via this patch.
> >
> > ---
> >  rtemstoolkit/execute.py | 21 ++---
> >  tester/rt/config.py |  2 +-
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/rtemstoolkit/execute.py b/rtemstoolkit/execute.py
> > index e47aa24..3b7dcb0 100755
> > --- a/rtemstoolkit/execute.py
> > +++ b/rtemstoolkit/execute.py
> > @@ -392,7 +392,7 @@ class execute(object):
> >  command[0] = command[0] + '.exe'
> >  # If a string split
> >  if isinstance(command, str):
> > -command = shlex.split(command, posix=False)
> > +command = shlex.split(command)
> >  # See if there is a pipe operator in the command. If present
> >  # split the commands by the pipe and run them with the pipe.
> >  # if no pipe it is the normal stdin and stdout
> > @@ -623,10 +623,11 @@ if __name__ == "__main__":
> >  proc.stdin.close()
> >  e.capture(proc)
> > 

Re: [PATCH rtems-tools] rtemstoolkit: Fix shlex.split to use posix mode and add unit test for pipe operation

[Chris Johns]

On 21/6/2023 11:55 am, Muhammad Sulthan Mazaya wrote:
> Turns out subprocess.Popen operates on posix mode. 

Is this true for non-POSIX systems?

Where is this documented?

Chris

Also, there is an
> issue with previous implementation of pipe command that is fixed by
> Chris. Now, it can also accepts command in form of a string. The unit
> test for that is added via this patch.
> 
> ---
>  rtemstoolkit/execute.py | 21 ++---
>  tester/rt/config.py |  2 +-
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/rtemstoolkit/execute.py b/rtemstoolkit/execute.py
> index e47aa24..3b7dcb0 100755
> --- a/rtemstoolkit/execute.py
> +++ b/rtemstoolkit/execute.py
> @@ -392,7 +392,7 @@ class execute(object):
>  command[0] = command[0] + '.exe'
>  # If a string split
>  if isinstance(command, str):
> -command = shlex.split(command, posix=False)
> +command = shlex.split(command)
>  # See if there is a pipe operator in the command. If present
>  # split the commands by the pipe and run them with the pipe.
>  # if no pipe it is the normal stdin and stdout
> @@ -623,10 +623,11 @@ if __name__ == "__main__":
>  proc.stdin.close()
>  e.capture(proc)
>  del proc
> -ec, proc = e.open(commands['open'])
> -if ec == 0:
> -e.capture(proc)
> -del proc
> +for c in commands['open']:
> +ec, proc = e.open(c)
> +if ec == 0:
> +e.capture(proc)
> +del proc
>  
>  def capture_output(text):
>  print(text, end = '')
> @@ -645,7 +646,10 @@ if __name__ == "__main__":
>  commands['windows']['csubsts'] = [('netstat %0', ['-a']),
>('netstat %0 %1', ['-a', '-n'])]
>  commands['windows']['pipe'] = ('ftp', None, 'help\nquit')
> -commands['windows']['open'] = ["echo",  "hello rtems", "|", "findstr", 
> "rtems"]
> +commands['windows']['open'] = [
> +["echo",  "hello rtems", "|", "findstr", "rtems"],
> +" ".join(["echo",  "hello rtems", "|", "findstr", "rtems"])
> +]
>  commands['unix']['shell'] = ['pwd', 'ls -las', './xyz', sh_shell_test]
>  commands['unix']['spawn'] = ['ls', 'execute.pyc', ['ls', '-i']]
>  commands['unix']['cmd'] = [('date'), ('date', '-R'), ('date', ['-u', 
> '+%d %D']),
> @@ -653,7 +657,10 @@ if __name__ == "__main__":
>  commands['unix']['csubsts'] = [('date %0 "+%d %D %S"', ['-u']),
> ('date %0 %1', ['-u', '+%d %D %S'])]
>  commands['unix']['pipe'] = ('grep', 'hello', 'hello world')
> -commands['unix']['open'] = ["echo", "hello world", "|", "cut", "-d ", 
> "-f2"]
> +commands['unix']['open'] = [
> +["echo", "hello world", "|", "cut", "-d ", "-f2"],
> +" ".join(["echo", "hello world", "|", "cut", "-d\" \"", "-f2"])
> +]
>  
>  print(arg_list('cmd a1 a2 "a3 is a string" a4'))
>  print(arg_list('cmd b1 b2 "b3 is a string a4'))
> diff --git a/tester/rt/config.py b/tester/rt/config.py
> index c0c31de..3b12c6c 100644
> --- a/tester/rt/config.py
> +++ b/tester/rt/config.py
> @@ -327,7 +327,7 @@ class file(config.file):
>  if len(_data):
>  ds = [_data[0]]
>  if len(_data) > 1:
> -ds += shlex.split(_data[1], posix=False)
> +ds += shlex.split(_data[1])
>  ds = self.expand(ds)
>  
>  if _directive == '%console':
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH rtems-tools] rtemstoolkit: Fix shlex.split to use posix mode and add unit test for pipe operation

[Muhammad Sulthan Mazaya]

Turns out subprocess.Popen operates on posix mode. Also, there is an
issue with previous implementation of pipe command that is fixed by
Chris. Now, it can also accepts command in form of a string. The unit
test for that is added via this patch.

---
 rtemstoolkit/execute.py | 21 ++---
 tester/rt/config.py |  2 +-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/rtemstoolkit/execute.py b/rtemstoolkit/execute.py
index e47aa24..3b7dcb0 100755
--- a/rtemstoolkit/execute.py
+++ b/rtemstoolkit/execute.py
@@ -392,7 +392,7 @@ class execute(object):
 command[0] = command[0] + '.exe'
 # If a string split
 if isinstance(command, str):
-command = shlex.split(command, posix=False)
+command = shlex.split(command)
 # See if there is a pipe operator in the command. If present
 # split the commands by the pipe and run them with the pipe.
 # if no pipe it is the normal stdin and stdout
@@ -623,10 +623,11 @@ if __name__ == "__main__":
 proc.stdin.close()
 e.capture(proc)
 del proc
-ec, proc = e.open(commands['open'])
-if ec == 0:
-e.capture(proc)
-del proc
+for c in commands['open']:
+ec, proc = e.open(c)
+if ec == 0:
+e.capture(proc)
+del proc
 
 def capture_output(text):
 print(text, end = '')
@@ -645,7 +646,10 @@ if __name__ == "__main__":
 commands['windows']['csubsts'] = [('netstat %0', ['-a']),
   ('netstat %0 %1', ['-a', '-n'])]
 commands['windows']['pipe'] = ('ftp', None, 'help\nquit')
-commands['windows']['open'] = ["echo",  "hello rtems", "|", "findstr", 
"rtems"]
+commands['windows']['open'] = [
+["echo",  "hello rtems", "|", "findstr", "rtems"],
+" ".join(["echo",  "hello rtems", "|", "findstr", "rtems"])
+]
 commands['unix']['shell'] = ['pwd', 'ls -las', './xyz', sh_shell_test]
 commands['unix']['spawn'] = ['ls', 'execute.pyc', ['ls', '-i']]
 commands['unix']['cmd'] = [('date'), ('date', '-R'), ('date', ['-u', '+%d 
%D']),
@@ -653,7 +657,10 @@ if __name__ == "__main__":
 commands['unix']['csubsts'] = [('date %0 "+%d %D %S"', ['-u']),
('date %0 %1', ['-u', '+%d %D %S'])]
 commands['unix']['pipe'] = ('grep', 'hello', 'hello world')
-commands['unix']['open'] = ["echo", "hello world", "|", "cut", "-d ", 
"-f2"]
+commands['unix']['open'] = [
+["echo", "hello world", "|", "cut", "-d ", "-f2"],
+" ".join(["echo", "hello world", "|", "cut", "-d\" \"", "-f2"])
+]
 
 print(arg_list('cmd a1 a2 "a3 is a string" a4'))
 print(arg_list('cmd b1 b2 "b3 is a string a4'))
diff --git a/tester/rt/config.py b/tester/rt/config.py
index c0c31de..3b12c6c 100644
--- a/tester/rt/config.py
+++ b/tester/rt/config.py
@@ -327,7 +327,7 @@ class file(config.file):
 if len(_data):
 ds = [_data[0]]
 if len(_data) > 1:
-ds += shlex.split(_data[1], posix=False)
+ds += shlex.split(_data[1])
 ds = self.expand(ds)
 
 if _directive == '%console':
-- 
2.34.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH rtems-tools] tester/rt: use shlex.split to split command args

[Chris Johns]

Pushed

Thanks
Chris

On 20/6/2023 8:47 am, Muhammad Sulthan Mazaya wrote:
> The regular split-by-space function used to split command arguments
> creates compatibility issues with many shell command syntaxes. A
> specific example is the handling of string arguments, as shown below:
> 
> %define renode_args -e start_opts -e "s %{bsp_resc_script}"
> 
> Thus, it is changed to use shlex.split instead. It splits
> the command arguments using shell-like syntax. More about shlex
> module here: https://docs.python.org/3/library/shlex.html
> 
> ---
>  tester/rt/config.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tester/rt/config.py b/tester/rt/config.py
> index 8a433af..bf6fdbf 100644
> --- a/tester/rt/config.py
> +++ b/tester/rt/config.py
> @@ -38,6 +38,7 @@ import datetime
>  import os
>  import re
>  import threading
> +import shlex
>  
>  from rtemstoolkit import configuration
>  from rtemstoolkit import config
> @@ -326,7 +327,7 @@ class file(config.file):
>  if len(_data):
>  ds = [_data[0]]
>  if len(_data) > 1:
> -ds += _data[1].split()
> +ds += shlex.split(_data[1], posix=False)
>  ds = self.expand(ds)
>  
>  if _directive == '%console':
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH rtems-tools v2] rtemstoolkit: add support for executing pipe command

[Chris Johns]

Hi,

I applied the patch and ran the unit tests and they passed however the change
has broken the execute class.

The FreeBSD host runs sysctl to detect the number of cores when options are
loaded and the `e.shell()` at the top of `freebsd.py` is failing with:

$ ./tester/rtems-test --help
s: /: Permission denied
rtems-test: [options] [args]
RTEMS Tools Project, 6 (60e793a17c38)

The issue is the `command` can be a string or it can be a list and your change
assumes the command provided is a string.

Splitting the command if a string helped but a further changed was needed if
there is only 1 process to run so the stdin and stdout are set correctly.

I have fixed the code and pushed it so please review for me.

Thanks
Chris

On 20/6/2023 12:32 pm, Muhammad Sulthan Mazaya wrote:
> added unit tests and shorten long lines.
> 
> ---
>  rtemstoolkit/execute.py | 43 -
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/rtemstoolkit/execute.py b/rtemstoolkit/execute.py
> index ed81589..9ec1cd7 100755
> --- a/rtemstoolkit/execute.py
> +++ b/rtemstoolkit/execute.py
> @@ -389,11 +389,38 @@ class execute(object):
>  r, e = os.path.splitext(command[0])
>  if e not in ['.exe', '.com', '.bat']:
>  command[0] = command[0] + '.exe'
> -proc = subprocess.Popen(command, shell = shell,
> -cwd = cwd, env = env,
> -stdin = stdin, stdout = stdout,
> -stderr = stderr,
> -close_fds = False)
> +
> +pipe_commands = []
> +current_command = []
> +for cmd in command:
> +if cmd == '|':
> +pipe_commands.append(current_command)
> +current_command = []
> +else:
> +current_command.append(cmd)
> +pipe_commands.append(current_command)
> +
> +proc = None
> +for i, cmd in enumerate(pipe_commands):
> +if i == 0:
> +proc = subprocess.Popen(
> +cmd, shell=shell,
> +cwd=cwd, env=env,
> +stdin=stdin, stdout=subprocess.PIPE)
> +elif i == len(pipe_commands) - 1:
> +proc = subprocess.Popen(
> +cmd, shell=shell,
> +cwd=cwd, env=env,
> +stdin=proc.stdout,
> +stdout=stdout, stderr=stderr,
> +close_fds=False)
> +else:
> +proc = subprocess.Popen(
> +cmd, shell=shell,
> +cwd=cwd, env=env,
> +stdin=proc.stdout,
> +stdout=subprocess.PIPE)
> +
>  if not capture:
>  return (0, proc)
>  if self.output is None:
> @@ -583,6 +610,10 @@ if __name__ == "__main__":
>  proc.stdin.close()
>  e.capture(proc)
>  del proc
> +ec, proc = e.open(commands['open'])
> +if ec == 0:
> +e.capture(proc)
> +del proc
>  
>  def capture_output(text):
>  print(text, end = '')
> @@ -601,6 +632,7 @@ if __name__ == "__main__":
>  commands['windows']['csubsts'] = [('netstat %0', ['-a']),
>('netstat %0 %1', ['-a', '-n'])]
>  commands['windows']['pipe'] = ('ftp', None, 'help\nquit')
> +commands['windows']['open'] = ["echo",  "hello rtems", "|", "findstr", 
> "rtems"]
>  commands['unix']['shell'] = ['pwd', 'ls -las', './xyz', sh_shell_test]
>  commands['unix']['spawn'] = ['ls', 'execute.pyc', ['ls', '-i']]
>  commands['unix']['cmd'] = [('date'), ('date', '-R'), ('date', ['-u', 
> '+%d %D']),
> @@ -608,6 +640,7 @@ if __name__ == "__main__":
>  commands['unix']['csubsts'] = [('date %0 "+%d %D %S"', ['-u']),
> ('date %0 %1', ['-u', '+%d %D %S'])]
>  commands['unix']['pipe'] = ('grep', 'hello', 'hello world')
> +commands['unix']['open'] = ["echo", "hello world", "|", "cut", "-d ", 
> "-f2"]
>  
>  print(arg_list('cmd a1 a2 "a3 is a string" a4'))
>  print(arg_list('cmd b1 b2 "b3 is a string a4'))
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel