Re: [PATCH 1/4] config --show-origin: report paths with forward slashes

[Johannes Sixt <j6t@kdbg.org>]

Am 22.03.2016 um 18:42 schrieb Johannes Schindelin:
> On Windows, the backslash is the native directory separator, but all
> supported Windows versions also accept the forward slash in most
> circumstances.
> 
> Our tests expect forward slashes.
> 
> Relative paths are generated by Git using forward slashes.
> 
> So let's try to be consistent and use forward slashes in the $HOME part
> of the paths reported by `git config --show-origin`, too.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   compat/mingw.h | 6 ++++++
>   path.c         | 3 +++
>   2 files changed, 9 insertions(+)
> 
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 8c5bf50..c008694 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -396,6 +396,12 @@ static inline char *mingw_find_last_dir_sep(const char *path)
>   			ret = (char *)path;
>   	return ret;
>   }
> +static inline void convert_slashes(char *path)
> +{
> +	for (; *path; path++)
> +		if (*path == '\\')
> +			*path = '/';
> +}
>   #define find_last_dir_sep mingw_find_last_dir_sep
>   int mingw_offset_1st_component(const char *path);
>   #define offset_1st_component mingw_offset_1st_component
> diff --git a/path.c b/path.c
> index 8b7e168..969b494 100644
> --- a/path.c
> +++ b/path.c
> @@ -584,6 +584,9 @@ char *expand_user_path(const char *path)
>   			if (!home)
>   				goto return_null;
>   			strbuf_addstr(&user_path, home);
> +#ifdef GIT_WINDOWS_NATIVE
> +			convert_slashes(user_path.buf);
> +#endif
>   		} else {
>   			struct passwd *pw = getpw_str(username, username_len);
>   			if (!pw)
> 

This change is not warranted for the following reasons:

- It is only there to satisfy the --show-origin tests, but not
  for the benefit of the users.

- The use of $HOME in those tests is just incidental, but not necessary.

- There is no reason to change only paths depending on $HOME,
  but no other paths imported through the environment.

I see no advantage for the users of --show-origin that they now
see C:/foo/bar instead of C:\foo\bar. (For this reason, I'm not
suggesting to change all paths imported from the environment, just
the contrary, to leave them all alone).

A change like this whould have been preferable:

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6767da8..4c96044 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1209,7 +1209,7 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '
 HOME="$(pwd)" # convert to Windows path
 
 test_expect_success 'set up --show-origin tests' '
-	INCLUDE_DIR="$HOME/include" &&
+	INCLUDE_DIR="$(pwd)/include" &&
 	mkdir -p "$INCLUDE_DIR" &&
 	cat >"$INCLUDE_DIR"/absolute.include <<-\EOF &&
 		[user]
@@ -1219,7 +1219,7 @@ test_expect_success 'set up --show-origin tests' '
 		[user]
 			relative = include
 	EOF
-	cat >"$HOME"/.gitconfig <<-EOF &&
+	cat >"$(pwd)"/.gitconfig <<-EOF &&
 		[user]
 			global = true
 			override = global
@@ -1237,9 +1237,9 @@ test_expect_success 'set up --show-origin tests' '
 
 test_expect_success '--show-origin with --list' '
 	cat >expect <<-EOF &&
-		file:$HOME/.gitconfig	user.global=true
-		file:$HOME/.gitconfig	user.override=global
-		file:$HOME/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
+		file:$(pwd)/.gitconfig	user.global=true
+		file:$(pwd)/.gitconfig	user.override=global
+		file:$(pwd)/.gitconfig	include.path=$INCLUDE_DIR/absolute.include
 		file:$INCLUDE_DIR/absolute.include	user.absolute=include
 		file:.git/config	user.local=true
 		file:.git/config	user.override=local
@@ -1253,9 +1253,9 @@ test_expect_success '--show-origin with --list' '
 
 test_expect_success '--show-origin with --list --null' '
 	cat >expect <<-EOF &&
-		file:$HOME/.gitconfigQuser.global
-		trueQfile:$HOME/.gitconfigQuser.override
-		globalQfile:$HOME/.gitconfigQinclude.path
+		file:$(pwd)/.gitconfigQuser.global
+		trueQfile:$(pwd)/.gitconfigQuser.override
+		globalQfile:$(pwd)/.gitconfigQinclude.path
 		$INCLUDE_DIR/absolute.includeQfile:$INCLUDE_DIR/absolute.includeQuser.absolute
 		includeQfile:.git/configQuser.local
 		trueQfile:.git/configQuser.override
@@ -1284,7 +1284,7 @@ test_expect_success '--show-origin with single file' '
 
 test_expect_success '--show-origin with --get-regexp' '
 	cat >expect <<-EOF &&
-		file:$HOME/.gitconfig	user.global true
+		file:$(pwd)/.gitconfig	user.global true
 		file:.git/config	user.local true
 	EOF
 	git config --show-origin --get-regexp "user\.[g|l].*" >output &&

But it seems I'm late in the game. Can't I take a break for a week
without something odd happening to the Windows port...?

-- Hannes