Thread (10 messages) 10 messages, 3 authors, 2020-11-16

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

From: brian m. carlson <hidden>
Date: 2020-11-16 02:22:20

On 2020-11-09 at 13:55:53, Johannes Schindelin wrote:
Hi brian,

On Fri, 9 Oct 2020, brian m. carlson wrote:
quoted
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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help