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

[Johannes Schindelin]

Hi Junio,

On Mon, 4 Apr 2016, Junio C Hamano wrote:

> Johannes Sixt  writes:
> 
> > Although I am convinced that the change is not necessary for
> > correctness, I can buy the justification that we should produce forward
> > slashes for consistency. There are a number of occasions where we
> > present paths to the user, and we do show forward-slashes in all cases
> > that I found. We should keep the commit.
> >
> > But then let's do this:
> 
> Sounds like a plan; even though I am mildly against adding more
> platform specific #ifdef to files outside compat/, this patch does
> not.
> 
> Dscho?

Fine by me!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Johannes Sixt  writes:

> Although I am convinced that the change is not necessary for
> correctness, I can buy the justification that we should produce forward
> slashes for consistency. There are a number of occasions where we
> present paths to the user, and we do show forward-slashes in all cases
> that I found. We should keep the commit.
>
> But then let's do this:

Sounds like a plan; even though I am mildly against adding more
platform specific #ifdef to files outside compat/, this patch does
not.

Dscho?

>
>  8< 
> Subject: [PATCH] Windows: shorten code by re-using convert_slashes()
>
> Make a few more spots more readable by using the recently introduced,
> Windows-specific helper.
>
> Signed-off-by: Johannes Sixt 
> ---
>  abspath.c  | 5 +
>  compat/mingw.c | 9 ++---
>  2 files changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index 5edb4e7..2825de8 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -167,7 +167,6 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
> const char *arg)
>   strbuf_add(, pfx, pfx_len);
>   strbuf_addstr(, arg);
>  #else
> - char *p;
>   /* don't add prefix to absolute paths, but still replace '\' by '/' */
>   strbuf_reset();
>   if (is_absolute_path(arg))
> @@ -175,9 +174,7 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
> const char *arg)
>   else if (pfx_len)
>   strbuf_add(, pfx, pfx_len);
>   strbuf_addstr(, arg);
> - for (p = path.buf + pfx_len; *p; p++)
> - if (*p == '\\')
> - *p = '/';
> + convert_slashes(path.buf + pfx_len);
>  #endif
>   return path.buf;
>  }
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 54c82ec..0413d5c 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -763,15 +763,12 @@ struct tm *localtime_r(const time_t *timep, struct tm 
> *result)
>  
>  char *mingw_getcwd(char *pointer, int len)
>  {
> - int i;
>   wchar_t wpointer[MAX_PATH];
>   if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
>   return NULL;
>   if (xwcstoutf(pointer, wpointer, len) < 0)
>   return NULL;
> - for (i = 0; pointer[i]; i++)
> - if (pointer[i] == '\\')
> - pointer[i] = '/';
> + convert_slashes(pointer);
>   return pointer;
>  }
>  
> @@ -2112,9 +2109,7 @@ static void setup_windows_environment()
>* executable (by not mistaking the dir separators
>* for escape characters).
>*/
> - for (; *tmp; tmp++)
> - if (*tmp == '\\')
> - *tmp = '/';
> + convert_slashes(tmp);
>   }
>  
>   /* simulate TERM to enable auto-color (see color.c) */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Sixt]

Am 29.03.2016 um 22:05 schrieb Junio C Hamano:
> Johannes Sixt  writes:
> 
>> This part of your 45bf3297 (t1300: fix the new --show-origin tests on
>> Windows)
>>
>> @@ -1205,6 +1205,9 @@ test_expect_success POSIXPERM,PERL 'preserves existing 
>> per
>>"die q(badrename) if ((stat(q(.git/config)))[2] & 0) != 0600"
>>   '
>>   
>> +! test_have_prereq MINGW ||
>> +HOME="$(pwd)" # convert to Windows path
>> +
>>   test_expect_success 'set up --show-origin tests' '
>>  INCLUDE_DIR="$HOME/include" &&
>>  mkdir -p "$INCLUDE_DIR" &&
>>
>> is actually a much more concise version of my proposed patch,
>> although the result still misuses $HOME where it does not have
>> to. In fact, if I revert 5ca6b7bb (config --show-origin: report
>> paths with forward slashes), the tests still pass. But since it
>> does not make a difference save for a few microseconds more or
>> less during startup, it is not worth the churn at this point.
> 
> Well, from the point of view of codebase cleanliness, if we can do
> without 5ca6b7bb4, we would be much better off in the longer term,
> so I would say it would be wonderful if we can safely revert it.

