Re: [PATCH v3 1/4] prefix_path: support prefixes not ending in trailing slash

[Junio C Hamano <gitster@pobox.com>]

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> The prefix_path_gently() -- and its wrapper prefix_path() -- function
> normalizes the provided path and optionally adds the provided prefix to
> relative paths.

I find "optionally" confusing here.  The original intended use case
of this function being that "the provided path", which comes from
the end user from the command line that names a file relative to the
directory the usre started the "git" command in, needs to be
adjusted after "git" chdir's up to the top-level of the working
tree, and the way to do so is to prepend the "prefix" computed by
"git" (which always ends with a slash).  Adding the prefix to
relative paths is the central part of what it has to do.  When the
end-user supplied path goes up e.g., "../file", we may have to lose
a level or more of the prefix before prepending, but that does not
change the fact that the helper function is about prepending the
prefix.

And as you can guess from the above description, if the caller
passes prefix that does not end in a trailing slash, the caller is
buggy.

> If the prefix does not end in a trailing slash, the
> function will combine the last component of the prefix with the first
> component of the relative path. This is unlikely to produce a desirable
> result.

True, and I do not mind being lenient to buggy callers, but given
that the majority of callers (i.e. all the existing callers) not
being buggy, I wonder if it is better to check and append a slash to
the end by a new caller that feeds a prefix that directly comes from
the end user?

Unlike prefix that is internally generated by Git, we'd need to be a
lot more careful if we are taking end-user input directly and
passing it as a "prefix" (I take that the possible lack of trailing
slash as a signal that you are doing something like that in this
series).  We may need to check and correct that they do not contain
"./", "//", or "../", for example, anyway, and adding the required
trailing slash at the end sounds like something we may want to do as
part of that input validation and massaging _before_ calling these
helper functions.