[dim PATCH 1/2] dim: add -s option to treat unset variables as an error

[dim PATCH 1/2] dim: add -s option to treat unset variables as an error

[Jani Nikula]

Use -s for "strict" to 'set -u'. Fix the variable references to not fail
before subcommand call. The goal is to unconditionally 'set -u' at the
top, but it can't be done in one go, so add a way to test drive first.

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 99120b82513e..3e10f360d482 100755
--- a/dim
+++ b/dim
@@ -145,7 +145,7 @@ function pause
 	echo
 }
 
-while getopts hdfi opt; do
+while getopts hdfis opt; do
 	case "$opt" in
 		d)
 			DRY_RUN=--dry-run
@@ -160,6 +160,11 @@ while getopts hdfi opt; do
 		h)
 			HELP=1
 			;;
+		s)
+			# FIXME: transitional, do unconditionally at the top
+			# when there are no more errors about unbound variables
+			set -u
+			;;
 		*)
 			echoerr "See '$dim help' for more information."
 			exit
@@ -180,7 +185,7 @@ fi
 #
 
 # Make sure we use 'dim_foo' within dim instead of 'dim foo'.
-if [[ -n "$__dim_running" ]]; then
+if [[ -n "${__dim_running:-}" ]]; then
 	echoerr "INTERNAL ERROR: do not recurse back to dim"
 	exit 1
 fi
