Re: [PATCH] mingw: make is_hidden tests in t0001/t5611 more robust

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2020, 21:59 UTC
Message-ID: <xmqqmu7locys.fsf@gitster.c.googlers.com>
In-Reply-To: <pull.603.git.1586374474512.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index 26f82063267..2456688b281 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -395,7 +395,7 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '
>  # Tests for the hidden file attribute on windows
>  is_hidden () {
>  	# Use the output of `attrib`, ignore the absolute path
> -	case "$(attrib "$1")" in *H*?:*) return 0;; esac
> +	case "$("$SYSTEMROOT"/system32/attrib "$1")" in *H*?:*) return 0;; esac
>  	return 1
>  }

I wondered if this (and the other one) want to be in test-lib but I am on the fence. All three tests that call this helper in t0001 are protected with MINGW prerequisite, but until I realized it, the call to "attrib" (whether it is given as a full path or relies on $PATH lookup) looked like a portability nightmare waiting to happen. It would make it even worse if we moved the above as-is to test-lib, as it is harder to see what the callers are doing once we did so.

With a change like this, however

	is_hidden () {
		if ! test_have_prereq MINGW
		then
			BUG "use of is_hidden outside MINGW prerequisite"
		fi
		case "$("$SYSTEMROOT"/system32/attrib "$1")" in 
		*H*?:*)	return 0 ;;
		*)	return 1 ;;
		esac
	}

I think it is OK to consolidate these two copies into one in test-lib

Thanks.

Show 15 quoted lines

> diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh
> index 60c1ba951b7..87b8073cd74 100755
> --- a/t/t5611-clone-config.sh
> +++ b/t/t5611-clone-config.sh
> @@ -95,7 +95,7 @@ test_expect_success 'clone -c remote.<remote>.fetch=<refspec> --origin=<name>' '
>  # Tests for the hidden file attribute on windows
>  is_hidden () {
>  	# Use the output of `attrib`, ignore the absolute path
> -	case "$(attrib "$1")" in *H*?:*) return 0;; esac
> +	case "$("$SYSTEMROOT"/system32/attrib "$1")" in *H*?:*) return 0;; esac
>  	return 1
>  }
>  
>
> base-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775