Use pre-commit for pre-commit hooks and CI

[akx]

This PR proposes to replace the hand-written pre-commit hook shell scripts with a configuration managed by the pre-commit project, and to then use https://github.com/pre-commit/action to run those actions in CI.

Improvements

• Configuration drift between CI and pre-commit hooks will be less likely (see Add tests/python_build to clang-format pre-commit hook ignore list #87000)
• It should be easier for new contributors to install the hooks
• Running pre-commit hooks will be faster, since pre-commit runs them in parallel (but naturally only for changed files)

Functional changes

• The pre-commit scripts will no longer pop up visual error messages or emit patch files; instead, for auto-formatting, pre-commit will apply the changes into the working tree and let you inspect them and retry the commit.

Removed functionality

• misc/hooks/pre-commit-custom-* is no longer supported, though it could be added via a wrapper of some sort if required.

Notes

• pre-commit itself requires Python 3 to be available. This should hopefully not be an issue, since SCons requires Python 3 anyway.
• The contribution documentation at https://docs.godotengine.org/en/latest/contributing/development/code_style_guidelines.html will require updating (but installing pre-commit hooks becomes easier: pre-commit install once pre-commit itself is installed).
• I didn't quite double-triple-check the exclusion regexps just yet, since this is a pretty big out-of-the-blue change, and I didn't want to cross all of the Ts and dot all of the Is before getting some sort of approval-like noises from Godot maintainers.
• Other checks that are currently only being run in CI could also be run via pre-commit, but these are not addressed; see the above point:
  • Various trivia checked by file_format.sh can be fixed via https://github.com/pre-commit/pre-commit-hooks' mixed-line-ending, end-of-file-fixer and trailing-whitespace lints
  • and many more: https://pre-commit.com/hooks.html

[akx]

Cc @akien-mga on these too? I can polish this up (see PR description) if the idea seems worthwhile.

[akien-mga]

Yeah this looks very promising. I need to test it to confirm it all works as expected, then I'll do a more detailed code review.

[akx]

Rebased post #87000 merge.

[Calinou]

Tested locally, it works as expected. Example with an existing clang-format issue before committing:

~/Documents/Git/godotengine/godot pre-commit*
❯ git commit
clang-format.............................................................Failed
- hook id: clang-format
- files were modified by this hook
make-rst.............................................(no files to check)Skipped
copyright-headers........................................................Passed
black................................................(no files to check)Skipped
black................................................(no files to check)Skipped

~/Documents/Git/godotengine/godot pre-commit*
❯ git commit
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /home/hugo/.cache/pre-commit/patch1707931012-17906.
clang-format.............................................................Failed
- hook id: clang-format
- files were modified by this hook
make-rst.............................................(no files to check)Skipped
copyright-headers........................................................Passed
black................................................(no files to check)Skipped
black................................................(no files to check)Skipped
[WARNING] Stashed changes conflicted with hook auto-fixes... Rolling back fixes...
[INFO] Restored changes from /home/hugo/.cache/pre-commit/patch1707931012-17906.

~/Documents/Git/godotengine/godot pre-commit*
❯ git add . 

~/Documents/Git/godotengine/godot pre-commit*
❯ git commit
clang-format.............................................................Passed
make-rst.............................................(no files to check)Skipped
copyright-headers........................................................Passed
black................................................(no files to check)Skipped
black................................................(no files to check)Skipped

Running pre-commit run --all-files results in a diff on my end, but this is likely because I'm using clang-format 17 (I disabled the version check in my local copy for testing):

 core/extension/gdextension_interface.cpp    |  2 +-
 core/math/geometry_3d.cpp                   |  2 +-
 core/variant/variant_internal.h             |  2 +-
 core/variant/variant_setget.cpp             |  8 ++++----
 drivers/windows/file_access_windows.cpp     |  2 +-
 modules/text_server_fb/text_server_fb.cpp   |  2 +-
 platform/android/export/export_plugin.cpp   |  2 +-
 platform/macos/display_server_macos.mm      |  2 +-
 platform/windows/display_server_windows.cpp |  4 ++--
 servers/physics_3d/godot_soft_body_3d.cpp   |  2 +-
 tests/test_macros.h                         | 10 +++++-----
 11 files changed, 19 insertions(+), 19 deletions(-)

We'll need to update https://github.com/godotengine/godot-docs after this is merged too.

[akien-mga]

Tested locally, it seems to work fine for the most part but I noticed a few issues.

I tested with this local diff, trying to commit it:

diff --git a/SConstruct b/SConstruct
index 04564ad149..90fbdad7d0 100644
--- a/SConstruct
+++ b/SConstruct
@@ -41,7 +41,6 @@ def _helper_module(name, path):
             sys.modules[parent_name] = parent_module
         setattr(parent_module, child_name, child_module)
 
-
 _helper_module("gles3_builders", "gles3_builders.py")
 _helper_module("glsl_builders", "glsl_builders.py")
 _helper_module("methods", "methods.py")
@@ -219,11 +218,7 @@ opts.Add(BoolVariable("disable_advanced_gui", "Disable advanced GUI nodes and be
 opts.Add("build_profile", "Path to a file containing a feature build profile", "")
 opts.Add(BoolVariable("modules_enabled_by_default", "If no, disable all modules except ones explicitly enabled", True))
 opts.Add(BoolVariable("no_editor_splash", "Don't use the custom splash screen for the editor", True))
-opts.Add(
-    "system_certs_path",
-    "Use this path as TLS certificates default for editor and Linux/BSD export templates (for package maintainers)",
-    "",
-)
+opts.Add("system_certs_path", "Use this path as TLS certificates default for editor and Linux/BSD export templates (for package maintainers)", "")
 opts.Add(BoolVariable("use_precise_math_checks", "Math checks use very precise epsilon (debug option)", False))
 opts.Add(BoolVariable("scu_build", "Use single compilation unit build", False))
 opts.Add("scu_limit", "Max includes per SCU file when using scu_build (determines RAM use)", "0")
@@ -389,6 +384,8 @@ for name, path in modules_detected.items():
 
     opts.Add(BoolVariable("module_" + name + "_enabled", "Enable module '%s'" % (name,), enabled))
 
+
+
     # Add module-specific options.
     try:
         for opt in config.get_opts(selected_platform):
@@ -432,7 +429,7 @@ elif env_base.debug_features:
 else:  # Release
     opt_level = "speed"
 
-env_base["optimize"] = ARGUMENTS.get("optimize", opt_level)
+env_base["optimize"]=ARGUMENTS.get("optimize", opt_level)
 env_base["debug_symbols"] = methods.get_cmdline_bool("debug_symbols", env_base.dev_build)
 
 if env_base.editor_build:
diff --git a/core/version.h b/core/version.h
index abb81312ac..ba2fd8a7fa 100644
--- a/core/version.h
+++ b/core/version.h
@@ -1,5 +1,5 @@
 /**************************************************************************/
-/*  version.h                                                             */
+/*  version-wrong.h                                                             */
 /**************************************************************************/
 /*                         This file is part of:                          */
 /*                             GODOT ENGINE                               */
diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml
index e3a13b8ecf..791494b336 100644
--- a/doc/classes/Node.xml
+++ b/doc/classes/Node.xml
@@ -6,7 +6,7 @@
 	<description>
 		Nodes are Godot's building blocks. They can be assigned as the child of another node, resulting in a tree arrangement. A given node can contain any number of nodes as children with the requirement that all siblings (direct children of a node) should have unique names.
 		A tree of nodes is called a [i]scene[/i]. Scenes can be saved to the disk and then instantiated into other scenes. This allows for very high flexibility in the architecture and data model of Godot projects.
-		[b]Scene tree:[/b] The [SceneTree] contains the active tree of nodes. When a node is added to the scene tree, it receives the [constant NOTIFICATION_ENTER_TREE] notification and its [method _enter_tree] callback is triggered. Child nodes are always added [i]after[/i] their parent node, i.e. the [method _enter_tree] callback of a parent node will be triggered before its child's.
+		[b]Scene tree:[/b] The [SceneTree2] contains [b]the active tree of nodes. When a node is added to the scene tree, it receives the [constant NOTIFICATION_ENTER_TREE] notification and its [method _enter_tree] callback is triggered. Child nodes are always added [i]after[/i] their parent node, i.e. the [method _enter_tree] callback of a parent node will be triggered before its child's.
 		Once all nodes have been added in the scene tree, they receive the [constant NOTIFICATION_READY] notification and their respective [method _ready] callbacks are triggered. For groups of nodes, the [method _ready] callback is called in reverse order, starting with the children and moving up to the parent nodes.
 		This means that when adding a node to the scene tree, the following order will be used for the callbacks: [method _enter_tree] of the parent, [method _enter_tree] of the children, [method _ready] of the children and finally [method _ready] of the parent (recursively for the entire scene tree).
 		[b]Processing:[/b] Nodes can override the "process" state, so that they receive a callback on each frame requesting them to process (do something). Normal processing (callback [method _process], toggled with [method set_process]) happens as fast as possible and is dependent on the frame rate, so the processing time [i]delta[/i] (in seconds) is passed as an argument. Physics processing (callback [method _physics_process], toggled with [method set_physics_process]) happens a fixed number of times per second (60 by default) and is useful for code related to the physics engine.
diff --git a/glsl_builders.py b/glsl_builders.py
index a6ca9aa2f3..2645dd4e08 100644
--- a/glsl_builders.py
+++ b/glsl_builders.py
@@ -84,7 +84,7 @@ def include_file_in_rd_header(filename: str, header_data: RDHeaderStruct, depth:
             if not included_file in header_data.vertex_included_files and header_data.reading == "vertex":
                 header_data.vertex_included_files += [included_file]
                 if include_file_in_rd_header(included_file, header_data, depth + 1) is None:
-                    print("Error in file '" + filename + "': #include " + includeline + "could not be found!")
+                    print("Error in file '" + filename + "': #include " + includeline + "could not be found this is now very long so it should be more than 120 chars!")
             elif not included_file in header_data.fragment_included_files and header_data.reading == "fragment":
                 header_data.fragment_included_files += [included_file]
                 if include_file_in_rd_header(included_file, header_data, depth + 1) is None:
@@ -101,6 +101,9 @@ def include_file_in_rd_header(filename: str, header_data: RDHeaderStruct, depth:
         if header_data.reading == "vertex":
             header_data.vertex_lines += [line]
         if header_data.reading == "fragment":
+
+
+
             header_data.fragment_lines += [line]
         if header_data.reading == "compute":
             header_data.compute_lines += [line]
diff --git a/main/main.cpp b/main/main.cpp
index 7d64eccbbf..c68052d8f9 100644
--- a/main/main.cpp
+++ b/main/main.cpp
@@ -45,6 +45,9 @@
 #include "core/io/file_access_zip.h"
 #include "core/io/image_loader.h"
 #include "core/io/ip.h"
+
+
+
 #include "core/io/resource_loader.h"
 #include "core/object/message_queue.h"
 #include "core/os/os.h"
@@ -56,6 +59,8 @@
 #include "main/app_icon.gen.h"
 #include "main/main_timer_sync.h"
 #include "main/performance.h"
+#include "servers/display_server.h"
+#include "servers/movie_writer/movie_writer.h"
 #include "main/splash.gen.h"
 #include "modules/register_module_types.h"
 #include "platform/register_platform_apis.h"
@@ -66,8 +71,6 @@
 #include "scene/theme/theme_db.h"
 #include "servers/audio_server.h"
 #include "servers/camera_server.h"
-#include "servers/display_server.h"
-#include "servers/movie_writer/movie_writer.h"
 #include "servers/movie_writer/movie_writer_mjpeg.h"
 #include "servers/navigation_server_2d.h"
 #include "servers/navigation_server_2d_dummy.h"
@@ -190,7 +193,7 @@ static bool single_threaded_scene = false;
 
 // Display
 
-static DisplayServer::WindowMode window_mode = DisplayServer::WINDOW_MODE_WINDOWED;
+static DisplayServer::WindowMode window_mode=DisplayServer::WINDOW_MODE_WINDOWED;
 static DisplayServer::ScreenOrientation window_orientation = DisplayServer::SCREEN_LANDSCAPE;
 static DisplayServer::VSyncMode window_vsync_mode = DisplayServer::VSYNC_ENABLED;
 static uint32_t window_flags = 0;
@@ -244,12 +247,12 @@ static const int OPTION_COLUMN_LENGTH = 32;
 
 /* Helper methods */
 
-bool Main::is_cmdline_tool() {
-	return cmdline_tool;
-}
+bool Main::is_cmdline_tool() { return cmdline_tool; }
 
 #ifdef TOOLS_ENABLED
 const Vector<String> &Main::get_forwardable_cli_arguments(Main::CLIScope p_scope) {
+
+
 	return forwardable_cli_arguments[p_scope];
 }
 #endif
diff --git a/modules/SCsub b/modules/SCsub
index 7c9946170f..2c2c7aa99f 100644
--- a/modules/SCsub
+++ b/modules/SCsub
@@ -3,7 +3,8 @@
 import modules_builders
 import os
 
-Import("env")
+Import(
+"env")
 
 env_modules = env.Clone()
 
@@ -15,8 +16,8 @@ Export("env_modules")
 # Header with MODULE_*_ENABLED defines.
 env.Depends("modules_enabled.gen.h", Value(env.module_list))
 env.CommandNoCache(
-    "modules_enabled.gen.h",
-    Value(env.module_list),
+    		"modules_enabled.gen.h",
+	    Value(env.module_list),
     env.Run(
         modules_builders.generate_modules_enabled,
         "Generating enabled modules header.",
diff --git a/servers/rendering/renderer_rd/shaders/effects/subsurface_scattering.glsl b/servers/rendering/renderer_rd/shaders/effects/subsurface_scattering.glsl
index fb35d3cde6..6a0ad0b214 100644
--- a/servers/rendering/renderer_rd/shaders/effects/subsurface_scattering.glsl
+++ b/servers/rendering/renderer_rd/shaders/effects/subsurface_scattering.glsl
@@ -4,14 +4,16 @@
 
 #VERSION_DEFINES
 
-layout(local_size_x = 8, local_size_y = 8, local_size_z = 1) in;
+layout(
+
+local_size_x = 8, local_size_y = 8, local_size_z = 1) in;
 
 #ifdef USE_25_SAMPLES
 const int kernel_size = 13;
 
 const vec2 kernel[kernel_size] = vec2[](
-		vec2(0.530605, 0.0),
-		vec2(0.0211412, 0.0208333),
+		vec2(0.530605,0.0),
+		vec2(0.0211412,0.0208333),
 		vec2(0.0402784, 0.0833333),
 		vec2(0.0493588, 0.1875),
 		vec2(0.0410172, 0.333333),

Here's the output:

$ git ci -m 'test'
clang-format.............................................................Failed
- hook id: clang-format
- files were modified by this hook

Warning: Your clang-format binary is the wrong version (17.0.6, expected between 13 and 16).
         Consider upgrading or downgrading clang-format as formatting may not be applied correctly.

make-rst.................................................................Failed
- hook id: make-rst
- exit code: 1

Checking for errors in the XML class reference...
Generating the RST class reference...
ERROR: Node.xml: Unrecognized opening tag "[SceneTree2]" in class "Node" description.
ERROR: Node.xml: Tag depth mismatch: too many (or too few) open/close tags in class "Node" description.

Generating the index file...

184 code samples failed parity check. Use --verbose to get more information.

2 errors were found in the class reference XML. Please check the messages above.

copyright-headers........................................................Failed
- hook id: copyright-headers
- files were modified by this hook
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted glsl_builders.py

All done! ✨ 🍰 ✨
1 file reformatted.

black................................................(no files to check)Skipped

And the files it fixes:

        modified:   core/version.h
        modified:   glsl_builders.py
        modified:   main/main.cpp
        modified:   servers/rendering/renderer_rd/shaders/effects/subsurface_scattering.glsl

---

• The black check doesn't handle SConstruct and SCsub files unlike misc/scripts/black_format.sh and misc/hooks/pre-commit-black.
• There's a second "black" check which says no files to check. Maybe that's related?
• If clang-format is not installed, it fails, but it's not very explicit to the user why, it just says "exit code 1". Our setup instructions need to make clear which tools still need to be installed manually.
• Otherwise it all seems to work great!

[akien-mga]

I did the necessary fixes and some further changes in #88866, which builds on top of this PR and supersedes it.

Thanks for the contribution! pre-commit is going to be a great upgrade for all contributors!