@@ -1829,7 +1834,7 @@ function dim_usage
 
 # dim subcommand aliases (with bash 4.3+)
 if ! declare -n subcmd=dim_alias_${subcommand//-/_} &> /dev/null || \
-		test -z "$subcmd"; then
+		test -z "${subcmd:-}"; then
 	subcmd="$subcommand"
 fi
 
-- 
2.1.4

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

[dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments

[Jani Nikula]

This will print a generic usage error message and bail out on required
arguments missing. This is in preparation for enabling 'set -u'.

This is not an exhaustive conversion, but a good start.

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

diff --git a/dim b/dim
index 3e10f360d482..463086b6f290 100755
--- a/dim
+++ b/dim
@@ -180,6 +180,9 @@ else
     shift
 fi
 
+# generic usage to be used for ${1:?$usage} style argument references
+usage="Missing arguments(s) for '$dim $subcommand'. See '$dim help' for usage."
+
 #
 # Sanity checks.
 #
@@ -487,8 +490,12 @@ function update_rerere_cache
 
 function dim_revert_rerere
 {
+	local commit
+
+	commit=${1:?$usage}
+
 	cd $DIM_PREFIX/drm-rerere/
-	git revert $1
+	git revert $commit
 	rm $(rr_cache_dir)/* -Rf
 }
 
@@ -628,12 +635,7 @@ function dim_push_branch
 {
 	local branch remote
 
-	if [[ "x$1" = "x" ]]; then
-		echo "usage: $dim $subcommand branch"
-		exit 1
-	fi
-
-	branch=$1
+	branch=${1:?$usage}
 	shift
 
 	assert_branch $branch
@@ -674,7 +676,7 @@ function dim_apply_branch
 {
 	local branch file message_id commiter_email patch_from sob rv
 
-	branch=$1
+	branch=${1:?$usage}
 	shift
 	file=$(mktemp)
 
@@ -719,7 +721,7 @@ function dim_add_link
 {
 	local branch file message_id
 
-	branch=$1
+	branch=${1:?$usage}
 	shift
 	file=$(mktemp)
 
@@ -800,17 +802,13 @@ function dim_cherry_pick
 {
 	local commit
 
-	if [[ "x$1" = "x" ]]; then
-		echo "usage: $dim $subcommand commit-ish"
-		exit 1
-	fi
-	commit=$(git rev-parse $1)
+	commit=$(git rev-parse ${1:?$usage})
 
 	git_fetch_helper $remote
 
 	commit_list_references $commit
 
-	$DRY git cherry-pick -s -x -e $1
+	$DRY git cherry-pick -s -x -e $commit
 }
 
 function git_list_fixes
@@ -826,7 +824,7 @@ function dim_cherry_pick_branch
 {
 	local branch log fail_log needed have_fixes
 
-	branch="$1"
+	branch=${1:?$usage}
 	log=$(mktemp)
 	fail_log=$(mktemp)
 
@@ -947,19 +945,10 @@ function dim_create_branch
 {
 	local branch repo remote
 
-	if [[ "x$1" = "x" ]]; then
-		echo "usage: $dim $subcommand branch [commit-ish]"
-		exit 1
-	fi
-	branch=$1
+	branch=${1:?$usage}
+	start=${2:-HEAD}
 	repo="drm-intel"
 
-	if [[ "x$2" = "x" ]]; then
-		start=HEAD
-	else
-		start=$2
-	fi
-
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
 
 	if ( repo_to_remote ${branch%%/*} ) &> /dev/null ; then
@@ -983,11 +972,7 @@ function dim_remove_branch
 {
 	local branch repo remote
 
-	if [[ "x$1" = "x" ]]; then
-		echo "usage: $dim $subcommand branch"
-		exit 1
-	fi
-	branch=$1
+	branch=${1:?$usage}
 
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
 
@@ -1040,11 +1025,7 @@ function dim_checkout
 {
 	local branch repo remote
 
-	if [[ "x$1" = "x" ]]; then
-		echo "usage: $dim $subcommand branch"
-		exit 1
-	fi
-	branch=$1
+	branch=${1:?$usage}
 
 	dim_cd $branch
 	if ! git_branch_exists $branch ; then
@@ -1145,9 +1126,8 @@ function dim_extract_tags
 {
 	local branch range file tags
 
-	branch=$1
-	shift
-	range=$(rangeish "$1")
+	branch=${1:?$usage}
+	range=$(rangeish "${2:-}")
 	file=$(mktemp)
 
 	assert_branch $branch
@@ -1189,7 +1169,7 @@ function dim_checkpatch
 {
 	local range rv
 
-	range=$(rangeish "$1")
+	range=$(rangeish "${1:-}")
 
 	for commit in $(git rev-list --reverse $range); do
 		if ! checkpatch_commit $commit; then
@@ -1204,7 +1184,7 @@ function dim_sparse
 {
 	local range
 
-	range=$(rangeish "$1")
+	range=$(rangeish "${1:-}")
 
 	make $DIM_MAKE_OPTIONS
 	touch --no-create $(git diff --name-only $range) $(git diff --name-only)
@@ -1433,13 +1413,8 @@ function dim_pull_request
 {
 	local branch upstream remote repo url git_url suffix tag
 
-	if [[ "x$1" = "x" || "x$2" = "x" ]]; then
-		echo "usage: $dim $subcommand branch upstream"
-		exit 1
-	fi
-
-	branch=$1
-	upstream=$2
+	branch=${1:?$usage}
+	upstream=${2:?$usage}
 	remote=$(branch_to_remote $branch)
 
 	if [ "$branch" != "drm-intel-next" ]; then
@@ -1721,16 +1696,18 @@ function dim_cat_to_fixup
 
 function dim_tc
 {
-	local tag dim_drm_upstream_remote
+	local sha1 tag dim_drm_upstream_remote
+
+	sha1=${1:?$usage}
 
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
-	tag=$(git tag --contains $1 | grep ^v | sort -V | head -n 1)
+	tag=$(git tag --contains $sha1 | grep ^v | sort -V | head -n 1)
 	if [[ -n "$tag" ]]; then
 		echo "$tag"
 	else
 		dim_drm_upstream_remote=$(url_to_remote $drm_upstream_git)
 		# not in a tagged release, show upstream branches
-		git branch -r --contains $1 \
+		git branch -r --contains $sha1 \
 		    $DIM_DRM_INTEL_REMOTE/* \
 		    $dim_drm_upstream_remote/drm-next \
 		    $dim_drm_upstream_remote/drm-fixes \
@@ -1742,11 +1719,7 @@ function dim_cite
 {
 	local sha1
 
-	sha1=$1
-	if [[ -z "$sha1" ]]; then
-		echoerr "usage: $dim $subcommand <commit-ish>"
-		return 1
-	fi
+	sha1=${1:?$usage}
 
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
 
@@ -1758,11 +1731,7 @@ function dim_fixes
 {
 	local sha1 tag
 
-	sha1=$1
-	if [[ -z "$sha1" ]]; then
-		echoerr "usage: $dim $subcommand <commit-ish>"
-		return 1
-	fi
+	sha1=${1:?$usage}
 
 	cd $DIM_PREFIX/$DIM_DRM_INTEL
 	echo "Fixes: $(dim_cite $sha1)"
@@ -1774,7 +1743,7 @@ function dim_fixes
 		git show $sha1 | scripts/get_maintainer.pl  --email --norolestats --pattern-depth 1 | sed -e "s/^/Cc: /"
 	) | awk '!x[$0]++'
 
-	tag=$(git tag --contains $1 | grep ^v | sort -V | head -n 1)
+	tag=$(git tag --contains $sha1 | grep ^v | sort -V | head -n 1)
 	if [[ -n "$tag" ]]; then
 		if echo "$tag" | grep -q -e "-rc" ; then
 			echo "Cc: <drm-intel-fixes@lists.freedesktop.org> # ${tag}+"
-- 
2.1.4

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

Re: [dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments

[Daniel Vetter]

On Tue, Mar 28, 2017 at 12:51:26PM +0300, Jani Nikula wrote:
> This will print a generic usage error message and bail out on required
> arguments missing. This is in preparation for enabling 'set -u'.
> 
> This is not an exhaustive conversion, but a good start.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I think given that a lot of commands simply failed to check for their
arguments (and then sometimes even silently fell over) this is a good
improvement. Ack on both.
-Daniel

> ---
>  dim | 95 +++++++++++++++++++++++----------------------------------------------
>  1 file changed, 32 insertions(+), 63 deletions(-)
> 
> diff --git a/dim b/dim
> index 3e10f360d482..463086b6f290 100755
> --- a/dim
> +++ b/dim
> @@ -180,6 +180,9 @@ else
>      shift
>  fi
>  
> +# generic usage to be used for ${1:?$usage} style argument references
> +usage="Missing arguments(s) for '$dim $subcommand'. See '$dim help' for usage."
> +
>  #
>  # Sanity checks.
>  #
> @@ -487,8 +490,12 @@ function update_rerere_cache
>  
>  function dim_revert_rerere
>  {
> +	local commit
> +
> +	commit=${1:?$usage}
> +
>  	cd $DIM_PREFIX/drm-rerere/
> -	git revert $1
> +	git revert $commit
>  	rm $(rr_cache_dir)/* -Rf
>  }
>  
> @@ -628,12 +635,7 @@ function dim_push_branch
>  {
>  	local branch remote
>  
> -	if [[ "x$1" = "x" ]]; then
> -		echo "usage: $dim $subcommand branch"
> -		exit 1
> -	fi
> -
> -	branch=$1
> +	branch=${1:?$usage}
>  	shift
>  
>  	assert_branch $branch
> @@ -674,7 +676,7 @@ function dim_apply_branch
>  {
>  	local branch file message_id commiter_email patch_from sob rv
>  
> -	branch=$1
> +	branch=${1:?$usage}
>  	shift
>  	file=$(mktemp)
>  
> @@ -719,7 +721,7 @@ function dim_add_link
>  {
>  	local branch file message_id
>  
> -	branch=$1
> +	branch=${1:?$usage}
>  	shift
>  	file=$(mktemp)
>  
> @@ -800,17 +802,13 @@ function dim_cherry_pick
>  {
>  	local commit
>  
> -	if [[ "x$1" = "x" ]]; then
> -		echo "usage: $dim $subcommand commit-ish"
> -		exit 1
> -	fi
> -	commit=$(git rev-parse $1)
> +	commit=$(git rev-parse ${1:?$usage})
>  
>  	git_fetch_helper $remote
>  
>  	commit_list_references $commit
>  
> -	$DRY git cherry-pick -s -x -e $1
> +	$DRY git cherry-pick -s -x -e $commit
>  }
>  
>  function git_list_fixes
> @@ -826,7 +824,7 @@ function dim_cherry_pick_branch
>  {
>  	local branch log fail_log needed have_fixes
>  
> -	branch="$1"
> +	branch=${1:?$usage}
>  	log=$(mktemp)
>  	fail_log=$(mktemp)
>  
> @@ -947,19 +945,10 @@ function dim_create_branch
>  {
>  	local branch repo remote
>  
> -	if [[ "x$1" = "x" ]]; then
> -		echo "usage: $dim $subcommand branch [commit-ish]"
> -		exit 1
> -	fi
> -	branch=$1
> +	branch=${1:?$usage}
> +	start=${2:-HEAD}
>  	repo="drm-intel"
>  
> -	if [[ "x$2" = "x" ]]; then
> -		start=HEAD
> -	else
> -		start=$2
> -	fi
> -
>  	cd $DIM_PREFIX/$DIM_DRM_INTEL
>  
>  	if ( repo_to_remote ${branch%%/*} ) &> /dev/null ; then
> @@ -983,11 +972,7 @@ function dim_remove_branch
>  {
>  	local branch repo remote
>  
> -	if [[ "x$1" = "x" ]]; then
> -		echo "usage: $dim $subcommand branch"
> -		exit 1
> -	fi
> -	branch=$1
> +	branch=${1:?$usage}
>  
>  	cd $DIM_PREFIX/$DIM_DRM_INTEL
>  
> @@ -1040,11 +1025,7 @@ function dim_checkout
>  {
>  	local branch repo remote
>  
> -	if [[ "x$1" = "x" ]]; then
> -		echo "usage: $dim $subcommand branch"
> -		exit 1
> -	fi
> -	branch=$1
> +	branch=${1:?$usage}
>  
>  	dim_cd $branch
>  	if ! git_branch_exists $branch ; then
> @@ -1145,9 +1126,8 @@ function dim_extract_tags
>  {
>  	local branch range file tags
>  
> -	branch=$1
> -	shift
> -	range=$(rangeish "$1")
> +	branch=${1:?$usage}
> +	range=$(rangeish "${2:-}")
>  	file=$(mktemp)
>  
>  	assert_branch $branch
> @@ -1189,7 +1169,7 @@ function dim_checkpatch
>  {
>  	local range rv
>  
> -	range=$(rangeish "$1")
> +	range=$(rangeish "${1:-}")
>  
>  	for commit in $(git rev-list --reverse $range); do
>  		if ! checkpatch_commit $commit; then
> @@ -1204,7 +1184,7 @@ function dim_sparse
>  {
>  	local range
>  
> -	range=$(rangeish "$1")
> +	range=$(rangeish "${1:-}")
>  
>  	make $DIM_MAKE_OPTIONS
>  	touch --no-create $(git diff --name-only $range) $(git diff --name-only)
> @@ -1433,13 +1413,8 @@ function dim_pull_request
>  {
>  	local branch upstream remote repo url git_url suffix tag
>  
> -	if [[ "x$1" = "x" || "x$2" = "x" ]]; then
> -		echo "usage: $dim $subcommand branch upstream"
> -		exit 1
> -	fi
> -
> -	branch=$1
> -	upstream=$2
> +	branch=${1:?$usage}
> +	upstream=${2:?$usage}
>  	remote=$(branch_to_remote $branch)
>  
>  	if [ "$branch" != "drm-intel-next" ]; then
> @@ -1721,16 +1696,18 @@ function dim_cat_to_fixup
>  
>  function dim_tc
>  {
> -	local tag dim_drm_upstream_remote
> +	local sha1 tag dim_drm_upstream_remote
> +
> +	sha1=${1:?$usage}
>  
>  	cd $DIM_PREFIX/$DIM_DRM_INTEL
> -	tag=$(git tag --contains $1 | grep ^v | sort -V | head -n 1)
> +	tag=$(git tag --contains $sha1 | grep ^v | sort -V | head -n 1)
>  	if [[ -n "$tag" ]]; then
>  		echo "$tag"
>  	else
>  		dim_drm_upstream_remote=$(url_to_remote $drm_upstream_git)
>  		# not in a tagged release, show upstream branches
> -		git branch -r --contains $1 \
> +		git branch -r --contains $sha1 \
>  		    $DIM_DRM_INTEL_REMOTE/* \
>  		    $dim_drm_upstream_remote/drm-next \
>  		    $dim_drm_upstream_remote/drm-fixes \
> @@ -1742,11 +1719,7 @@ function dim_cite
>  {
>  	local sha1
>  
> -	sha1=$1
> -	if [[ -z "$sha1" ]]; then
> -		echoerr "usage: $dim $subcommand <commit-ish>"
> -		return 1
> -	fi
> +	sha1=${1:?$usage}
>  
>  	cd $DIM_PREFIX/$DIM_DRM_INTEL
>  
> @@ -1758,11 +1731,7 @@ function dim_fixes
>  {
>  	local sha1 tag
>  
> -	sha1=$1
> -	if [[ -z "$sha1" ]]; then
> -		echoerr "usage: $dim $subcommand <commit-ish>"
> -		return 1
> -	fi
> +	sha1=${1:?$usage}
>  
>  	cd $DIM_PREFIX/$DIM_DRM_INTEL
>  	echo "Fixes: $(dim_cite $sha1)"
> @@ -1774,7 +1743,7 @@ function dim_fixes
>  		git show $sha1 | scripts/get_maintainer.pl  --email --norolestats --pattern-depth 1 | sed -e "s/^/Cc: /"
>  	) | awk '!x[$0]++'
>  
> -	tag=$(git tag --contains $1 | grep ^v | sort -V | head -n 1)
> +	tag=$(git tag --contains $sha1 | grep ^v | sort -V | head -n 1)
>  	if [[ -n "$tag" ]]; then
>  		if echo "$tag" | grep -q -e "-rc" ; then
>  			echo "Cc: <drm-intel-fixes@lists.freedesktop.org> # ${tag}+"
> -- 
> 2.1.4
> 

-- 
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