git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Ramsay Jones <ramsay@ramsayjones.plus.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	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: Thu, 08 Nov 2018 09:16:39 +0900	[thread overview]
Message-ID: <xmqqr2fwa0ew.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <d7a70226-3441-76c4-df6a-e8fb32249f27@ramsayjones.plus.com> (Ramsay Jones's message of "Wed, 7 Nov 2018 17:42:00 +0000")

Ramsay Jones <ramsay@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-fjwnyjA0ntB2Dw@mail.gmail.com>

  reply	other threads:[~2018-11-08  0:16 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-06 14:53 [PATCH 0/1] mingw: handle absolute paths in expand_user_path() Johannes Schindelin via GitGitGadget
2018-11-06 14:53 ` [PATCH 1/1] " Johannes Schindelin via GitGitGadget
2018-11-06 15:54   ` Ramsay Jones
2018-11-06 16:10     ` Ramsay Jones
2018-11-06 18:27       ` Duy Nguyen
2018-11-07  1:21     ` Junio C Hamano
2018-11-07 11:19       ` Johannes Schindelin
2018-11-07 17:42         ` Ramsay Jones
2018-11-08  0:16           ` Junio C Hamano [this message]
2018-11-08 13:04             ` Johannes Schindelin
2018-11-08 14:43               ` Junio C Hamano
2018-11-08 15:39                 ` Johannes Schindelin
2018-11-09  2:05             ` Joseph Moisan
2018-11-09 10:21               ` Jeff King
2018-11-06 18:24   ` Duy Nguyen
2018-11-07 11:19     ` Johannes Schindelin
2018-11-06 21:32   ` Johannes Sixt
2018-11-07 11:23     ` Johannes Schindelin
2018-11-07 18:52       ` Johannes Sixt
2018-11-07 20:41         ` Jeff King
2018-11-07 21:36           ` Johannes Sixt
2018-11-07 22:03             ` Jeff King
2018-11-08  0:30               ` Junio C Hamano
2018-11-08  1:18                 ` Jeff King
2018-11-08  3:26                   ` Junio C Hamano
2018-11-08 13:11               ` Johannes Schindelin
2018-11-08 14:25                 ` Duy Nguyen
2018-11-08 15:45                   ` Johannes Schindelin
2018-11-08 17:40                     ` Eric Sunshine
2018-11-09 10:19                     ` Jeff King
2018-11-09 16:16                       ` Duy Nguyen
2018-11-12  3:03                         ` Junio C Hamano
2018-11-08 14:47                 ` Junio C Hamano
2018-11-08 15:46                   ` Johannes Schindelin
2021-07-01 16:03 ` [PATCH v2 0/2] " Johannes Schindelin via GitGitGadget
2021-07-01 16:03   ` [PATCH v2 1/2] tests: exercise the RUNTIME_PREFIX feature Johannes Schindelin via GitGitGadget
2021-07-01 16:03   ` [PATCH v2 2/2] expand_user_path(): support specifying paths relative to the runtime prefix Johannes Schindelin via GitGitGadget
2021-07-01 17:42   ` [PATCH v2 0/2] mingw: handle absolute paths in expand_user_path() Junio C Hamano
2021-07-24 22:06   ` [PATCH v3 0/5] " Johannes Schindelin via GitGitGadget
2021-07-24 22:06     ` [PATCH v3 1/5] tests: exercise the RUNTIME_PREFIX feature Johannes Schindelin via GitGitGadget
2021-07-24 22:06     ` [PATCH v3 2/5] expand_user_path(): remove stale part of the comment Johannes Schindelin via GitGitGadget
2021-07-24 22:06     ` [PATCH v3 3/5] expand_user_path(): clarify the role of the `real_home` parameter Johannes Schindelin via GitGitGadget
2021-07-24 22:06     ` [PATCH v3 4/5] Use a better name for the function interpolating paths Johannes Schindelin via GitGitGadget
2021-07-26 22:05       ` Junio C Hamano
2021-07-27  7:57         ` Fabian Stelzer
2021-07-27 22:56           ` Junio C Hamano
2021-07-28  0:14             ` Junio C Hamano
2021-07-28  0:39               ` Junio C Hamano
2021-07-28  5:43                 ` Junio C Hamano
2021-07-28  8:18                   ` Fabian Stelzer
2021-07-28 17:08                     ` Junio C Hamano
2021-07-24 22:06     ` [PATCH v3 5/5] interpolate_path(): allow specifying paths relative to the runtime prefix Johannes Schindelin via GitGitGadget
2021-07-26 17:56     ` [PATCH v3 0/5] mingw: handle absolute paths in expand_user_path() Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqr2fwa0ew.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=ramsay@ramsayjones.plus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).