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
- signature.asc [application/pgp-signature] 263 bytes