Closed jaraco closed 1 year ago
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.
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.
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.
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:
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?