Although I am convinced that the change is not necessary for
correctness, I can buy the justification that we should produce forward
slashes for consistency. There are a number of occasions where we
present paths to the user, and we do show forward-slashes in all cases
that I found. We should keep the commit.

But then let's do this:

 8< 
Subject: [PATCH] Windows: shorten code by re-using convert_slashes()

Make a few more spots more readable by using the recently introduced,
Windows-specific helper.

Signed-off-by: Johannes Sixt 
---
 abspath.c  | 5 +
 compat/mingw.c | 9 ++---
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/abspath.c b/abspath.c
index 5edb4e7..2825de8 100644
--- a/abspath.c
+++ b/abspath.c
@@ -167,7 +167,6 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
const char *arg)
strbuf_add(, pfx, pfx_len);
strbuf_addstr(, arg);
 #else
-   char *p;
/* don't add prefix to absolute paths, but still replace '\' by '/' */
strbuf_reset();
if (is_absolute_path(arg))
@@ -175,9 +174,7 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
const char *arg)
else if (pfx_len)
strbuf_add(, pfx, pfx_len);
strbuf_addstr(, arg);
-   for (p = path.buf + pfx_len; *p; p++)
-   if (*p == '\\')
-   *p = '/';
+   convert_slashes(path.buf + pfx_len);
 #endif
return path.buf;
 }
diff --git a/compat/mingw.c b/compat/mingw.c
index 54c82ec..0413d5c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -763,15 +763,12 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result)
 
 char *mingw_getcwd(char *pointer, int len)
 {
-   int i;
wchar_t wpointer[MAX_PATH];
if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
return NULL;
if (xwcstoutf(pointer, wpointer, len) < 0)
return NULL;
-   for (i = 0; pointer[i]; i++)
-   if (pointer[i] == '\\')
-   pointer[i] = '/';
+   convert_slashes(pointer);
return pointer;
 }
 
@@ -2112,9 +2109,7 @@ static void setup_windows_environment()
 * executable (by not mistaking the dir separators
 * for escape characters).
 */
-   for (; *tmp; tmp++)
-   if (*tmp == '\\')
-   *tmp = '/';
+   convert_slashes(tmp);
}
 
/* simulate TERM to enable auto-color (see color.c) */
-- 
2.7.0.118.g90056ae

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Sixt]

Am 30.03.2016 um 07:52 schrieb Johannes Sixt:

Am 29.03.2016 um 21:18 schrieb Johannes Sixt:

It does pass. The reason is that pwd -W generates forward slashes.


It just occurred to me that we might be observing a difference in
behavior of pwd -W between the modern MSYS2 bash and the old MSYS1 bash
that I am using.


No that isn't it. Both versions produce forward slashes. Puzzled...

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Sixt]

Am 29.03.2016 um 21:18 schrieb Johannes Sixt:

Am 28.03.2016 um 17:14 schrieb Johannes Schindelin:

The problem with your patch is that it does not account for backslashes in
paths resulting in quoting. I am afraid that your patch will most likely
*not* let the tests pass in Git for Windows SDK, while my patch does.


It does pass. The reason is that pwd -W generates forward slashes.


It just occurred to me that we might be observing a difference in 
behavior of pwd -W between the modern MSYS2 bash and the old MSYS1 bash 
that I am using. If that is the case the tests won't pass under MSYS2 
without the change made by 5ca6b7bb (config --show-origin: report paths 
with forward slashes).


-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Johannes Sixt  writes:

