Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pre-commit for pre-commit hooks and CI #87003

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ jobs:
run: |
bash ./misc/scripts/header_guards.sh changed.txt

- name: Python style checks via black (black_format.sh)
run: |
if grep -qE '\.py$|SConstruct|SCsub' changed.txt || [ -z "$(cat changed.txt)" ]; then
bash ./misc/scripts/black_format.sh
else
echo "Skipping Python formatting as no Python files were changed."
fi

- name: Python scripts static analysis (mypy_check.sh)
run: |
if grep -qE '\.py$|SConstruct|SCsub' changed.txt || [ -z "$(cat changed.txt)" ]; then
Expand Down Expand Up @@ -92,12 +84,6 @@ jobs:
- name: Documentation checks
run: |
doc/tools/doc_status.py doc/classes modules/*/doc_classes platform/*/doc_classes
doc/tools/make_rst.py --dry-run --color doc/classes modules platform

- name: Style checks via clang-format (clang_format.sh)
run: |
clang-format --version
bash ./misc/scripts/clang_format.sh changed.txt

- name: Style checks via dotnet format (dotnet_format.sh)
run: |
Expand All @@ -114,3 +100,6 @@ jobs:
skip: "./bin,./thirdparty,*.desktop,*.gen.*,*.po,*.pot,*.rc,./AUTHORS.md,./COPYRIGHT.txt,./DONORS.md,./core/input/gamecontrollerdb.txt,./core/string/locales.h,./editor/project_converter_3_to_4.cpp,./misc/scripts/codespell.sh,./platform/android/java/lib/src/com,./platform/web/node_modules,./platform/web/package-lock.json"
ignore_words_list: "breaked,curvelinear,doubleclick,expct,findn,gird,hel,inout,lod,mis,nd,numer,ot,requestor,te,vai"
path: ${{ env.CHANGED_FILES }}

- name: Other checks via pre-commit
uses: pre-commit/[email protected]
51 changes: 51 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
repos:
- repo: local
hooks:
- id: clang-format
name: clang-format
entry: ./misc/hooks/clang-format-wrap
language: system
files: \.(c|h|cpp|hpp|cc|cxx|m|mm|inc|java|glsl)$
exclude: |
(?x)^(
tests/python_build.*|
.*thirdparty.*|
.*platform/android/java/lib/src/com.*|
.*-so_wrap.*
)

- id: make-rst
akx marked this conversation as resolved.
Show resolved Hide resolved
name: make-rst
entry: python3 doc/tools/make_rst.py doc/classes modules platform --dry-run --color
pass_filenames: false
language: python
files: ^(doc|modules|platform).*xml$

- id: copyright-headers
name: copyright-headers
language: python
files: \.(c|h|cpp|hpp|cc|cxx|m|mm|inc|java)$
entry: python3 misc/scripts/copyright_headers.py
exclude: |
(?x)^(
tests/python_build.*|
.*thirdparty.*|
.*platform/android/java/lib/src/com.*|
.*-so_wrap.*|
platform/android/java/lib/src/org/godotengine/godot/gl/GLSurfaceView.*|
platform/android/java/lib/src/org/godotengine/godot/gl/EGLLogWrapper.*|
platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.*
)

- repo: https://github.com/psf/black-pre-commit-mirror # mypyc-compiled black
rev: 23.3.0
hooks:
- id: black
exclude: .*thirdparty.*
args:
- --line-length=120

- id: black
akx marked this conversation as resolved.
Show resolved Hide resolved
files: SConstruct|SCsub
args:
- --line-length=120
42 changes: 0 additions & 42 deletions misc/hooks/README.md

This file was deleted.

59 changes: 0 additions & 59 deletions misc/hooks/asmessage.applescript

This file was deleted.

48 changes: 0 additions & 48 deletions misc/hooks/canonicalize_filename.sh

This file was deleted.

27 changes: 27 additions & 0 deletions misc/hooks/clang-format-wrap
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env bash

