RE: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
- Date: Fri, 9 Nov 2018 02:05:48 +0000
- From: Joseph Moisan <joseph.moisan@xxxxxxx>
- Subject: RE: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Can someone please tell me how to unsubscribe from this email. I am no longer interested in receiving these emails, and cannot find how to unsubscribe.
Thank you in advance.
Joseph Moisan | Software Engineer
Building Technologies and Solutions
From: git-owner@xxxxxxxxxxxxxxx [mailto:git-owner@xxxxxxxxxxxxxxx] On Behalf Of Junio C Hamano
Sent: Wednesday, November 7, 2018 7:17 PM
To: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
Cc: Johannes Schindelin <Johannes.Schindelin@xxxxxx>; Johannes Schindelin via GitGitGadget <gitgitgadget@xxxxxxxxx>; git@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> writes:
>> 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.
> Ah, sorry, I must have misunderstood a comment in your cover letter:
> The reason is this: something like this (make paths specified e.g. via
> http.sslCAInfo relative to the runtime prefix) is potentially useful
> also in the non-Windows context, as long as Git was built with the
> runtime prefix feature.
> ... so I thought you meant to add this code for POSIX systems as well.
> My mistake. :(
Well, I do not think you should feel so bad about it, as you are not alone.
It wasn't clear to me either that it was to introduce a new syntax that users would not have otherwise used before (i.e. avoids negatively affecting existing settings---because the users would have used a path that begins with a backslash if they meant "full path down from the top of the current drive") to mean a new thing (i.e. "relative to the runtime prefix") that the patch wanted to do.
If that is the motivation, then it does make sense to extend this function, and possibly rename it, like this patch does, as we would want this new syntax available in anywhere we take a pathname to an untracked thing. And for such a pathname, we should be consistently using expand_user_path() *anyway* even without this new "now we know how to spell 'relative to the runtime prefix'" feature.
So I agree with the patch that the issue it wants to address is worth addressing, and the function to enhance is this one.
I am still unsure about the solution, though.
I suspect that any build that uses runtime prefix would want access to this feature, regardless of the platform. The new syntax that is "a pathname that begins with a slash is not an absolute path but is relative to the runtime prefix" cannot avoid ambiguity with users'
existing settings on platforms other than Windows, which means this feature cannot be made platform neutral in its posted form. The documentation must say "On windows, the way to spell runtime prefix relative paths is this; on macs, you must spell it differently like this." etc. Which is rather unfortunate.
Perhaps we need to make an effort to find a syntax that is universally unambiguous and use it consistently across platforms?
I am tempted to say "//<token>/<the remainder>" might also be such a way, even in the POSIX world, but am not brave enough to do so, as I suspect that may have a fallout in the Windows world X-<.
An earlier suggestion by Duy in [*1*] to pretend as if we take $VARIABLE and define special variables might be a better direction.
Are there security implications if we started allowing references to environment varibables in strings we pass expand_user_path()? If we cannot convince ourselves that it is safe to allow access to any and all environment variables, then we probably can start by specific "pseudo variables" under our control, like GIT_EXEC_PATH and GIT_INSTALL_ROOT, that do not even have to be tied to environment variables, perhaps with further restriction to allow it only at the beginning of the string, or something like that, if necessary.