> This part of your 45bf3297 (t1300: fix the new --show-origin tests on
> Windows)
>
> @@ -1205,6 +1205,9 @@ test_expect_success POSIXPERM,PERL 'preserves existing 
> per
>   "die q(badrename) if ((stat(q(.git/config)))[2] & 0) != 0600"
>  '
>  
> +! test_have_prereq MINGW ||
> +HOME="$(pwd)" # convert to Windows path
> +
>  test_expect_success 'set up --show-origin tests' '
> INCLUDE_DIR="$HOME/include" &&
> mkdir -p "$INCLUDE_DIR" &&
>
> is actually a much more concise version of my proposed patch,
> although the result still misuses $HOME where it does not have
> to. In fact, if I revert 5ca6b7bb (config --show-origin: report
> paths with forward slashes), the tests still pass. But since it
> does not make a difference save for a few microseconds more or
> less during startup, it is not worth the churn at this point.

Well, from the point of view of codebase cleanliness, if we can do
without 5ca6b7bb4, we would be much better off in the longer term,
so I would say it would be wonderful if we can safely revert it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Sixt]

Am 28.03.2016 um 17:14 schrieb Johannes Schindelin:
> Hi Hannes,
> 
> On Mon, 28 Mar 2016, Johannes Sixt wrote:
> 
>> A change like this whould have been preferable:
>> [...]
> 
> The problem with your patch is that it does not account for backslashes in
> paths resulting in quoting. I am afraid that your patch will most likely
> *not* let the tests pass in Git for Windows SDK, while my patch does.

It does pass. The reason is that pwd -W generates forward slashes.

This part of your 45bf3297 (t1300: fix the new --show-origin tests on
Windows)

@@ -1205,6 +1205,9 @@ test_expect_success POSIXPERM,PERL 'preserves existing per
  "die q(badrename) if ((stat(q(.git/config)))[2] & 0) != 0600"
 '
 
+! test_have_prereq MINGW ||
+HOME="$(pwd)" # convert to Windows path
+
 test_expect_success 'set up --show-origin tests' '
INCLUDE_DIR="$HOME/include" &&
mkdir -p "$INCLUDE_DIR" &&

is actually a much more concise version of my proposed patch,
although the result still misuses $HOME where it does not have
to. In fact, if I revert 5ca6b7bb (config --show-origin: report
paths with forward slashes), the tests still pass. But since it
does not make a difference save for a few microseconds more or
less during startup, it is not worth the churn at this point.

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Schindelin]

Hi Hannes,

On Mon, 28 Mar 2016, Johannes Sixt wrote:

> A change like this whould have been preferable:
> [...]

The problem with your patch is that it does not account for backslashes in
paths resulting in quoting. I am afraid that your patch will most likely
*not* let the tests pass in Git for Windows SDK, while my patch does.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Sixt]

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 
> ---
>   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(_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/configuser.local=true
file:.git/configuser.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
  

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

[Junio C Hamano]

Johannes Schindelin  writes:

>> > 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(_path, home);
>> > +#ifdef GIT_WINDOWS_NATIVE
>> > +  convert_slashes(user_path.buf);
>> > +#endif
>> 
>> Hmm, I wonder if we want to do this at a bit lower level,
>
> Well, I tried to be careful. There *are* circumstamces when backslashes
> are required (CreateProcess() comes to mind), so I wanted to have this
> conversion as much only in the user-visible output as possible.

I was able to guess that it would be the reason, and I was willing
to accept this as a short-term workaround.

As you are very well aware, the usual pattern we use is to implement
a higher level function (e.g. expand_user_path() in this case) in
terms of helpers that offer abstraction of implementation details
that may be platform specific (e.g. getenv() may be implemented
differently on Windows).  An "#ifdef" in otherwise platform agnostic
codepath like this one is a sign that the code is not well thought
out to find the right abstraction to use to follow that pattern.

I was mostly reacting to that "#ifdef" and thinking aloud what the
right abstraction is appropriate.  As a short-term workaround, the
above would have to do.

And no, I do not think convert_slashes() that becomes no-op on
non-windows platforms is the right abstraction.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Schindelin]

