| Message ID | 20200426222323.17245-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | fcfbec801bab520bcbb246f38f8efc448c901946 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2020-04-27 01:23:23 +0300, Laurent Pinchart wrote: > Improve the pre-push git hook script to reject commits without the > committer's Signed-off-by line. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > utils/hooks/pre-push | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push > index 34906ddee5ed..0eb8f5ce193d 100755 > --- a/utils/hooks/pre-push > +++ b/utils/hooks/pre-push > @@ -32,12 +32,35 @@ do > range="$remote_sha..$local_sha" > fi > > + # > # Find invalid commits. > - commit=$(git rev-list -n 1 --grep '^---' "$range") > - if [ -n "$commit" ] > + # > + errors=0 > + for commit in $(git rev-list "$range") > + do > + msg=$(git cat-file commit "$commit") > + > + # 1. The commit message shall not contain a local changelog. > + if echo "$msg" | grep -q '^--- *$' This sends cold down my spine, too many times have I been bitten by trying to check the 'sum' of piped return codes in sh if statements. $ if true | false; then echo "1"; else echo "0"; fi 0 $ if false | true; then echo "1"; else echo "0"; fi 1 If we really wish to check the status piped commands in shell scripts please switch this to #!/bin/bash and use 'set -o pipefail'. $ set -o pipefail $ if true | false; then echo "1"; else echo "0"; fi 0 $ if false | true; then echo "1"; else echo "0"; fi 0 > + then > + echo >&2 "Found local changelog in commit $commit" > + errors=$((errors+1)) > + fi > + > + # 2. The commit message shall have a Signed-off-by line > + # corresponding the committer. > + 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}" > + then > + echo >&2 "Missing committer Signed-off-by in commit $commit" > + errors=$((errors+1)) > + fi > + done > + > + if [ $errors != 0 ] > then > - echo >&2 "Found local changelog in $local_ref, not pushing" > - echo >&2 "Check commit $commit" > + echo >&2 "Found $errors errors in $local_ref, not pushing" > exit 1 > fi > done > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Tue, Apr 28, 2020 at 12:36:27AM +0200, Niklas Söderlund wrote: > On 2020-04-27 01:23:23 +0300, Laurent Pinchart wrote: > > Improve the pre-push git hook script to reject commits without the > > committer's Signed-off-by line. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > utils/hooks/pre-push | 31 +++++++++++++++++++++++++++---- > > 1 file changed, 27 insertions(+), 4 deletions(-) > > > > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push > > index 34906ddee5ed..0eb8f5ce193d 100755 > > --- a/utils/hooks/pre-push > > +++ b/utils/hooks/pre-push > > @@ -32,12 +32,35 @@ do > > range="$remote_sha..$local_sha" > > fi > > > > + # > > # Find invalid commits. > > - commit=$(git rev-list -n 1 --grep '^---' "$range") > > - if [ -n "$commit" ] > > + # > > + errors=0 > > + for commit in $(git rev-list "$range") > > + do > > + msg=$(git cat-file commit "$commit") > > + > > + # 1. The commit message shall not contain a local changelog. > > + if echo "$msg" | grep -q '^--- *$' > > This sends cold down my spine, too many times have I been bitten by > trying to check the 'sum' of piped return codes in sh if statements. > > $ if true | false; then echo "1"; else echo "0"; fi > 0 > $ if false | true; then echo "1"; else echo "0"; fi > 1 > > If we really wish to check the status piped commands in shell scripts > please switch this to #!/bin/bash and use 'set -o pipefail'. > > $ set -o pipefail > $ if true | false; then echo "1"; else echo "0"; fi > 0 > $ if false | true; then echo "1"; else echo "0"; fi > 0 But in this case I want to check the return value of grep, not "summing" the two. > > + then > > + echo >&2 "Found local changelog in commit $commit" > > + errors=$((errors+1)) > > + fi > > + > > + # 2. The commit message shall have a Signed-off-by line > > + # corresponding the committer. > > + 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}" > > + then > > + echo >&2 "Missing committer Signed-off-by in commit $commit" > > + errors=$((errors+1)) > > + fi > > + done > > + > > + if [ $errors != 0 ] > > then > > - echo >&2 "Found local changelog in $local_ref, not pushing" > > - echo >&2 "Check commit $commit" > > + echo >&2 "Found $errors errors in $local_ref, not pushing" > > exit 1 > > fi > > done
Hi Laurent, On 2020-04-29 03:56:23 +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Tue, Apr 28, 2020 at 12:36:27AM +0200, Niklas Söderlund wrote: > > On 2020-04-27 01:23:23 +0300, Laurent Pinchart wrote: > > > Improve the pre-push git hook script to reject commits without the > > > committer's Signed-off-by line. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > utils/hooks/pre-push | 31 +++++++++++++++++++++++++++---- > > > 1 file changed, 27 insertions(+), 4 deletions(-) > > > > > > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push > > > index 34906ddee5ed..0eb8f5ce193d 100755 > > > --- a/utils/hooks/pre-push > > > +++ b/utils/hooks/pre-push > > > @@ -32,12 +32,35 @@ do > > > range="$remote_sha..$local_sha" > > > fi > > > > > > + # > > > # Find invalid commits. > > > - commit=$(git rev-list -n 1 --grep '^---' "$range") > > > - if [ -n "$commit" ] > > > + # > > > + errors=0 > > > + for commit in $(git rev-list "$range") > > > + do > > > + msg=$(git cat-file commit "$commit") > > > + > > > + # 1. The commit message shall not contain a local changelog. > > > + if echo "$msg" | grep -q '^--- *$' > > > > This sends cold down my spine, too many times have I been bitten by > > trying to check the 'sum' of piped return codes in sh if statements. > > > > $ if true | false; then echo "1"; else echo "0"; fi > > 0 > > $ if false | true; then echo "1"; else echo "0"; fi > > 1 > > > > If we really wish to check the status piped commands in shell scripts > > please switch this to #!/bin/bash and use 'set -o pipefail'. > > > > $ set -o pipefail > > $ if true | false; then echo "1"; else echo "0"; fi > > 0 > > $ if false | true; then echo "1"; else echo "0"; fi > > 0 > > But in this case I want to check the return value of grep, not "summing" the two. As discussed on IRC the problem with dash not supporting pipefail and the unlikely event of echo failing I think the pragmatic thing to do is Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > + then > > > + echo >&2 "Found local changelog in commit $commit" > > > + errors=$((errors+1)) > > > + fi > > > + > > > + # 2. The commit message shall have a Signed-off-by line > > > + # corresponding the committer. > > > + 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}" > > > + then > > > + echo >&2 "Missing committer Signed-off-by in commit $commit" > > > + errors=$((errors+1)) > > > + fi > > > + done > > > + > > > + if [ $errors != 0 ] > > > then > > > - echo >&2 "Found local changelog in $local_ref, not pushing" > > > - echo >&2 "Check commit $commit" > > > + echo >&2 "Found $errors errors in $local_ref, not pushing" > > > exit 1 > > > fi > > > done > > -- > Regards, > > Laurent Pinchart
Hi Laurent, On 26/04/2020 23:23, Laurent Pinchart wrote: > Improve the pre-push git hook script to reject commits without the > committer's Signed-off-by line. > I see Niklas has some concerns about the potential pipe errors here, but as I've reviewed the patch that goes on top of this, I figure I should add this here too: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> I wonder when this will get turned into an extended python script ;-) > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > utils/hooks/pre-push | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push > index 34906ddee5ed..0eb8f5ce193d 100755 > --- a/utils/hooks/pre-push > +++ b/utils/hooks/pre-push > @@ -32,12 +32,35 @@ do > range="$remote_sha..$local_sha" > fi > > + # > # Find invalid commits. > - commit=$(git rev-list -n 1 --grep '^---' "$range") > - if [ -n "$commit" ] > + # > + errors=0 > + for commit in $(git rev-list "$range") > + do > + msg=$(git cat-file commit "$commit") > + > + # 1. The commit message shall not contain a local changelog. > + if echo "$msg" | grep -q '^--- *$' > + then > + echo >&2 "Found local changelog in commit $commit" > + errors=$((errors+1)) > + fi > + > + # 2. The commit message shall have a Signed-off-by line > + # corresponding the committer. > + 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}" > + then > + echo >&2 "Missing committer Signed-off-by in commit $commit" > + errors=$((errors+1)) > + fi > + done > + > + if [ $errors != 0 ] > then > - echo >&2 "Found local changelog in $local_ref, not pushing" > - echo >&2 "Check commit $commit" > + echo >&2 "Found $errors errors in $local_ref, not pushing" > exit 1 > fi > done >
diff --git a/utils/hooks/pre-push b/utils/hooks/pre-push index 34906ddee5ed..0eb8f5ce193d 100755 --- a/utils/hooks/pre-push +++ b/utils/hooks/pre-push @@ -32,12 +32,35 @@ do range="$remote_sha..$local_sha" fi + # # Find invalid commits. - commit=$(git rev-list -n 1 --grep '^---' "$range") - if [ -n "$commit" ] + # + errors=0 + for commit in $(git rev-list "$range") + do + msg=$(git cat-file commit "$commit") + + # 1. The commit message shall not contain a local changelog. + if echo "$msg" | grep -q '^--- *$' + then + echo >&2 "Found local changelog in commit $commit" + errors=$((errors+1)) + fi + + # 2. The commit message shall have a Signed-off-by line + # corresponding the committer. + 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}" + then + echo >&2 "Missing committer Signed-off-by in commit $commit" + errors=$((errors+1)) + fi + done + + if [ $errors != 0 ] then - echo >&2 "Found local changelog in $local_ref, not pushing" - echo >&2 "Check commit $commit" + echo >&2 "Found $errors errors in $local_ref, not pushing" exit 1 fi done
Improve the pre-push git hook script to reject commits without the committer's Signed-off-by line. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- utils/hooks/pre-push | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)