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

Re: [PATCH v2 1/2] abspath: add a function to resolve paths with missing components

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Nov 16, 2020, 02:21 UTC
Message-ID
<20201116022143.GB389879@camp.crustytoothpaste.net>
In-Reply-To
<nycvar.QRO.7.76.6.2011091442190.18437@tvgsbejvaqbjf.bet>
On 2020-11-09 at 13:55:53, Johannes Schindelin wrote:
Show 62 quoted lines
> Hi brian,
> 
> On Fri, 9 Oct 2020, brian m. carlson wrote:
> 
> > We'd like to canonicalize paths such that we can preserve any number of
> > trailing components that may be missing.  Let's add a function to do
> > that that calls strbuf_realpath to find the canonical path for the
> > portion we do have and then append the missing part.  We adjust
> > strip_last_component to return us the component it has stripped and use
> > that to help us accumulate the missing part.
> >
> > Note that it is intentional that we invoke strbuf_realpath here,
> > repeatedly if necessary, because on Windows that function is replaced
> > with a version that uses the proper system semantics for
> > canonicalization.  Trying to adjust strbuf_realpath to perform this kind
> > of canonicalization with an additional option would fail to work
> > properly on Windows.  The present approach is equivalent to
> > strbuf_realpath for cases where the path exists, and the only other
> > cases where we will use this function the additional overhead of
> > multiple invocations is not significant.
> 
> Thank you for being so considerate. Yes, on Windows, we use (wherever
> possible) a shortcut that tells us the canonicalized path of existing
> entries.
> 
> Technically, it is not `strbuf_realpath()` that we override, but we take a
> shortcut _in_ that function. That's semantics, though.
> 
> More importantly, we recently fixed a bug in our code to allow for a quirk
> in the `strbuf_realpath()` function: `strbuf_realpath()` allows the last
> path component to not exist. If that is the case, now it's time to try
> without last component.
> 
> In a sense, this is a 1-level version of your infinite-level
> `strbuf_realpath_missing()` function.
> 
> An idea that immediately crosses my mind is whether that level could be
> something we want to pass directly into `strbuf_realpath()` as a parameter
> (it would be 1 to imitate the current behavior and -1 for the
> infinite-level case). What do you think? Does that make sense?
> 
> In any case, I think this `_missing()` functionality should be implemented
> a bit more tightly with the `strbuf_realpath()` function because of the
> logic that already allows the last component to be missing:
> 
>                 if (lstat(resolved->buf, &st)) {
>                         /* error out unless this was the last component */
>                         if (errno != ENOENT || remaining.len) {
>                                 if (die_on_error)
>                                         die_errno("Invalid path '%s'",
>                                                   resolved->buf);
>                                 else
>                                         goto error_out;
>                         }
> 
> See https://github.com/git/git/blob/v2.29.2/abspath.c#L130-L138 for the
> exact code and context.
> 
> Seeing as we _already_ have some code to allow for _some_ missing
> component, it should be possible to extend the logic to allow for
> different levels (e.g. using `count_slashes()` if we want to allow more
> than just the last component to be missing).

Okay, if you'd prefer to do it that way, that's fine with me. I'll reroll with that change.

-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US
Previous: Johannes SchindelinNext: Emily Shaffer
Message 10 of 13 in “rev-parse options for absolute or relative paths”
  1. 0/2 rev-parse options for absolute or relative pathsbrian m. carlson, Oct 9, 2020
  2. 2/2 rev-parse: add option for absolute or relative path formattingbrian m. carlson, Oct 9, 2020
  3. Johannes SchindelinNov 9, 2020
  4. brian m. carlsonNov 16, 2020
  5. 1/2 abspath: add a function to resolve paths with missing componentsbrian m. carlson, Oct 9, 2020
  6. Junio C HamanoOct 9, 2020
  7. brian m. carlsonOct 10, 2020
  8. Johannes SchindelinNov 9, 2020
  9. Johannes SchindelinNov 9, 2020
  10. brian m. carlsonNov 16, 2020
  11. Emily ShafferNov 4, 2020
  12. brian m. carlsonNov 5, 2020
  13. Johannes SchindelinNov 9, 2020

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.