Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/830?usp=email
to review the following change.
Change subject: Remove uncrustify config and scripts, switch GHA
......................................................................
Remove uncrustify config and scripts, switch GHA
Replaced with clang-format and pre-commit.
Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Signed-off-by: Frank Lichtenheld <fr...@lichtenheld.com>
---
M .github/workflows/build.yaml
D dev-tools/git-pre-commit-uncrustify.sh
D dev-tools/reformat-all.sh
D dev-tools/special-files.lst
D dev-tools/uncrustify.conf
5 files changed, 12 insertions(+), 402 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/30/830/3
diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index b1af7ec..b33e0ce 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -5,35 +5,26 @@
pull_request:
jobs:
- checkuncrustify:
- name: "Check code style with Uncrustify"
- # Ubuntu 22.04 has uncrustify 0.72_f
- runs-on: ubuntu-22.04
+ clang-format:
+ name: Check code style with clang-format
+ runs-on: ubuntu-24.04
steps:
- name: Install dependencies
- run: sudo apt update && sudo apt install -y uncrustify
+ run: |
+ sudo apt update && sudo apt install -y python3-pip
+ pip3 install pre-commit
- name: Checkout OpenVPN
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 #
v4.2.2
- with:
- path: openvpn
- - name: Show uncrustify version
- run: uncrustify --version
- - name: Run uncrustify
- run: ./dev-tools/reformat-all.sh
- working-directory: openvpn
+ - name: Run clang-format
+ run: pre-commit run -a --show-diff-on-failure || true
- name: Check for changes
- run: git diff --output=uncrustify-changes.patch
- working-directory: openvpn
- - name: Show changes on standard output
- run: git diff
- working-directory: openvpn
+ run: git diff --output=format-changes.patch
- uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08
# v4.6.0
with:
- name: uncrustify-changes.patch
- path: 'openvpn/uncrustify-changes.patch'
+ name: format-changes.patch
+ path: format-changes.patch
- name: Set job status
- run: test ! -s uncrustify-changes.patch
- working-directory: openvpn
+ run: test ! -s format-changes.patch
android:
strategy:
diff --git a/dev-tools/git-pre-commit-uncrustify.sh
b/dev-tools/git-pre-commit-uncrustify.sh
deleted file mode 100755
index 9851c21..0000000
--- a/dev-tools/git-pre-commit-uncrustify.sh
+++ /dev/null
@@ -1,161 +0,0 @@
-#!/bin/sh
-
-# Copyright (c) 2015, David Martin
-# 2022, Heiko Hund
-# All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are met:
-#
-# * Redistributions of source code must retain the above copyright notice, this
-# list of conditions and the following disclaimer.
-#
-# * Redistributions in binary form must reproduce the above copyright notice,
-# this list of conditions and the following disclaimer in the documentation
-# and/or other materials provided with the distribution.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
-# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE
-# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
-# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
-# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
-# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
-# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
-# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-
-# git pre-commit hook that runs an Uncrustify stylecheck.
-# Features:
-# - abort commit when commit does not comply with the style guidelines
-# - create a patch of the proposed style changes
-#
-# More info on Uncrustify: http://uncrustify.sourceforge.net/
-
-# This file was taken from a set of unofficial pre-commit hooks available
-# at https://github.com/ddddavidmartin/Pre-commit-hooks and modified to
-# fit the openvpn project's needs
-
-# exit on error
-set -e
-
-
-# If called so, install this script as pre-commit hook
-if [ "$1" = "install" ] ; then
- TARGET="$(git rev-parse --git-path hooks)/pre-commit"
-
- if [ -e "$TARGET" ] ; then
- printf "$TARGET file exists. Won't overwrite.\n"
- printf "Aborting installation.\n"
- exit 1
- fi
-
- read -p "Install as $TARGET? [y/N] " INPUT
- [ "$INPUT" = "y" ] || exit 0
- cp "$0" "$TARGET"
- chmod +x $TARGET
- exit 0
-fi
-
-# check whether the given file matches any of the set extensions
-matches_extension() {
- local filename="$(basename -- "$1")"
- local extension=".${filename##*.}"
- local ext
-
- for ext in .c .h ; do [ "$ext" = "$extension" ] && return 0; done
-
- return 1
-}
-
-# necessary check for initial commit
-if git rev-parse --verify HEAD >/dev/null 2>&1 ; then
- against=HEAD
-else
- # Initial commit: diff against an empty tree object
- against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-fi
-
-UNCRUSTIFY=$(command -v uncrustify)
-UNCRUST_CONFIG="$(git rev-parse --show-toplevel)/dev-tools/uncrustify.conf"
-
-# make sure the config file and executable are correctly set
-if [ ! -f "$UNCRUST_CONFIG" ] ; then
- printf "Error: uncrustify config file not found.\n"
- printf "Expected to find it at $UNCRUST_CONFIG.\n"
- printf "Aborting commit.\n"
- exit 1
-fi
-
-if [ -z "$UNCRUSTIFY" ] ; then
- printf "Error: uncrustify executable not found.\n"
- printf "Is it installed and in your \$PATH?\n"
- printf "Aborting commit.\n"
- exit 1
-fi
-
-# create a filename to store our generated patch
-patch=$(mktemp /tmp/ovpn-fmt-XXXXXX)
-tmpout=$(mktemp /tmp/uncrustify-XXXXXX)
-
-# create one patch containing all changes to the files
-# sed to remove quotes around the filename, if inserted by the system
-# (done sometimes, if the filename contains special characters, like the quote
itself)
-git diff-index --cached --diff-filter=ACMR --name-only $against -- | \
-sed -e 's/^"\(.*\)"$/\1/' | \
-while read file
-do
- # ignore file if we do check for file extensions and the file
- # does not match the extensions .c or .h
- if ! matches_extension "$file"; then
- continue;
- fi
-
- # escape special characters in the target filename:
- # phase 1 (characters escaped in the output diff):
- # - '\': backslash needs to be escaped in the output diff
- # - '"': quote needs to be escaped in the output diff if present inside
- # of the filename, as it used to bracket the entire filename
part
- # phase 2 (characters escaped in the match replacement):
- # - '\': backslash needs to be escaped again for sed itself
- # (i.e. double escaping after phase 1)
- # - '&': would expand to matched string
- # - '|': used as sed split char instead of '/'
- # printf %s particularly important if the filename contains the % character
- file_escaped_target=$(printf "%s" "$file" | sed -e 's/[\"]/\\&/g' -e
's/[\&|]/\\&/g')
-
- # uncrustify our sourcefile, create a patch with diff and append it to our
$patch
- # The sed call is necessary to transform the patch from
- # --- - timestamp
- # +++ $tmpout timestamp
- # to both lines working on the same file and having a a/ and b/ prefix.
- # Else it can not be applied with 'git apply'.
- git show ":$file" | "$UNCRUSTIFY" -q -l C -c "$UNCRUST_CONFIG" -o "$tmpout"
- git show ":$file" | diff -u -- - "$tmpout" | \
- sed -e "1s|--- -|--- \"b/$file_escaped_target\"|" -e "2s|+++
$tmpout|+++ \"a/$file_escaped_target\"|" >> "$patch"
-done
-
-rm -f "$tmpout"
-
-# if no patch has been generated all is ok, clean up the file stub and exit
-if [ ! -s "$patch" ] ; then
- rm -f "$patch"
- exit 0
-fi
-
-# a patch has been created, notify the user and exit
-printf "Formatting of some code does not follow the project guidelines.\n"
-
-if [ $(wc -l < $patch) -gt 80 ] ; then
- printf "The file $patch contains the necessary fixes.\n"
-else
- printf "Here's the patch that fixes the formatting:\n\n"
- cat $patch
-fi
-
-printf "\nYou can apply these changes with:\n git apply $patch\n"
-printf "(from the root directory of the repository) and then commit again.\n"
-printf "\nAborting commit.\n"
-
-exit 1
diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
deleted file mode 100755
index 42d7368..0000000
--- a/dev-tools/reformat-all.sh
+++ /dev/null
@@ -1,136 +0,0 @@
-#!/bin/sh
-# reformat-all.sh - Reformat all git files in the checked out
-# git branch using uncrustify.
-#
-# Copyright (C) 2016-2024 - David Sommerseth <dav...@openvpn.net>
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
USA.
-#
-
-tstamp="$(date +%Y%m%d-%H%M%S)"
-files="$(pwd)/reformat-all_files-$tstamp.lst"
-log="$(pwd)/reformat-all_log-$tstamp.txt"
-
-srcroot="$(git rev-parse --show-toplevel)"
-cfg="$srcroot/dev-tools/uncrustify.conf"
-specialfiles="$srcroot/dev-tools/special-files.lst"
-
-export gitfiles=0
-export procfiles=0
-
-# Go to the root of the source tree
-cd "$srcroot"
-
-{
- echo -n "** Starting $0: "
- date
-
- # Find all C source/header files
- git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
-
- # Manage files which needs special treatment
- awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > "${files}.sp"
- while read srcfile
- do
- res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
- if [ $? -ne 0 ]; then
- # If grep didn't find the file among special files,
- # process it normally
- echo "$srcfile" >> "$files"
- else
- mode=$(echo "$res" | cut -d: -f1)
- case "$mode" in
- E)
- echo "** INFO ** Excluding '$srcfile'"
- ;;
- P)
- echo "** INFO ** Pre-patching '$srcfile'"
-
patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | tr
"/" "_").patch
- if [ -r "$patchfile" ]; then
- git apply "$patchfile"
- if [ $? -ne 0 ]; then
- echo "** ERROR ** Failed to apply pre-patch file:
$patchfile"
- exit 2
- fi
- else
- echo "** WARN ** Pre-patch file for $srcfile is
missing: $patchfile"
- fi
- echo "$srcfile" >> "${files}.postpatch"
- echo "$srcfile" >> "$files"
- ;;
- *)
- echo "** WARN ** Unknown mode '$mode' for file '$srcfile'"
- ;;
- esac
- fi
- done < "${files}.git"
- rm -f "${files}.git" "${files}.sp"
-
- # Kick off uncrustify
- echo
- echo "** INFO ** Running: uncrustify -c $cfg --no-backup -l C -F $files"
- uncrustify -c "$cfg" --no-backup -l C -F "$files" 2>&1
- res=$?
- echo "** INFO ** Uncrustify completed (exit code $res)"
-} | tee "${log}-1" # Log needs to be closed here, to be processed in next
block
-
-{
- # Check the results
- gitfiles=$(wc -l "$files" | cut -d\ -f1)
- procfiles=$(grep "Parsing: " "${log}-1" | wc -l)
- echo
- echo "C source/header files checked into git: $gitfiles"
- echo "Files processed by uncrustify: $procfiles"
- echo
-
- # Post-Patch files modified after we uncrustify have adjusted them
- if [ -r "${files}.postpatch" ]; then
- while read srcfile;
- do
- patchfile="${srcroot}"/dev-tools/reformat-patches/after_$(echo
"$srcfile" | tr "/" "_").patch
- if [ -r "$patchfile" ]; then
- echo "** INFO ** Post-patching '$srcfile'"
- git apply "$patchfile"
- if [ $? -ne 0 ]; then
- echo "** WARN ** Failed to apply $patchfile"
- fi
- else
- echo "** WARN ** Post-patch file for $srcfile is missing:
$patchfile"
- fi
- done < "${files}.postpatch"
- rm -f "${files}.postpatch"
- fi
-} | tee "${log}-2" # Log needs to be closed here, to be processed in next block
-
-cat "${log}-1" "${log}-2" > "$log"
-
-{
- ec=1
- echo
- if [ "$gitfiles" -eq "$procfiles" ]; then
- echo "Reformatting completed successfully"
- ec=0
- else
- last=$(tail -n1 "${log}-1")
- echo "** ERROR ** Reformating failed to process all files."
- echo " uncrustify exit code: $res"
- echo " Last log line: $last"
- echo
- fi
- rm -f "${log}-1" "${log}-2"
-} | tee -a "$log"
-rm -f "${files}"
-
-exit $ec
diff --git a/dev-tools/special-files.lst b/dev-tools/special-files.lst
deleted file mode 100644
index e5f2fc2..0000000
--- a/dev-tools/special-files.lst
+++ /dev/null
@@ -1,5 +0,0 @@
-E:doc/doxygen/doc_key_generation.h # @verbatim section gets mistreated,
exclude it
-E:src/compat/compat-lz4.c # Preserve LZ4 upstream formatting
-E:src/compat/compat-lz4.h # Preserve LZ4 upstream formatting
-E:src/openvpn/ovpn_dco_linux.h # Preserve ovpn-dco upstream formatting
-E:src/openvpn/ovpn_dco_win.h # Preserve ovpn-dco-win upstream
formatting
diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
deleted file mode 100644
index 325f310..0000000
--- a/dev-tools/uncrustify.conf
+++ /dev/null
@@ -1,79 +0,0 @@
-# Use Allman-style
-indent_columns=4
-indent_braces=false
-indent_else_if=false
-indent_switch_case=4
-indent_label=1
-nl_if_brace=add
-nl_brace_else=add
-nl_elseif_brace=add
-nl_else_brace=add
-nl_else_if=remove
-nl_for_brace=add
-nl_while_brace=add
-nl_switch_brace=add
-nl_fdef_brace=add
-nl_do_brace=add
-sp_func_proto_paren=Remove
-sp_func_def_paren=Remove
-sp_func_call_paren=Remove
-sp_sizeof_paren=Remove
-
-# No tabs, spaces only
-indent_with_tabs=0
-align_with_tabs=false
-cmt_convert_tab_to_spaces=true
-
-# Do not put spaces between the # and preprocessor statements
-pp_space=remove
-
-# Various whitespace fiddling
-sp_assign=add
-sp_before_sparen=add
-sp_inside_sparen=remove
-sp_cond_colon=add
-sp_cond_question=add
-sp_bool=add
-sp_else_brace=add
-sp_brace_else=add
-sp_after_comma=add
-pos_arith=Lead
-pos_bool=Lead
-nl_func_type_name=add
-nl_before_case=true
-nl_assign_leave_one_liners=true
-nl_enum_leave_one_liners=true
-nl_brace_fparen=add
-nl_max=4
-nl_after_func_proto=2
-nl_end_of_file_min=1
-nl_end_of_file=force
-
-# Always use scoping braces for conditionals
-mod_full_brace_if=add
-mod_full_brace_if_chain=false
-mod_full_brace_while=add
-mod_full_brace_for=add
-mod_full_brace_do=add
-
-# Annotate #else and #endif statements
-mod_add_long_ifdef_endif_comment=20
-mod_add_long_ifdef_else_comment=5
-
-# Misc cleanup
-mod_remove_extra_semicolon=true
-
-# leave blank at end of empty for() statements
-sp_after_semi_for_empty=Add
-
-# Use C-style comments (/* .. */)
-cmt_c_nl_end=true
-cmt_star_cont=true
-cmt_cpp_to_c=true
-
-# Use "char **a"-style pointer stars/dereferences
-sp_before_ptr_star=Add
-sp_between_ptr_star=Remove
-sp_after_ptr_star=Remove
-sp_before_byref=Add
-sp_after_byref=Remove
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/830?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Gerrit-Change-Number: 830
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
