Skip to content

Commit

Permalink
Replace some subprocess.run() calls with subprocess.check_output()
Browse files Browse the repository at this point in the history
wpiformat calls "git diff --name-only master" in a subprocess to get a
list of files which have been modified from master. In GitHub Actions,
there is no master branch by default. When wpiformat is run in GitHub
Actions, this causes the git subprocess to fail. subprocess.run()
doesn't check the return code, so wpiformat was reporting success when
the program had actually failed with an error.

subprocess.check_output() does check the return code by default. A
return code check was also added to "git check-ignore".
  • Loading branch information
calcmogul committed Jul 11, 2020
1 parent d389b56 commit dd16185
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 15 deletions.
2 changes: 1 addition & 1 deletion wpiformat/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Provides linters and formatters for ensuring WPILib's C++, Java, and Python code
Dependencies
************

- `Python 3.5 or newer <https://www.python.org/downloads/>`_
- `Python 3.6 or newer <https://www.python.org/downloads/>`_
- clang-format (included with `LLVM <http://llvm.org/releases/download.html>`_)

To obtain newer versions of clang-format on older Debian and Ubuntu releases, either upgrade to one that does or add the appropriate ``deb ... main`` line from `apt.llvm.org <http://apt.llvm.org/>`_ to ``/etc/apt/sources.list``. Then install ``clang-format-#`` where ``#`` is the version number.
Expand Down
20 changes: 11 additions & 9 deletions wpiformat/wpiformat/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ def filter_ignored_files(names):
# Windows, so os.linesep isn't used here.
encoded_names = "\n".join(names).encode()

output_list = subprocess.run(
proc = subprocess.run(
["git", "check-ignore", "--no-index", "-n", "-v", "--stdin"],
input=encoded_names,
stdout=subprocess.PIPE).stdout.decode().split("\n")
stdout=subprocess.PIPE)
if proc.returncode == 128:
raise subprocess.CalledProcessError

output_list = proc.stdout.decode().split("\n")

# "git check-ignore" prefixes the names of non-ignored files with "::",
# wraps names in quotes on Windows, and outputs "\n" line separators on all
Expand Down Expand Up @@ -303,13 +307,11 @@ def main():
files = filter_ignored_files(files)

# Create list of all changed files
changed_file_list = []

output_list = subprocess.run(["git", "diff", "--name-only", "master"],
stdout=subprocess.PIPE).stdout.split()
for line in output_list:
changed_file_list.append(root_path + os.sep +
line.strip().decode("ascii"))
output_list = subprocess.check_output(
["git", "diff", "--name-only", "master"], encoding="ascii").split()
changed_file_list = [
root_path + os.sep + line.strip() for line in output_list
]

# Don't run tasks on modifiable or generated files
work = []
Expand Down
9 changes: 4 additions & 5 deletions wpiformat/wpiformat/licenseupdate.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,12 @@ def run_pipeline(self, config_file, name, lines):
# log" because the year the file was last modified in the history should
# be used. Author dates can be older than this or even out of order in
# the log.
cmd = ["git", "log", "-n", "1", "--format=%ci", "--", name]
last_year = subprocess.run(cmd,
stdout=subprocess.PIPE).stdout.decode()[:4]
last_year = subprocess.check_output(
["git", "log", "-n", "1", "--format=%ci", "--", name]).decode()[:4]

# Check if file has uncomitted changes in the working directory
cmd = ["git", "diff-index", "--quiet", "HEAD", "--", name]
has_uncommitted_changes = subprocess.run(cmd).returncode
has_uncommitted_changes = subprocess.run(
["git", "diff-index", "--quiet", "HEAD", "--", name]).returncode

# If file hasn't been committed yet or has changes in the working
# directory, use current calendar year as end of copyright year range
Expand Down

0 comments on commit dd16185

Please sign in to comment.