| Message ID | 20240109141623.4131307-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | ffcd1b2804bf32be0aa96c108f9b3b2bfebdfa99 |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Tue, Jan 09, 2024 at 02:16:23PM +0000, Kieran Bingham via libcamera-devel wrote: > The pre-push hook validates the commit messages utilising 'echo' to send > the captured data from the git commit through grep. > > Commit messages may occasionally contain strings that could appear to be > escape sequences such as doxygen style references to \struct. > > The '\' 'c' escape sequence can be interpreted to supress all further > output [0] which then breaks the processing and string matching. > > Unfortunatley for us, doxygen's class reference constructed in the same > form as \struct can be interpreted as the escape sequence to supress > further output. > > [0] https://www.gnu.org/software/bash/manual/bash.html#Bash-Builtins > > Update the pre-push hook to explicitly disable escape sequence > interpretation using the '-E' flag. This is not available on the > posix-compliant shell 'dash', so also switch to bash explicitly to > prevent potential failures. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > utils/hooks/pre-push | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push > index 90ffdf6f1755..9918b2861705 100755 > --- a/utils/hooks/pre-push > +++ b/utils/hooks/pre-push > @@ -1,4 +1,4 @@ > -#!/bin/sh > +#!/bin/bash > > # SPDX-License-Identifier: GPL-2.0-or-later > > @@ -61,7 +61,7 @@ do > msg=$(git cat-file commit "$commit") > > # 1. The commit message shall not contain a local changelog. > - if echo "$msg" | grep -q '^--- *$' > + if echo -E "$msg" | grep -q '^--- *$' > then > echo >&2 "Found local changelog in commit $commit" > errors=$((errors+1)) > @@ -71,7 +71,7 @@ do > # corresponding the committer and the author. > committer=$(echo "$msg" | grep '^committer ' | head -1 | \ > cut -d ' ' -f 2- | rev | cut -d ' ' -f 3- | rev) > - if ! echo "$msg" | grep -F -q "Signed-off-by: ${committer}" > + if ! echo -E "$msg" | grep -F -q "Signed-off-by: ${committer}" > then > echo >&2 "Missing committer Signed-off-by in commit $commit" > errors=$((errors+1)) > @@ -79,21 +79,21 @@ do > > author=$(echo "$msg" | grep '^author ' | head -1 | \ > cut -d ' ' -f 2- | rev | cut -d ' ' -f 3- | rev) > - if ! echo "$msg" | grep -F -q "Signed-off-by: ${author}" > + if ! echo -E "$msg" | grep -F -q "Signed-off-by: ${author}" > then > echo >&2 "Missing author Signed-off-by in commit $commit" > errors=$((errors+1)) > fi > > # 3. A Reviewed-by or Acked-by is required. > - if ! echo "$msg" | grep -q '^\(Reviewed\|Acked\)-by: ' > + if ! echo -E "$msg" | grep -q '^\(Reviewed\|Acked\)-by: ' > then > echo >&2 "No Reviewed-by or Acked-by in commit $commit" > errors=$((errors+1)) > fi > > # 4. The commit message shall not contain a Change-Id. > - if echo "$msg" | grep -q '^Change-Id:' > + if echo -E "$msg" | grep -q '^Change-Id:' > then > echo >&2 "Found Change-Id in commit $commit" > errors=$((errors+1))
diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push index 90ffdf6f1755..9918b2861705 100755 --- a/utils/hooks/pre-push +++ b/utils/hooks/pre-push @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # SPDX-License-Identifier: GPL-2.0-or-later @@ -61,7 +61,7 @@ do msg=$(git cat-file commit "$commit") # 1. The commit message shall not contain a local changelog. - if echo "$msg" | grep -q '^--- *$' + if echo -E "$msg" | grep -q '^--- *$' then echo >&2 "Found local changelog in commit $commit" errors=$((errors+1)) @@ -71,7 +71,7 @@ do # corresponding the committer and the author. committer=$(echo "$msg" | grep '^committer ' | head -1 | \ cut -d ' ' -f 2- | rev | cut -d ' ' -f 3- | rev) - if ! echo "$msg" | grep -F -q "Signed-off-by: ${committer}" + if ! echo -E "$msg" | grep -F -q "Signed-off-by: ${committer}" then echo >&2 "Missing committer Signed-off-by in commit $commit" errors=$((errors+1)) @@ -79,21 +79,21 @@ do author=$(echo "$msg" | grep '^author ' | head -1 | \ cut -d ' ' -f 2- | rev | cut -d ' ' -f 3- | rev) - if ! echo "$msg" | grep -F -q "Signed-off-by: ${author}" + if ! echo -E "$msg" | grep -F -q "Signed-off-by: ${author}" then echo >&2 "Missing author Signed-off-by in commit $commit" errors=$((errors+1)) fi # 3. A Reviewed-by or Acked-by is required. - if ! echo "$msg" | grep -q '^\(Reviewed\|Acked\)-by: ' + if ! echo -E "$msg" | grep -q '^\(Reviewed\|Acked\)-by: ' then echo >&2 "No Reviewed-by or Acked-by in commit $commit" errors=$((errors+1)) fi # 4. The commit message shall not contain a Change-Id. - if echo "$msg" | grep -q '^Change-Id:' + if echo -E "$msg" | grep -q '^Change-Id:' then echo >&2 "Found Change-Id in commit $commit" errors=$((errors+1))
The pre-push hook validates the commit messages utilising 'echo' to send the captured data from the git commit through grep. Commit messages may occasionally contain strings that could appear to be escape sequences such as doxygen style references to \struct. The '\' 'c' escape sequence can be interpreted to supress all further output [0] which then breaks the processing and string matching. Unfortunatley for us, doxygen's class reference constructed in the same form as \struct can be interpreted as the escape sequence to supress further output. [0] https://www.gnu.org/software/bash/manual/bash.html#Bash-Builtins Update the pre-push hook to explicitly disable escape sequence interpretation using the '-E' flag. This is not available on the posix-compliant shell 'dash', so also switch to bash explicitly to prevent potential failures. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- utils/hooks/pre-push | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)