* [dim PATCH 1/2] dim: add -s option to treat unset variables as an error
@ 2017-03-28 9:51 Jani Nikula
2017-03-28 9:51 ` [dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments Jani Nikula
0 siblings, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2017-03-28 9:51 UTC (permalink / raw)
To: intel-gfx; +Cc: 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
^ permalink raw reply related [flat|nested] 3+ messages in thread* [dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments 2017-03-28 9:51 [dim PATCH 1/2] dim: add -s option to treat unset variables as an error Jani Nikula @ 2017-03-28 9:51 ` Jani Nikula 2017-03-28 14:33 ` Daniel Vetter 0 siblings, 1 reply; 3+ messages in thread From: Jani Nikula @ 2017-03-28 9:51 UTC (permalink / raw) To: intel-gfx; +Cc: 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments 2017-03-28 9:51 ` [dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments Jani Nikula @ 2017-03-28 14:33 ` Daniel Vetter 0 siblings, 0 replies; 3+ messages in thread From: Daniel Vetter @ 2017-03-28 14:33 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-28 14:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-28 9:51 [dim PATCH 1/2] dim: add -s option to treat unset variables as an error Jani Nikula
2017-03-28 9:51 ` [dim PATCH 2/2] dim: start using ${1:?$usage} to refer to arguments Jani Nikula
2017-03-28 14:33 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox