Ramsay Jones <ram...@ramsayjones.plus.com> 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.

[References]

*1* <cacsjy8dmyu_kn4yytu_pqdpppxo8wlcuuzq-fjwnyja0ntb...@mail.gmail.com>

Reply via email to