Skip to content

Draft: Use pre-commit for all the style/format/code checks

Andoni Morales Alastruey requested to merge ylatuya/gstreamer:pre-commit into main

The following MR migrates all the existing code/format/message checks to pre-commit:

  • gitlint: Commit messages with gitlint
  • rustfmt: Rust format checks
  • gst-indent: C format checks
  • dotnet-format: C# format checks

Caching of dependencies follows the same decisions discussed in cerbero!1315 (merged) cerbero!1322 (merged).

Installation

pre-commit hooks are installed with the following command, using the -f option to override the previous hooks:

pre-commit install -f

This will install gitlint's hook as well, which is a commit-msg hook, using the option default_install_hook_types: [pre-commit, commit-msg] . There is no need to install it separately.

The meson setup installs the pre-commit hooks, similar to how it was done with the previous system.

Testing

All hooks can be tested over a list of files or a range of commits.

The following example shows how to test all the pre-commit format hooks for C, C# and, Rust files:

➜ pre-commit run  --show-diff-on-failure --files subprojects/gstreamer-sharp/sources/custom/Caps.cs subprojects/gstreamer/libs/gst/helpers/ptp/args.rs subprojects/gstreamer/gst/gst.c  
dotnet-format............................................................Passed
gst-indent...............................................................Failed
- hook id: gst-indent
- exit code: 1

--Checking style--
--- .merge_file_YzhXVL  2024-01-14 20:35:33
+++ /tmp/.merge_file_YzhXVL.ut9WE0      2024-01-14 20:35:33
@@ -1291,7 +1335,7 @@ gst_version_string (void)
     return g_strdup_printf ("GStreamer %d.%d.%d (GIT)", major, minor, micro);
   else
     return g_strdup_printf ("GStreamer %d.%d.%d (prerelease)", major, minor,
-        micro);
+                           micro);
 }
 
 /**
=================================================================================================
 Code style error in: subprojects/gstreamer/gst/gst.c                                                                      
=================================================================================================

=================================================================================================
 Code style error in:                                                                            
   subprojects/gstreamer/gst/gst.c
                                                                                                 
 Please fix before committing. Don't forget to run git add before trying to commit again.        
 If the whole file is to be committed, this should work (run from the top-level directory):      
   scripts/gst-indent subprojects/gstreamer/gst/gst.c ; git add subprojects/gstreamer/gst/gst.c ; git commit
                                                                                                 
=================================================================================================

rustfmt..................................................................Passed

To test a range of commits, the following example can be used instead:

➜ pre-commit run  --show-diff-on-failure --from-ref 077470913d084ef1fc834c98ca3a24ea57f92f3b --to-ref HEAD
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /Users/andoni/.cache/pre-commit/patch1705262188-21936.
dotnet-format........................................(no files to check)Skipped
gst-indent...........................................(no files to check)Skipped
rustfmt..............................................(no files to check)Skipped
[INFO] Restored changes from /Users/andoni/.cache/pre-commit/patch1705262188-21936.

TODO

  • Update documentation (especially migration to using pre-commit if old hooks are installed)
  • Check that all hooks work in the CI

Nice-to-have

  • Migrate gst-indent script to python so it's usable in Windows from cmd/powershell
  • Use pre-commit for gitlint in the CI (I was unable to pass a list of commits to validate with pre-commit run --hook-stage manual gitlint-ci)
Edited by Andoni Morales Alastruey

Merge request reports