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

FileNotFoundError attempting to launch sys.executable on Windows #139

Closed
jaraco opened this issue Dec 9, 2022 · 3 comments · Fixed by #140
Closed

FileNotFoundError attempting to launch sys.executable on Windows #139

jaraco opened this issue Dec 9, 2022 · 3 comments · Fixed by #140

Comments

@jaraco
Copy link

jaraco commented Dec 9, 2022

In jaraco/safety-tox, I'm attempting to write a script that will wrap tox and tee its output so the output can be inspected after the run and conditionally alter the return code (bypassing failures for missing dependencies).

When I try to use ubelt.cmd to launch the subprocess, however, it fails with this traceback:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\safety-tox.py", line 72, in <module>
    __name__ == '__main__' and run(sys.argv[1:])
                               ^^^^^^^^^^^^^^^^^
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\safety-tox.py", line 69, in run
    raise SystemExit(Handler().run(args))
                     ^^^^^^^^^^^^^^^^^^^
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\safety-tox.py", line 51, in run
    proc = self.runner(cmd)
           ^^^^^^^^^^^^^^^^
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\jaraco\functools.py", line 35, in <lambda>
    return lambda *args, **kwargs: f1(f2(*args, **kwargs))
                                      ^^^^^^^^^^^^^^^^^^^
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\ubelt\util_cmd.py", line 316, in cmd
    proc = make_proc()
           ^^^^^^^^^^^
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\ubelt\util_cmd.py", line 293, in make_proc
    proc = subprocess.Popen(args, stdout=subprocess.PIPE,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\subprocess.py", line 1022, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\subprocess.py", line 1491, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified

It seems that the way ubelt is manipulating the arguments, it doesn't allow the command to execute the way it would naturally.

I try to avoid shell=True and always pass a sequence of args for the command as I find that to be the most portable. Forcing a string for the cmd leads to issues like seen above.

Is there any way to get the tee functionality from ubelt without any other manipulation?

@Erotemic
Copy link
Owner

Erotemic commented Dec 9, 2022

The idea is that ubelt shouldn't be manipulating the inputs unless it needs to. I want to support the ability to execute a command via either a str or a List[str], regardless of the value of shell, so it is easy for the user to turn that flag on / off without modifying the other inputs. That means when shell=True and the command is a List[str] I have to convert it to a single str, and when it is a str and shell=False, I have to break it up into a List[str] to conform to the Popen API.

Note that it does build the str variant in both cases for logging purposes, but if shell=True that should be exactly the imputed command with no modification, and when shell=False it doesn't use it for any execution.

However, it does look like this is broken on windows. This might simply be a matter of removing the windows check on the line if shell or sys.platform.startswith('win32'):. I'm not exactly sure why this is the case, it might just be a bug. I made a PR #140 that removes it, so I suppose we will see if the dashboards break.

If you could test that patch on your end and verify that it fixes the issue that would be very helpful. I think this is just an oversight on my part because I don't use windows often.

@Erotemic
Copy link
Owner

Erotemic commented Dec 9, 2022

It looks like the naive fix does break existing tests which submit commands like:

    py_script = ub.codeblock(
        r'''
        {pyexe} -c "
        while True:
            pass
        "
        ''').lstrip().format(pyexe=PYEXE)

and:

'{pyexe} -c "for i in range(10): print(str(i))"'.format(pyexe=PYEXE)

as the command to ubelt.cmd.

So it seems like Popen on windows accepts a regular string even when shell=False? That looks like the reason I had the check in there in the first place. I also have a reference to https://stackoverflow.com/questions/33560364/python-windows-parsing-command-lines-with-shlex nearby the area of interest, which is reminding me that it was tricky to try and get ubelt.cmd to parse a str input into a valid List[str] that could be sent to Popen. Do you have a recommendation for how to do that robustly, or an explanation for why it's not possible in general?

One of my original goals with ubelt.cmd was to make it trivial to copy commands from a script into a Python program and have them "just work". So I'd really like to support the case where it parses input into an appropriate input for Popen in the case where shell=False that works on windows and linux, especially when that command is just running a python -c ... script, which is ideally cross platform.

@Erotemic
Copy link
Owner

Looking more at this is seems like SO#33560364 is saying on windows Popen can be a str even when shell=False. If that is true then we can just set args to the string and ignore building command_tup if shell=False and it is None on windows. Testing that now.

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 a pull request may close this issue.

2 participants