CLANG_FORMAT=$(which clang-format 2>/dev/null)

# To get consistent formatting, we recommend contributors to use the same
# clang-format version as CI.
RECOMMENDED_CLANG_FORMAT_MAJOR_MIN="13"
RECOMMENDED_CLANG_FORMAT_MAJOR_MAX="16"

if [ ! -x "$CLANG_FORMAT" ]; then
message="Error: clang-format executable not found. Please install clang-format $RECOMMENDED_CLANG_FORMAT_MAJOR_MAX."
exit 1
fi

# The returned string can be inconsistent depending on where clang-format comes from.
# Example output strings reported by `clang-format --version`:
# - Ubuntu: "Ubuntu clang-format version 11.0.0-2"
# - Fedora: "clang-format version 11.0.0 (Fedora 11.0.0-2.fc33)"
CLANG_FORMAT_VERSION="$(clang-format --version | sed "s/[^0-9\.]*\([0-9\.]*\).*/\1/")"
CLANG_FORMAT_MAJOR="$(echo "$CLANG_FORMAT_VERSION" | cut -d. -f1)"

if [[ "$CLANG_FORMAT_MAJOR" -lt "$RECOMMENDED_CLANG_FORMAT_MAJOR_MIN" || "$CLANG_FORMAT_MAJOR" -gt "$RECOMMENDED_CLANG_FORMAT_MAJOR_MAX" ]]; then
echo "Warning: Your clang-format binary is the wrong version ($CLANG_FORMAT_VERSION, expected between $RECOMMENDED_CLANG_FORMAT_MAJOR_MIN and $RECOMMENDED_CLANG_FORMAT_MAJOR_MAX)."
echo " Consider upgrading or downgrading clang-format as formatting may not be applied correctly."
fi

exec "$CLANG_FORMAT" -style=file --Wno-error=unknown -i "$@"
52 changes: 3 additions & 49 deletions misc/hooks/pre-commit
Original file line number Diff line number Diff line change
@@ -1,50 +1,4 @@
#!/bin/sh
# Git pre-commit hook that runs multiple hooks specified in $HOOKS.
# Make sure this script is executable. Bypass hooks with git commit --no-verify.

# This file is part of a set of unofficial pre-commit hooks available
# at github.
# Link: https://github.com/githubbrowser/Pre-commit-hooks
# Contact: David Martin, [email protected]


###########################################################
# CONFIGURATION:
# pre-commit hooks to be executed. They should be in the same .git/hooks/ folder
# as this script. Hooks should return 0 if successful and nonzero to cancel the
# commit. They are executed in the order in which they are listed.
HOOKS="pre-commit-clang-format pre-commit-black pre-commit-make-rst"
HOOKS="$HOOKS $(find $(dirname -- "$0") -type f -name 'pre-commit-custom-*' -exec basename {} \;)"
###########################################################
# There should be no need to change anything below this line.

. "$(dirname -- "$0")/canonicalize_filename.sh"

# exit on error
set -e

# Absolute path to this script, e.g. /home/user/bin/foo.sh
SCRIPT="$(canonicalize_filename "$0")"

# Absolute path this script is in, thus /home/user/bin
SCRIPTPATH="$(dirname -- "$SCRIPT")"


for hook in $HOOKS
do
echo "Running hook: $hook"
# run hook if it exists
# if it returns with nonzero exit with 1 and thus abort the commit
if [ -f "$SCRIPTPATH/$hook" ]; then
"$SCRIPTPATH/$hook"
if [ $? != 0 ]; then
exit 1
fi
else
echo "Error: file $hook not found."
echo "Aborting commit. Make sure the hook is in $SCRIPTPATH and executable."
echo "You can disable it by removing it from the list in $SCRIPT."
echo "You can skip all pre-commit hooks with --no-verify (not recommended)."
exit 1
fi
done
# This script does not do anything anymore;
# please follow https://pre-commit.com/#installation instructions.
exit 0
Loading
Loading