Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 14, 2018, 17:03 UTC
Message-ID: <xmqqd0tgc8pn.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqqmuskc8wj.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 36 quoted lines

> Ben Peart <benpeart@microsoft.com> writes:
>
>> +if test -n "$GIT_FSMONITOR_TEST"
>> +then
>> +	if test -n "$GIT_TEST_FSMONITOR"
>> +	then
>> +		echo "warning: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
>> +	else
>> +		echo "error: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR"
>> +		exit 1
>> +	fi
>> +fi
>
> I would have expected that, because we are now doing multiple pairs
> of variables in a single series, we would add a helper function that
> can be called like so:
>
> 	check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>
> in the earliest step.  Perhaps something like this.
>
> check_var_migration () {
> 	old_name=$1 new_name=$2
> 	eval "old_isset=\${${old_name}:+isset}"
> 	eval "new_isset=\${${new_name}:+isset}"
> 	case "$old_isset,$new_isset" in
> 	isset,)
> 		echo >&2 "error: $old_name is now $new_name"
> 		exit 1 ;;
> 	isset,isset)
> 		# enable this, once $old_name no longer is valid anywhere
> 		# echo >&2 "warning: $old_name is now $new_name"
> 		# echo >&2 "hint: remove $old_name"
> 		;;
> 	esac
> }

Alternatively, we could do this, to warn and then migrate the value given to the old variable automatically to the new variable and let the test proceed.

check_var_migration () {
	old_name=$1 new_name=$2
	eval "old_isset=\${${old_name}:+isset}"
	eval "new_isset=\${${new_name}:+isset}"
	case "$old_isset,$new_isset" in
	isset,)
		echo >&2 "warning: $old_name is now $new_name"
		echo >&2 "hint: set $new_name too during the transition period"
		eval "$new_name=\$$old_name"
		;;
	isset,isset)
		# do this later
		# echo >&2 "warning: $old_name is now $new_name"
		# echo >&2 "hint: remove $old_name"
		;;
	esac
}