Hi Junio,

On Tue, 22 Mar 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > 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.
> 
> ... and the paths tracked by Git (in the index) use 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.
> 
> OK, sounds sensible.
> 
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> >  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(_path, home);
> > +#ifdef GIT_WINDOWS_NATIVE
> > +   convert_slashes(user_path.buf);
> > +#endif
> 
> Hmm, I wonder if we want to do this at a bit lower level,

Well, I tried to be careful. There *are* circumstamces when backslashes
are required (CreateProcess() comes to mind), so I wanted to have this
conversion as much only in the user-visible output as possible.

> e.g.
> 
> 1. have a fallback for other platforms in git-compat-util.h
> 
> #ifndef get_HOME
> #define get_HOME() getenv("HOME")
> #endif

Once upon a time, I had something like that (without the ugly uppercase,
to account for the fact that Windows has no single HOME setting):

http://thread.gmane.org/gmane.comp.version-control.msysgit/20339

> 2. have the one that does convert-slashes for mingw
> 
> static inline const char *mingw_getenv_HOME(void) {
>   ... do convert-slashes on the result of getenv("HOME");
> }
> #define get_HOME() mingw_getenv_HOME()

We could do that much easier now:

-- snip --
diff --git a/compat/mingw.c b/compat/mingw.c
index 54c82ec..96022b7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2117,6 +2117,12 @@ static void setup_windows_environment()
*tmp = '/';
}
 
+   tmp = getenv("HOME");
+   if (tmp)
+   for (; *tmp; tmp++)
+   if (*tmp == '\\')
+   *tmp = '/';
+
/* simulate TERM to enable auto-color (see color.c) */
if (!getenv("TERM"))
setenv("TERM", "cygwin", 1);
-- snap --

But again, I am uncomfortable to do this wholesale without having the time
to do anything quickly about unintended side effects.


> 3. Instead of the above patch to path.c, change the line before
>the precontext with s/getenv("HOME")/get_HOME()/
> 
> Or even lower, inside mingw_getenv() and catch variables that deal
> with paths (not just HOME but PATH, TMP, TMPDIR, etc.)

That outright scares me. Don't do that.

:-)

Again, just to make sure my point is heard: there *are* circumstances when
the Win32 API expects backslashes and cannot handle forward ones. I would
not dare to claim to be an expert in that matter, so I would rather want
to err on the side of converting backslashes to forward slashes *only*
when really needed.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Johannes Schindelin  writes:

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

... and the paths tracked by Git (in the index) use 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.

OK, sounds sensible.

>
> Signed-off-by: Johannes Schindelin 
> ---
>  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(_path, home);
> +#ifdef GIT_WINDOWS_NATIVE
> + convert_slashes(user_path.buf);
> +#endif

Hmm, I wonder if we want to do this at a bit lower level, e.g.

1. have a fallback for other platforms in git-compat-util.h

#ifndef get_HOME
#define get_HOME() getenv("HOME")
#endif

2. have the one that does convert-slashes for mingw

static inline const char *mingw_getenv_HOME(void) {
... do convert-slashes on the result of getenv("HOME");
}
#define get_HOME() mingw_getenv_HOME()

3. Instead of the above patch to path.c, change the line before
   the precontext with s/getenv("HOME")/get_HOME()/

Or even lower, inside mingw_getenv() and catch variables that deal
with paths (not just HOME but PATH, TMP, TMPDIR, etc.)

>   } else {
>   struct passwd *pw = getpw_str(username, username_len);
>   if (!pw)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[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 
---
 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(_path, home);
+#ifdef GIT_WINDOWS_NATIVE
+   convert_slashes(user_path.buf);
+#endif
} else {
struct passwd *pw = getpw_str(username, username_len);
if (!pw)
-- 
2.7.4.windows.1


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html