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

Error using run() on Windows - for eg run("cmd /c dir") [fixed] #46

Closed
ck81 opened this issue Aug 17, 2019 · 9 comments
Closed

Error using run() on Windows - for eg run("cmd /c dir") [fixed] #46

ck81 opened this issue Aug 17, 2019 · 9 comments
Labels

Comments

@ck81
Copy link

ck81 commented Aug 17, 2019

Hi Ken,

Tried to run a DOS command using your suggestion in: aisingapore/TagUI#473

The following works in TagUI:

run cmd /c dir
echo run_result

In TagUI-Python on windows 10:

run_result = t.run("cmd /c dir")
print("run_result)

The above will give the error message:

Traceback (most recent call last):
  File "test1.py", line 2, in <module>
    run_result = t.run("cmd /c dir")
  File "C:\Users\user\AppData\Local\Programs\Python\Python37-32\lib\site-packages\tagui.py", line 1173, in run
    shell=True))
  File "C:\Users\user\AppData\Local\Programs\Python\Python37-32\lib\subprocess.py", line 395, in check_output
    **kwargs).stdout
  File "C:\Users\user\AppData\Local\Programs\Python\Python37-32\lib\subprocess.py", line 487, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command 'cmd /c dir; exit 0' returned non-zero exit status 1.
@kensoh
Copy link
Member

kensoh commented Aug 18, 2019

Hi CK, thanks for raising this! May need your help to test because I don't have access to a Windows laptop now. Issues #45 #46 should be related to the same cause.

The best practice for running commands from Python using subprocess requires using the exit 0 to be always appended. This is to capture error messages if the command results in error -
https://docs.python.org/3.4/library/subprocess.html?highlight=exp#subprocess.check_output

Below shows a replication case if exit 0 is not appended and what happens if appended. When it is not appended as part of the command, if the command returns error it will raise a Python exception with error code instead of showing the error message which is more meaningful for debugging.

>>> t.run('abc')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tagui.py", line 1173, in run
    shell=True))
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 573, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command 'abc' returned non-zero exit status 127

>>> t.run('abc; exit 0')
'/bin/sh: abc: command not found\n'

PS - in above example, I disabled appending in run() in order to test the effect. Will dig further. There may not be a solution or the solution could be not appending for echo commands. For the cmd error, looks like it is some other cause.

@kensoh
Copy link
Member

kensoh commented Aug 18, 2019

For echo on macOS, it looks alright with appending exit 0.

>>> t.run('echo 123; exit 0')
'123\n'

@kensoh
Copy link
Member

kensoh commented Aug 18, 2019

On macOS, the ; works in echo as a delimiter for next command. From your above example, it looks like for Windows, the ; is not recognised by echo as a delimiter. Thus the ;exit 0 string becomes part of the echo output result when you run echo.

Tebel:tagui-python kensoh$ echo 123; echo 456
123
456

@kensoh
Copy link
Member

kensoh commented Aug 18, 2019

Solution may be to use & when run() is invoked on Windows environment. I'll have to borrow a Windows laptop and test on this.

https://stackoverflow.com/questions/8055371/how-do-i-run-two-commands-in-one-line-in-windows-cmd

@kensoh kensoh changed the title run("cmd /c dir") run("cmd /c dir") - shows error with exit status 1 Aug 18, 2019
@kensoh kensoh added the query label Aug 18, 2019
@kensoh kensoh changed the title run("cmd /c dir") - shows error with exit status 1 run("cmd /c dir") on Windows shows error with exit status 1 Aug 18, 2019
@kensoh
Copy link
Member

kensoh commented Aug 18, 2019

The issue should have the same root cause at #45 - probably the ; gets interpreted as part of the command resulting in the error message, will check on this when I borrow a Windows laptop.

@kensoh
Copy link
Member

kensoh commented Aug 20, 2019

Going to update run() to below, tested ok on Windows. Will test on macOS and Linux before patching -

def run(command_to_run = None):
    if command_to_run is None or command_to_run == '':
        print('[TAGUI][ERROR] - command(s) missing for run()')
        return ''

    else:
        if platform.system() == 'Windows':
            command_delimiter = ' & '
        else:
            command_delimiter = '; '
        return _py23_decode(subprocess.check_output(
            command_to_run + command_delimiter + 'exit 0',
            stderr=subprocess.STDOUT,
            shell=True))

@kensoh kensoh added bug and removed query labels Aug 20, 2019
@kensoh kensoh changed the title run("cmd /c dir") on Windows shows error with exit status 1 Error using run() on Windows - for eg run("cmd /c dir") Aug 20, 2019
kensoh added a commit that referenced this issue Aug 20, 2019
The run() function does not work correctly on Windows.

The root cause is semi-colon is not approrpriate as delimiter to combine with exit 0 command (for retrieving error output without triggering exception).

This fix uses ; for Linux and macOS and & as the delimiter in Windows environment. More details in #46.
@kensoh kensoh changed the title Error using run() on Windows - for eg run("cmd /c dir") Error using run() on Windows - for eg run("cmd /c dir") [fixed] Aug 20, 2019
@kensoh
Copy link
Member

kensoh commented Aug 20, 2019

This is now fixed in v1.10 - to upgrade, using pip install tagui --upgrade

Thank you very much @ck81 for spotting this and raising this issue!

@ck81
Copy link
Author

ck81 commented Aug 27, 2019

Hi Ken,

Just downloaded the latest version.

Yes, the following now run's perfectly ok on windows 10 now.

run_result = t.run("cmd /c dir")
print("run_result)

Thanks for the fix!

@kensoh
Copy link
Member

kensoh commented Aug 27, 2019

Thanks CK for your update! I appreciate very much that you spotted this and feedback.

@kensoh kensoh closed this as completed Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants