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.Popenand
Popen.communicateto 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.gitwhere we try to perform string operations on the output
ofexecute(), but we receive bytes instead of strings.Python 3.7 added a
textparameter toPopento 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 owntextparameter toexecute()
to control whether to return the stdout output as a string or bytes. This
defaults toFalseto match the default behavior ofPopenin 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.