Re: [PATCH 1/4] config --show-origin: report paths with forward slashes
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Mar 23, 2016, 08:20 UTC
- Message-ID
- <alpine.DEB.2.20.1603230821420.4690@virtualbox>
- In-Reply-To
- <xmqqbn66iedh.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 22 Mar 2016, Junio C Hamano wrote:
Show 54 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> 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 <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
>
> 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.
Show 7 quoted lines
> e.g.
>
> 1. have a fallback for other platforms in git-compat-util.h
>
> #ifndef get_HOME
> #define get_HOME() getenv("HOME")
> #endifOnce 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
Show 6 quoted lines
> 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