mingw: make is_hidden tests in t0001/t5611 more robust

Commit Message

Linus Arver via GitGitGadget April 8, 2020, 7:34 p.m. UTC

From: Johannes Schindelin <[e-mail]>

We should not actually expect the first `attrib.exe` in the PATH to
be the one we are looking for. Or that it is in the PATH, for that
matter.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
    Make the tests that test core.hideDotFiles more robust
    
    We have this feature on Windows where the files starting with a dot can
    be marked hidden (whether a file is hidden by default or not is a matter
    of naming convention on Unix, but it is an explicit flag on Windows).
    This patch improves the regression tests of this feature, and it has
    been carried in Git for Windows for over three years.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-603%2Fdscho%2Frobustify-is-hidden-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-603/dscho/robustify-is-hidden-tests-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/603

 t/t0001-init.sh         | 2 +-
 t/t5611-clone-config.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


base-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775

Comments

Junio C Hamano April 8, 2020, 9:59 p.m. UTC | #1

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

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


> 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

Patch

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