git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
May 20, 2025, 14:35 UTC
Message-ID
<xmqqh61fl58v.fsf@gitster.g>
In-Reply-To
<20250520000125.2162144-2-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
Show 5 quoted lines
> 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.

Previous: Jacob KellerNext: Jacob Keller
Message 3 of 15 in “diff: add pathspec support to --no-index”
  1. 0/4 diff: add pathspec support to --no-indexJacob Keller, May 20, 2025
  2. 1/4 prefix_path: support prefixes not ending in trailing slashJacob Keller, May 20, 2025
  3. Junio C HamanoMay 20, 2025
  4. Jacob KellerMay 20, 2025
  5. 2/4 pathspec: expose match_pathspec_with_flagsJacob Keller, May 20, 2025
  6. Junio C HamanoMay 20, 2025
  7. Jacob KellerMay 20, 2025
  8. 3/4 pathspec: add flag to indicate operation without repositoryJacob Keller, May 20, 2025
  9. Junio C HamanoMay 20, 2025
  10. Jacob KellerMay 20, 2025
  11. Jacob KellerMay 21, 2025
  12. 4/4 diff --no-index: support limiting by pathspecJacob Keller, May 20, 2025
  13. Junio C HamanoMay 20, 2025
  14. Jacob KellerMay 20, 2025
  15. Jacob KellerMay 20, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.