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

Information

RBTools
release-5.x

Reviewers

Our rbtools.hooks.common.execute() function uses subprocess.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
of execute(), but we receive bytes instead of strings.

Python 3.7 added a text parameter to Popen 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 own text parameter to execute()
to control whether to return the stdout output as a string or bytes. This
defaults to False to match the default behavior of Popen 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 with git diff.

This changes also deprecates the rbtools.hooks.common.execute() function
in favour of rbtools.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.

Commits

Files

    Loading...