Re: [PATCH blktests 1/3] check: Add configuration file option

From: "André Almeida" <andrealmeid@collabora.com>
To: Omar Sandoval <osandov@osandov.com>
Cc: linux-block@vger.kernel.org, osandov@fb.com,
	kernel@collabora.com, krisman@collabora.com
Subject: Re: [PATCH blktests 1/3] check: Add configuration file option
Date: Wed, 30 Oct 2019 19:05:14 -0300	[thread overview]
Message-ID: <2f9f649b-31db-b774-5c41-1d5f4340881e@collabora.com> (raw)
In-Reply-To: <20191030211652.GB326591@vader>

On 10/30/19 6:16 PM, Omar Sandoval wrote:
> On Tue, Oct 29, 2019 at 05:09:40PM -0300, André Almeida wrote:
>> Add an option to be possible to use a different configuration file
>> rather than the default "config" file.
>>
>> Signed-off-by: André Almeida <andrealmeid@collabora.com>
>> ---
>>  check | 29 ++++++++++++++++++++++++-----
>>  1 file changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/check b/check
>> index 981058c..936b0c3 100755
>> --- a/check
>> +++ b/check
>> @@ -635,6 +635,9 @@ Test runs:
>>    -x, --exclude=TEST	 exclude a test (or test group) from the list of
>>  			 tests to run
>>  
>> +  -c, --config=FILE	 use FILE for loading configuration instead of
>> +			 default 'config' filename.
>> +
>>  Miscellaneous:
>>    -h, --help             display this help message and exit"
>>  
>> @@ -650,7 +653,7 @@ Miscellaneous:
>>  	esac
>>  }
>>  
>> -if ! TEMP=$(getopt -o 'do:q::x:h' --long 'quick::,exclude:,output:,help' -n "$0" -- "$@"); then
>> +if ! TEMP=$(getopt -o 'do:q::x:c:h' --long 'quick::,exclude:,output:,config:,help' -n "$0" -- "$@"); then
>>  	exit 1
>>  fi
>>  
>> @@ -659,10 +662,8 @@ unset TEMP
>>  
>>  LOGGER_PROG="$(type -P logger)" || LOGGER_PROG=true
>>  
>> -if [[ -r config ]]; then
>> -	# shellcheck disable=SC1091
>> -	. config
>> -fi
>> +# true if the default configuration file "config" should be used
>> +DEFAULT_CONFIG=true
>>  
>>  # Default configuration.
>>  : "${DEVICE_ONLY:=0}"
>> @@ -706,6 +707,17 @@ while true; do
>>  			EXCLUDE+=("$2")
>>  			shift 2
>>  			;;
>> +		'-c'|'--config')
>> +			if [[ -r "$2" ]]; then
>> +				# shellcheck source=/dev/null
>> +				. "$2"
>> +				DEFAULT_CONFIG=false
>> +			else
>> +				echo "Configuration file $2 not found!"
>> +				usage err
>> +			fi
>> +			shift 2
>> +			;;
> 
> If the user specifies -c multiple times, it will source each one, not
> just the last one. Is that intentional? That might actually be a useful
> feature, but we'd want to document it.

Hmm, good question. Actually, I wasn't thinking about that when I
created this feature. I believe that using all the multiple `-c <file>`
is more useful than just using the last one. I will send a v2
documenting this behavior.

> 
>>  		'-h'|'--help')
>>  			usage out
>>  			;;
>> @@ -719,6 +731,13 @@ while true; do
>>  	esac
>>  done
>>  
>> +# if '-c' was not used, try to use the default config file
>> +if [[ -r config ]] && $DEFAULT_CONFIG; then
>> +	# shellcheck source=/dev/null
>> +	. config
>> +fi
>> +
>> +
>>  if [[ $QUICK_RUN -ne 0 && ! "${TIMEOUT:-}" ]]; then
>>  	_error "QUICK_RUN specified without TIMEOUT"
>>  fi
>> -- 
>> 2.23.0
>>