Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ramsay Jones <ramsay@ramsayjones.plus.com>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Date: Wed, 7 Nov 2018 12:19:22 +0100 (STD)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1811071217280.39@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <xmqqo9b1d6na.fsf@gitster-ct.c.googlers.com>

Hi,

On Wed, 7 Nov 2018, Junio C Hamano wrote:

> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > On 06/11/2018 14:53, Johannes Schindelin via GitGitGadget wrote:
> >> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >> 
> >> On Windows, an absolute POSIX path needs to be turned into a Windows
> >> one.
> >> 
> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >> ---
> >>  path.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >> 
> >> diff --git a/path.c b/path.c
> >> index 34f0f98349..a72abf0e1f 100644
> >> --- a/path.c
> >> +++ b/path.c
> >> @@ -11,6 +11,7 @@
> >>  #include "path.h"
> >>  #include "packfile.h"
> >>  #include "object-store.h"
> >> +#include "exec-cmd.h"
> >>  
> >>  static int get_st_mode_bits(const char *path, int *mode)
> >>  {
> >> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)
> >>  
> >>  	if (path == NULL)
> >>  		goto return_null;
> >> +#ifdef __MINGW32__
> >> +	if (path[0] == '/')
> >> +		return system_path(path + 1);
> >> +#endif
> >
> > Hmm, this doesn't quite fit with the intended use of this
> > function! ;-) (even on windows!)
> >
> > I haven't looked very deeply, but doesn't this affect all
> > absolute paths in the config read by git_config_pathname(),
> > along with all 'included config' files?
> 
> I think so.  I have not thought things through to say if replacing a
> "full path in the current drive" with system_path() is a sensible
> thing to do in the first place, but I am getting the impression from
> review comments that it probably is not.
> 
> > I am pretty sure that I would not want the absolute paths
> > in my config file(s) magically 'moved' depending on whether
> > git has been compiled with 'runtime prefix' support or not!

The cute thing is: your absolute paths would not be moved because we are
talking about Windows. Therefore your absolute paths would not start with
a forward slash.

> In any case, the helper is about expanding ~/foo and ~who/foo to
> absolute paths, without touching other paths, so it is a wrong
> function to implement it in, even if the motivation were sensible.

It could be renamed. In any case, for this feature we would need to expand
a path that is not the final path, and this here location is the most
logical place to do so.

Ciao,
Dscho