diff --git a/.github/workflows/quality_check.yml b/.github/workflows/quality_check.yml index eb20453..0a41308 100644 --- a/.github/workflows/quality_check.yml +++ b/.github/workflows/quality_check.yml @@ -7,7 +7,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.7" + python-version: "3.11" - name: Install dependencies run: | python -m pip install --upgrade pip diff --git a/.github/workflows/status_check.yml b/.github/workflows/status_check.yml index 4259a8c..3e89078 100644 --- a/.github/workflows/status_check.yml +++ b/.github/workflows/status_check.yml @@ -13,7 +13,7 @@ jobs: fetch-depth: 0 - uses: actions/setup-python@v4 with: - python-version: "3.7" + python-version: "3.11" - name: Get the commit message run: | echo 'commit_message<> $GITHUB_ENV diff --git a/main/githooks.py b/main/githooks.py index ecfda0c..7be88b3 100755 --- a/main/githooks.py +++ b/main/githooks.py @@ -56,9 +56,8 @@ # File types that need a terminating newline TERMINATING_NEWLINE_EXTS = ['.c', '.cpp', '.h', '.inl'] - -def _get_output(command, cwd='.'): - return subprocess.check_output(command, shell=True, cwd=cwd).decode(errors='replace') +def _get_output(command_list, cwd='.'): + return subprocess.check_output(command_list, cwd=cwd).decode(errors='replace') def _is_github_event(): @@ -91,7 +90,7 @@ def get_user(): if _is_github_event(): return os.environ['GITHUB_ACTOR'] else: - output = _get_output('git var GIT_AUTHOR_IDENT') + output = _get_output(['git', 'var', 'GIT_AUTHOR_IDENT']) match = re.match(r'^(.+) <', output) return match.group(1) @@ -104,7 +103,7 @@ def get_branch(): else: return os.environ['GITHUB_REF'].split('/')[-1] else: - return _get_output('git branch').split()[-1] + return _get_output(['git', 'branch']).split()[-1] def get_file_content_as_binary(filename): @@ -121,7 +120,7 @@ def get_file_content_as_binary(filename): _skip(filename, 'File is not UTF-8 encoded') data = None else: - data = _get_output(f'git show :{filename}') + data = _get_output(['git','show', f':{filename}']) return data @@ -134,7 +133,7 @@ def get_text_file_content(filename): if _is_github_event() or 'pytest' in sys.modules: data = Path(filename).read_text() else: - data = _get_output(f'git show :{filename}') + data = _get_output(['git', 'show', f':{filename}']) return data @@ -147,7 +146,7 @@ def get_sha(): GITHUB_SHA cannot be used because in a pull request it gives the sha of the fake merge commit. ''' - return _get_output(f'git rev-parse {get_branch()}') + return _get_output(['git','rev-parse', get_branch()]) def get_event(): @@ -160,13 +159,12 @@ def get_event(): def get_branch_files(): '''Get all files in branch''' - branch = get_branch() - return _get_output(f'git ls-tree -r {branch} --name-only').splitlines() + return _get_output(['git','ls-tree', '-r', get_branch(),'--name-only']).splitlines() def add_file_to_index(filename): '''Add file to current commit''' - return _get_output(f'git add {filename}') + return _get_output(['git','add',filename]) def get_commit_files(): @@ -178,12 +176,15 @@ def get_commit_files(): ''' if _is_github_event(): + commands = ['git','diff','--ignore-submodules','--name-status'] if _is_pull_request(): - output = _get_output(f'git diff --ignore-submodules --name-status remotes/origin/{os.environ["GITHUB_BASE_REF"]}..remotes/origin/{os.environ["GITHUB_HEAD_REF"]} --') + commands += [f'remotes/origin/{os.environ["GITHUB_BASE_REF"]}..remotes/origin/{os.environ["GITHUB_HEAD_REF"]}','--'] else: - output = _get_output('git diff --ignore-submodules --name-status HEAD~.. --') + commands += ['HEAD~..', '--'] else: - output = _get_output('git diff-index --ignore-submodules HEAD --cached') + commands = ['git', 'diff-index', '--ignore-submodules', 'HEAD', '--cached'] + + output = _get_output(commands) result = defaultdict(list) for line in output.splitlines(): parts = line.split() @@ -242,15 +243,14 @@ def get_changed_lines(modified_file): :returns: A list of line number (integers and ranges) of changed lines ''' if _is_github_event(): + commands = ['git','diff','--unified=0'] if _is_pull_request(): - output = _get_output( - f'git diff --unified=0 remotes/origin/{os.environ["GITHUB_BASE_REF"]}..remotes/origin/{os.environ["GITHUB_HEAD_REF"]} -- {modified_file}') + commands += [f'remotes/origin/{os.environ["GITHUB_BASE_REF"]}..remotes/origin/{os.environ["GITHUB_HEAD_REF"]}', '--',f'{modified_file}'] else: - output = _get_output( - f'git diff --unified=0 HEAD~ {modified_file}') + commands += ['HEAD~', f'{modified_file}'] else: - output = _get_output( - f'git diff-index HEAD --unified=0 {modified_file}') + commands = [f'git', 'diff-index', 'HEAD', '--unified=0', f'{modified_file}'] + output = _get_output(commands) lines = [] for line in output.splitlines(): @@ -283,7 +283,7 @@ def _test(input, output): def get_config_setting(setting): '''Get the value of a config setting''' try: - return _get_output(f'git config --get {setting}').strip() + return _get_output(['git', 'config', '--get', setting]).strip() except subprocess.CalledProcessError: return None @@ -460,20 +460,24 @@ class TestTrimTrailingWhitespace(unittest.TestCase): def test_trim_trailing_whitespace(self): content = 'first line\nsecond line \nthird line ' trimmed_content = 'first line\nsecond line\nthird line' - with NamedTemporaryFile() as tmp: - Path(tmp.name).write_text(content) + name = NamedTemporaryFile().name + try: + Path(name).write_text(content) # Trailing whitespace found - retval = trim_trailing_whitespace_in_file(tmp.name, True, True) + retval = trim_trailing_whitespace_in_file(name, True, True) self.assertEqual(retval, 1) - self.assertEqual(Path(tmp.name).read_text(), content) + self.assertEqual(Path(name).read_text(), content) # Now remove the trailing whitespace - trim_trailing_whitespace_in_file(tmp.name, True, False, False) + trim_trailing_whitespace_in_file(name, True, False, False) # Trailing whitespace no longer found - self.assertEqual(Path(tmp.name).read_text(), trimmed_content) - retval = trim_trailing_whitespace_in_file(tmp.name, True, True) + self.assertEqual(Path(name).read_text(), trimmed_content) + retval = trim_trailing_whitespace_in_file(name, True, True) self.assertEqual(retval, 0) + finally: + Path(name).unlink() + def test_decodeerror(self): # A text file that is not utf-8 encoded - report and skip