Deprecate execute(), allow it to return strings or bytes and make stripping whitespace from the output optional.
Review Request #13283 — Created Sept. 22, 2023 and submitted — Latest diff uploaded
Our
rbtools.hooks.common.execute()
function usessubprocess.Popen
and
Popen.communicate
to execute commands and read their output. Between Python 2
and 3,subprocess.Popen(...).communicate()
changed from returning a tuple of
strings to a tuple of bytes. This breaks some of our hook util methods in
rbtools.hooks.git
where we try to perform string operations on the output
ofexecute()
, but we receive bytes instead of strings.Python 3.7 added a
text
parameter toPopen
to control whether file
objects, stdin, stdout, and stderr are opened in text mode or binary mode.
We make use of that parameter by adding our owntext
parameter toexecute()
to control whether to return the stdout output as a string or bytes. This
defaults toFalse
to match the default behavior ofPopen
in Python 3.This also makes it optional to strip whitespace from the output, instead of
always stripping it. This is useful when dealing with commands where we
need to preserve the whitespace in the output, such as withgit diff
.This changes also deprecates the
rbtools.hooks.common.execute()
function
in favour ofrbtools.utils.process.run_process()
, which is more
future-proof and has better type safety.
- Ran into an error when running the git pre-receive hook, which came
from the clash between string and bytes. Applied this fix and ran
the hook again, saw that the error was gone. - Ran unit tests.