[dim PATCH 1/7] dim: don't fail on grep not matching

[dim PATCH 1/7] dim: don't fail on grep not matching

[Jani Nikula]

Oops, the comment told us to watch out for this.

Fixes: 56e53a49e28f ("dim: declare and assign separately")
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/dim b/dim
index c1ac9e546ea9..ddcc18f17f0d 100755
--- a/dim
+++ b/dim
@@ -1091,10 +1091,7 @@ function checkpatch_commit
 	git --no-pager log --oneline -1 $commit
 	$cmd | scripts/checkpatch.pl -q --emacs --strict - || true
 
-	# FIXME: this relies on local assignment not failing on command
-	# substitution failures
-	bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG')
-	if test "$bug_lines" -eq 1; then
+	if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then
 		warn_or_fail "New BUG macro added"
 	fi
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[dim PATCH 2/7] dim: abstract and fix maintainer scope check for dinq

[Jani Nikula]

Separate maintainer scope check from checkpatch, and only do the check
on applying patches. Also, fix the failures on grep not matching.

Fixes: 56e53a49e28f ("dim: declare and assign separately")
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/dim b/dim
index ddcc18f17f0d..268bc6ca280d 100755
--- a/dim
+++ b/dim
@@ -698,6 +698,7 @@ function dim_apply_branch
 	fi
 
 	checkpatch_commit HEAD
+	check_maintainer $branch HEAD
 
 	eval $DRY $DIM_POST_APPLY_ACTION
 }
@@ -1080,6 +1081,25 @@ function dim_conf
 	dim_checkout drm-intel-next-fixes "$@"
 }
 
+# $1 branch
+# $2 commit
+function check_maintainer
+{
+	local branch commit
+
+	branch=$1
+	commit=$2
+
+        if [ "$branch" = "drm-intel-next-queued" ]; then
+		if non_i915_files=$(git diff-tree --no-commit-id --name-only -r $commit | \
+			grep -v "^\(drivers/gpu/drm/i915/\|include/drm/i915\|include/uapi/drm/i915\)") && [[ -n "$non_i915_files" ]]; then
+			echo -e "The following files are outside of i915 maintenance scope:\n"
+			echo "$non_i915_files"
+			echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files."
+		fi
+	fi
+}
+
 # $1 is the git sha1 to check
 function checkpatch_commit
 {
@@ -1094,19 +1114,6 @@ function checkpatch_commit
 	if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then
 		warn_or_fail "New BUG macro added"
 	fi
-
-        if [ "$branch" = "drm-intel-next-queued" ]; then
-		# FIXME: this relies on local assignment not failing on command
-		# substitution failures
-		non_i915_files=$(git diff-tree --no-commit-id --name-only -r $commit | \
-			grep -v "^\(drivers/gpu/drm/i915/\|include/drm/i915\|include/uapi/drm/i915\)")
-
-		if [ -n "$non_i915_files" ]; then
-			echo -e "The following files are outside of i915 maintenance scope:\n"
-			echo "$non_i915_files"
-			echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files."
-		fi
-	fi
 }
 
 # turn $1 in to a git commit range
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[dim PATCH 3/7] dim: only issue warnings on new BUG macros

[Jani Nikula]

We apply the patch regardless of the warn_or_fail result, so just switch
to using an error message.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dim b/dim
index 268bc6ca280d..e6943d34c43f 100755
--- a/dim
+++ b/dim
@@ -1112,7 +1112,7 @@ function checkpatch_commit
 	$cmd | scripts/checkpatch.pl -q --emacs --strict - || true
 
 	if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then
-		warn_or_fail "New BUG macro added"
+		echoerr "WARNING: New BUG macro added"
 	fi
 }
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[dim PATCH 4/7] dim: return exit code from checkpatch_commit

[Jani Nikula]

Return the exit code to let callers make better decisions on what to do.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/dim b/dim
index e6943d34c43f..c282782b429d 100755
--- a/dim
+++ b/dim
@@ -697,7 +697,7 @@ function dim_apply_branch
 		echo "No message-id found in the patch file."
 	fi
 
-	checkpatch_commit HEAD
+	checkpatch_commit HEAD || true
 	check_maintainer $branch HEAD
 
 	eval $DRY $DIM_POST_APPLY_ACTION
@@ -893,7 +893,7 @@ dim_alias_ar=apply-resolved
 function dim_apply_resolved
 {
 	make $DIM_MAKE_OPTIONS && git add -u && git am --resolved
-	checkpatch_commit HEAD
+	checkpatch_commit HEAD || true
 
 	eval $DRY $DIM_POST_APPLY_ACTION
 }
@@ -1103,17 +1103,22 @@ function check_maintainer
 # $1 is the git sha1 to check
 function checkpatch_commit
 {
-	local commit cmd bug_lines non_i915_files
+	local commit cmd bug_lines non_i915_files rv
 
 	commit=$1
 	cmd="git show --pretty=email $commit"
 
 	git --no-pager log --oneline -1 $commit
-	$cmd | scripts/checkpatch.pl -q --emacs --strict - || true
+	if ! $cmd | scripts/checkpatch.pl -q --emacs --strict -; then
+		rv=1
+	fi
 
 	if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then
 		echoerr "WARNING: New BUG macro added"
+		rv=1
 	fi
+
+	return $rv
 }
 
 # turn $1 in to a git commit range
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[dim PATCH 5/7] dim: make dim checkpatch subcommand return exit codes

[Jani Nikula]

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/dim b/dim
index c282782b429d..e23617c8907c 100755
--- a/dim
+++ b/dim
@@ -1179,13 +1179,17 @@ dim_alias_check_patch=checkpatch
 dim_alias_cp=checkpatch
 function dim_checkpatch
 {
-	local range
+	local range rv
 
 	range=$(rangeish "$1")
 
 	for commit in $(git rev-list --reverse $range); do
-		checkpatch_commit $commit || true
+		if ! checkpatch_commit $commit; then
+			rv=1
+		fi
 	done
+
+	return $rv
 }
 
 function dim_sparse
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[dim PATCH 6/7] dim: make dim apply-branch subcommand return exit codes

[Jani Nikula]

For example, piping from emacs only pops up the stdout/stderr buffer
when the exit code is non-zero.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/dim b/dim
index e23617c8907c..7b6275e7796f 100755
--- a/dim
+++ b/dim
@@ -667,7 +667,7 @@ dim_alias_ab=apply-branch
 dim_alias_sob=apply-branch
 function dim_apply_branch
 {
-	local branch file message_id commiter_email patch_from sob
+	local branch file message_id commiter_email patch_from sob rv
 
 	branch=$1
 	shift
@@ -694,13 +694,18 @@ function dim_apply_branch
 	if [ -n "$message_id" ]; then
 		dim_commit_add_tag "Link: http://patchwork.freedesktop.org/patch/msgid/$message_id"
 	else
-		echo "No message-id found in the patch file."
+		echoerr "WARNING: No message-id found in the patch file."
+		rv=1
 	fi
 
-	checkpatch_commit HEAD || true
+	if ! checkpatch_commit HEAD; then
+		rv=1
+	fi
 	check_maintainer $branch HEAD
 
 	eval $DRY $DIM_POST_APPLY_ACTION
+
+	return $rv
 }
 
 function dim_add_link
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[dim PATCH 7/7] dim: propagate errors from check_maintainer

[Jani Nikula]

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 dim | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/dim b/dim
index 7b6275e7796f..989674ab7a91 100755
--- a/dim
+++ b/dim
@@ -701,7 +701,9 @@ function dim_apply_branch
 	if ! checkpatch_commit HEAD; then
 		rv=1
 	fi
-	check_maintainer $branch HEAD
+	if ! check_maintainer $branch HEAD; then
+		rv=1
+	fi
 
 	eval $DRY $DIM_POST_APPLY_ACTION
 
@@ -1090,7 +1092,7 @@ function dim_conf
 # $2 commit
 function check_maintainer
 {
-	local branch commit
+	local branch commit rv
 
 	branch=$1
 	commit=$2
@@ -1101,8 +1103,11 @@ function check_maintainer
 			echo -e "The following files are outside of i915 maintenance scope:\n"
 			echo "$non_i915_files"
 			echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files."
+			rv=1
 		fi
 	fi
+
+	return $rv
 }
 
 # $1 is the git sha1 to check
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [dim PATCH 1/7] dim: don't fail on grep not matching

[Daniel Vetter]

On Thu, Mar 23, 2017 at 12:06:16PM +0200, Jani Nikula wrote:
> Oops, the comment told us to watch out for this.
> 
> Fixes: 56e53a49e28f ("dim: declare and assign separately")

I just hacked around this with a || true :-)

Ack.
-Daniel

> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  dim | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/dim b/dim
> index c1ac9e546ea9..ddcc18f17f0d 100755
> --- a/dim
> +++ b/dim
> @@ -1091,10 +1091,7 @@ function checkpatch_commit
>  	git --no-pager log --oneline -1 $commit
>  	$cmd | scripts/checkpatch.pl -q --emacs --strict - || true
>  
> -	# FIXME: this relies on local assignment not failing on command
> -	# substitution failures
> -	bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG')
> -	if test "$bug_lines" -eq 1; then
> +	if bug_lines=$($cmd | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG') && [[ "$bug_lines" = "1" ]]; then
>  		warn_or_fail "New BUG macro added"
>  	fi
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [dim PATCH 7/7] dim: propagate errors from check_maintainer

[Daniel Vetter]

On Thu, Mar 23, 2017 at 12:06:22PM +0200, Jani Nikula wrote:
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Ack on the entire pile.
-Daniel

> ---
>  dim | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/dim b/dim
> index 7b6275e7796f..989674ab7a91 100755
> --- a/dim
> +++ b/dim
> @@ -701,7 +701,9 @@ function dim_apply_branch
>  	if ! checkpatch_commit HEAD; then
>  		rv=1
>  	fi
> -	check_maintainer $branch HEAD
> +	if ! check_maintainer $branch HEAD; then
> +		rv=1
> +	fi
>  
>  	eval $DRY $DIM_POST_APPLY_ACTION
>  
> @@ -1090,7 +1092,7 @@ function dim_conf
>  # $2 commit
>  function check_maintainer
>  {
> -	local branch commit
> +	local branch commit rv
>  
>  	branch=$1
>  	commit=$2
> @@ -1101,8 +1103,11 @@ function check_maintainer
>  			echo -e "The following files are outside of i915 maintenance scope:\n"
>  			echo "$non_i915_files"
>  			echo -e "\nConfirm you have appropriate Acked-by and Reviewed-by for above files."
> +			rv=1
>  		fi
>  	fi
> +
> +	return $rv
>  }
>  
>  # $1 is the git sha1 to check
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx