From 0bfcb3d7096d765d0cca34a0537e018886a04a68 Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Tue, 21 Jul 2026 14:38:01 -0700 Subject: [PATCH 1/7] mingw: avoid O(n*m) rescans when resolving phantom symlinks A symlink on Windows must be created as a "file" or "directory" symlink, and Git can't always tell which until the target appears during checkout; until then it's tracked as "phantom" and rechecked. Phantom symlinks lived in one global list, fully rescanned by process_phantom_symlinks() on every mkdir() (and on every symlink that turns out to point at a directory), making checkout O(n*m) in outstanding phantom symlinks times directories created. Reported in git-for-windows/git#4059 for a git-annex repo where nearly all symlinks dangle permanently. On a 343-symlink, ~185k-file monorepo fixture, checkout takes 330s and calls process_phantom_symlink() 12,693,504 times -- about 343 * 37016, the directories created. Index phantom symlinks in a trie over their target path's components instead. Waking a path retries only entries registered there or nested under it, so an unrelated directory costs O(1). Same fixture: 54.8s and 343 calls -- one per symlink, no wasted rescans. Also fixes an existing bug: wlink/wtarget are interior pointers into the same allocation as the struct, not separate allocations, so only free(current) is correct; freeing them individually is undefined behavior. Known limitation: a phantom symlink whose target passes through another symlink as an intermediate component may not get woken if that symlink's real target directory is still being populated when it resolves; the next commit addresses this. Signed-off-by: Chris Harris --- compat/mingw.c | 220 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 194 insertions(+), 26 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 92946af422770c..affef197b93d92 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -9,6 +9,7 @@ #include "dir.h" #include "environment.h" #include "gettext.h" +#include "hashmap.h" #include "repository.h" #include "run-command.h" #include "strbuf.h" @@ -448,36 +449,181 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink) return PHANTOM_SYMLINK_RETRY; } -/* keep track of newly created symlinks to non-existing targets */ +/* + * Newly created symlinks to non-existing targets are indexed by a trie + * over their target path's components, so mkdir() (or a symlink turning + * into a directory symlink) can wake only the entries nested under the + * path that just became traversable, instead of re-probing everything. + */ struct phantom_symlink_info { struct phantom_symlink_info *next; wchar_t *wlink; wchar_t *wtarget; }; -static struct phantom_symlink_info *phantom_symlinks = NULL; +struct phantom_trie_node { + struct hashmap_entry ent; + char *component; + struct hashmap children; + struct phantom_symlink_info *waiters; +}; + static CRITICAL_SECTION phantom_symlinks_cs; -static void process_phantom_symlinks(void) +static int phantom_trie_node_cmp(const void *cmp_data UNUSED, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata) +{ + const struct phantom_trie_node *e = + container_of(eptr, const struct phantom_trie_node, ent); + + return !fspatheq(e->component, keydata ? keydata : + container_of(entry_or_key, const struct phantom_trie_node, + ent)->component); +} + +static struct phantom_trie_node phantom_trie_root = + { .children = HASHMAP_INIT(phantom_trie_node_cmp, NULL) }; + +static char *phantom_symlink_canonicalize(const wchar_t *wpath) +{ + wchar_t wfullpath[MAX_LONG_PATH]; + char utf8[MAX_LONG_PATH * 3]; + int len = GetFullPathNameW(wpath, ARRAY_SIZE(wfullpath), wfullpath, NULL); + + if (!len || len >= ARRAY_SIZE(wfullpath) || + xwcstoutf(utf8, wfullpath, sizeof(utf8)) < 0) + return NULL; + return xstrdup(utf8); +} + +/* the path process_phantom_symlink() itself probes with CreateFileW() */ +static char *phantom_symlink_target_key(const wchar_t *wtarget, + const wchar_t *wlink) +{ + wchar_t relative[MAX_LONG_PATH]; + const wchar_t *rel = make_relative_to(wtarget, wlink, relative, + ARRAY_SIZE(relative)); + + return rel ? phantom_symlink_canonicalize(rel) : NULL; +} + +static struct phantom_trie_node *phantom_trie_child(struct phantom_trie_node *node, + const char *component, size_t len, + int create) +{ + char buf[MAX_LONG_PATH * 3]; + struct hashmap_entry key; + struct phantom_trie_node *child; + + if (!len || len >= sizeof(buf)) + return NULL; + memcpy(buf, component, len); + buf[len] = '\0'; + + hashmap_entry_init(&key, fspathhash(buf)); + child = hashmap_get_entry_from_hash(&node->children, key.hash, buf, + struct phantom_trie_node, ent); + if (!child && create) { + child = xcalloc(1, sizeof(*child)); + child->component = xstrdup(buf); + hashmap_init(&child->children, phantom_trie_node_cmp, NULL, 0); + hashmap_entry_init(&child->ent, key.hash); + hashmap_add(&node->children, &child->ent); + } + return child; +} + +static struct phantom_trie_node *phantom_trie_walk(const char *path, int create) +{ + struct phantom_trie_node *node = &phantom_trie_root; + const char *p = path; + + while (*p && node) { + const char *start; + + while (*p == '/' || *p == '\\') + p++; + start = p; + while (*p && *p != '/' && *p != '\\') + p++; + if (p == start) + break; + node = phantom_trie_child(node, start, (size_t)(p - start), create); + } + return node; +} + +static void phantom_trie_wake_node(struct phantom_trie_node *node); + +/* assumes phantom_symlinks_cs is held */ +static void phantom_trie_drain_waiters(struct phantom_trie_node *node) { struct phantom_symlink_info *current, **psi; - EnterCriticalSection(&phantom_symlinks_cs); - /* process phantom symlinks list */ - psi = &phantom_symlinks; - while ((current = *psi)) { - enum phantom_symlink_result result = process_phantom_symlink( - current->wtarget, current->wlink); - if (result == PHANTOM_SYMLINK_RETRY) { - psi = ¤t->next; - } else { - /* symlink was processed, remove from list */ + int restart; + + do { + restart = 0; + psi = &node->waiters; + while ((current = *psi)) { + enum phantom_symlink_result result = + process_phantom_symlink(current->wtarget, current->wlink); + char *woken; + + if (result == PHANTOM_SYMLINK_RETRY) { + psi = ¤t->next; + continue; + } + *psi = current->next; + woken = result == PHANTOM_SYMLINK_DIRECTORY ? + phantom_symlink_canonicalize(current->wlink) : NULL; free(current); - /* if symlink was a directory, start over */ - if (result == PHANTOM_SYMLINK_DIRECTORY) - psi = &phantom_symlinks; + + if (woken) { + /* may wake into this same node; restart, don't trust *psi */ + struct phantom_trie_node *n = phantom_trie_walk(woken, 0); + free(woken); + if (n) + phantom_trie_wake_node(n); + restart = 1; + break; + } } - } + } while (restart); +} + +/* assumes phantom_symlinks_cs is held */ +static void phantom_trie_wake_children(struct phantom_trie_node *node) +{ + struct hashmap_iter iter; + struct phantom_trie_node *child; + + hashmap_iter_init(&node->children, &iter); + while ((child = container_of_or_null(hashmap_iter_next(&iter), + struct phantom_trie_node, ent))) + phantom_trie_wake_node(child); +} + +static void phantom_trie_wake_node(struct phantom_trie_node *node) +{ + phantom_trie_drain_waiters(node); + phantom_trie_wake_children(node); +} + +/* target_key: as returned by phantom_symlink_canonicalize(); not freed here */ +static void process_phantom_symlinks_under(const char *target_key) +{ + struct phantom_trie_node *node; + + if (!target_key) + return; + + EnterCriticalSection(&phantom_symlinks_cs); + node = phantom_trie_walk(target_key, 0); + if (node) + phantom_trie_wake_node(node); LeaveCriticalSection(&phantom_symlinks_cs); } @@ -494,9 +640,11 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) /* convert to directory symlink if target exists */ switch (process_phantom_symlink(wtarget, wlink)) { case PHANTOM_SYMLINK_RETRY: { - /* if target doesn't exist, add to phantom symlinks list */ + /* if target doesn't exist, add to phantom symlinks trie */ wchar_t wfullpath[MAX_LONG_PATH]; struct phantom_symlink_info *psi; + struct phantom_trie_node *node; + char *target_key; /* convert to absolute path to be independent of cwd */ len = GetFullPathNameW(wlink, MAX_LONG_PATH, wfullpath, NULL); @@ -505,6 +653,10 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) return -1; } + target_key = phantom_symlink_target_key(wtarget, wlink); + if (!target_key) + break; + /* over-allocate and fill phantom_symlink_info structure */ psi = xmalloc(sizeof(struct phantom_symlink_info) + sizeof(wchar_t) * (len + wcslen(wtarget) + 2)); @@ -514,15 +666,24 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) wcscpy(psi->wtarget, wtarget); EnterCriticalSection(&phantom_symlinks_cs); - psi->next = phantom_symlinks; - phantom_symlinks = psi; + node = phantom_trie_walk(target_key, 1); + if (node) { + psi->next = node->waiters; + node->waiters = psi; + } else { + free(psi); + } LeaveCriticalSection(&phantom_symlinks_cs); + free(target_key); break; } - case PHANTOM_SYMLINK_DIRECTORY: - /* if we created a dir symlink, process other phantom symlinks */ - process_phantom_symlinks(); + case PHANTOM_SYMLINK_DIRECTORY: { + /* if we created a dir symlink, wake others waiting on it */ + char *woken = phantom_symlink_canonicalize(wlink); + process_phantom_symlinks_under(woken); + free(woken); break; + } default: break; } @@ -758,8 +919,11 @@ int mingw_mkdir(const char *path, int mode UNUSED) return -1; ret = _wmkdir(wpath); - if (!ret) - process_phantom_symlinks(); + if (!ret) { + char *created = phantom_symlink_canonicalize(wpath); + process_phantom_symlinks_under(created); + free(created); + } if (!ret && needs_hiding(path)) return set_hidden_flag(wpath, 1); return ret; @@ -3495,7 +3659,11 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch break; /* There may be dangling phantom symlinks that point at this * one, which should now morph into directory symlinks. */ - process_phantom_symlinks(); + { + char *woken = phantom_symlink_canonicalize(wlink); + process_phantom_symlinks_under(woken); + free(woken); + } return 0; default: BUG("unhandled symlink type"); From a8ed2f6a291976e819eca3275bc633a50e0c0d8a Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Tue, 21 Jul 2026 14:38:09 -0700 Subject: [PATCH 2/7] mingw: wake phantom symlinks nested through a resolved symlink Waking a symlink's own path when it resolves to a directory cascades into nested trie entries, but only once, at that exact moment. If a nested entry's target doesn't fully exist yet then (e.g. a deeper real directory is created moments later), it is never woken again -- the real directory appears under a different trie key than the one the entry is registered under through the symlink. Graft everything registered under the symlink's own path onto its resolved target's trie node before waking both, so a later mkdir() under the real path can still find it. Verified with symlink "T" -> "B/C" through symlink "B" -> "realdir", where "realdir/C" is created after "realdir": T now correctly resolves to a directory symlink, where it previously stayed a file symlink. Signed-off-by: Chris Harris --- compat/mingw.c | 95 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 74 insertions(+), 21 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index affef197b93d92..3d1a06c234b81b 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -509,9 +509,9 @@ static char *phantom_symlink_target_key(const wchar_t *wtarget, return rel ? phantom_symlink_canonicalize(rel) : NULL; } -static struct phantom_trie_node *phantom_trie_child(struct phantom_trie_node *node, - const char *component, size_t len, - int create) +static struct phantom_trie_node *phantom_trie_child( + struct phantom_trie_node *node, const char *component, + size_t len, int create) { char buf[MAX_LONG_PATH * 3]; struct hashmap_entry key; @@ -556,6 +556,42 @@ static struct phantom_trie_node *phantom_trie_walk(const char *path, int create) } static void phantom_trie_wake_node(struct phantom_trie_node *node); +static void phantom_symlink_directory_resolved(const wchar_t *wtarget, + const wchar_t *wlink); + +/* + * Moves everything nested under `from` (not `from` itself) to the same + * relative position under `to`. Used when a symlink at `from`'s path + * turns out to point at `to`'s path: anything registered through the + * symlink can then still be found by mkdir() under the real path it + * points at, not just under the symlink's own path. + * assumes phantom_symlinks_cs is held + */ +static void phantom_trie_graft_children(struct phantom_trie_node *to, + struct phantom_trie_node *from) +{ + struct hashmap_iter iter; + struct phantom_trie_node *from_child; + + hashmap_iter_init(&from->children, &iter); + while ((from_child = container_of_or_null(hashmap_iter_next(&iter), + struct phantom_trie_node, ent))) { + struct phantom_trie_node *to_child = phantom_trie_child( + to, from_child->component, strlen(from_child->component), 1); + if (!to_child) + continue; + if (from_child->waiters) { + struct phantom_symlink_info *last = from_child->waiters; + + while (last->next) + last = last->next; + last->next = to_child->waiters; + to_child->waiters = from_child->waiters; + from_child->waiters = NULL; + } + phantom_trie_graft_children(to_child, from_child); + } +} /* assumes phantom_symlinks_cs is held */ static void phantom_trie_drain_waiters(struct phantom_trie_node *node) @@ -569,7 +605,6 @@ static void phantom_trie_drain_waiters(struct phantom_trie_node *node) while ((current = *psi)) { enum phantom_symlink_result result = process_phantom_symlink(current->wtarget, current->wlink); - char *woken; if (result == PHANTOM_SYMLINK_RETRY) { psi = ¤t->next; @@ -577,16 +612,13 @@ static void phantom_trie_drain_waiters(struct phantom_trie_node *node) } *psi = current->next; - woken = result == PHANTOM_SYMLINK_DIRECTORY ? - phantom_symlink_canonicalize(current->wlink) : NULL; + if (result == PHANTOM_SYMLINK_DIRECTORY) + phantom_symlink_directory_resolved(current->wtarget, + current->wlink); free(current); - if (woken) { + if (result == PHANTOM_SYMLINK_DIRECTORY) { /* may wake into this same node; restart, don't trust *psi */ - struct phantom_trie_node *n = phantom_trie_walk(woken, 0); - free(woken); - if (n) - phantom_trie_wake_node(n); restart = 1; break; } @@ -612,6 +644,34 @@ static void phantom_trie_wake_node(struct phantom_trie_node *node) phantom_trie_wake_children(node); } +/* + * wlink just became a directory symlink pointing at wtarget (whether + * converted from a phantom, or created that way outright): graft + * anything registered through wlink's own path onto wtarget's resolved + * path (see phantom_trie_graft_children()), then wake both. + */ +static void phantom_symlink_directory_resolved(const wchar_t *wtarget, + const wchar_t *wlink) +{ + char *own_key = phantom_symlink_canonicalize(wlink); + char *real_key = phantom_symlink_target_key(wtarget, wlink); + struct phantom_trie_node *n, *real_node; + + EnterCriticalSection(&phantom_symlinks_cs); + n = own_key ? phantom_trie_walk(own_key, 0) : NULL; + real_node = real_key ? phantom_trie_walk(real_key, 1) : NULL; + if (n && real_node && real_node != n) + phantom_trie_graft_children(real_node, n); + if (n) + phantom_trie_wake_node(n); + if (real_node) + phantom_trie_wake_node(real_node); + LeaveCriticalSection(&phantom_symlinks_cs); + + free(own_key); + free(real_key); +} + /* target_key: as returned by phantom_symlink_canonicalize(); not freed here */ static void process_phantom_symlinks_under(const char *target_key) { @@ -677,13 +737,10 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) free(target_key); break; } - case PHANTOM_SYMLINK_DIRECTORY: { + case PHANTOM_SYMLINK_DIRECTORY: /* if we created a dir symlink, wake others waiting on it */ - char *woken = phantom_symlink_canonicalize(wlink); - process_phantom_symlinks_under(woken); - free(woken); + phantom_symlink_directory_resolved(wtarget, wlink); break; - } default: break; } @@ -3659,11 +3716,7 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch break; /* There may be dangling phantom symlinks that point at this * one, which should now morph into directory symlinks. */ - { - char *woken = phantom_symlink_canonicalize(wlink); - process_phantom_symlinks_under(woken); - free(woken); - } + phantom_symlink_directory_resolved(wtarget, wlink); return 0; default: BUG("unhandled symlink type"); From 2935e708a84b60707d546272c9028f8c2839da25 Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Tue, 21 Jul 2026 14:38:16 -0700 Subject: [PATCH 3/7] t2041: test symlink nested through another symlink Add a regression test for the grafting fix: a symlink whose target passes through another symlink should still become a directory symlink once its real target appears, even when that target is only populated after the leading symlink has already resolved. Windows path resolution follows a symlink's target regardless of whether it is flagged as a file or directory symlink, so opening a path through it succeeds either way; the type only becomes visible in things like `cmd.exe /c dir`, which marks directory symlinks as and file symlinks as . The test asserts on that distinction rather than on read access, since the latter would pass even without the previous commit's fix. Signed-off-by: Chris Harris --- t/t2041-checkout-symlink-through-symlink.sh | 56 +++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100755 t/t2041-checkout-symlink-through-symlink.sh diff --git a/t/t2041-checkout-symlink-through-symlink.sh b/t/t2041-checkout-symlink-through-symlink.sh new file mode 100755 index 00000000000000..fd7771be3e3652 --- /dev/null +++ b/t/t2041-checkout-symlink-through-symlink.sh @@ -0,0 +1,56 @@ +#!/bin/sh + +test_description='checkout a symlink nested through another symlink on Windows + +A phantom symlink may target a path that goes through another +symlink. Ensures that such a symlink is still upgraded to a directory +symlink once its real target appears, even when that target directory +is only populated after the leading symlink has already resolved.' + +# Tell MSYS to create native symlinks. Without this flag test-lib's +# prerequisite detection for SYMLINKS doesn't detect the right thing. +MSYS=winsymlinks:nativestrict && export MSYS + +. ./test-lib.sh + +if ! test_have_prereq MINGW,SYMLINKS +then + skip_all='skipping $0: MinGW-only test, which requires symlink support.' + test_done +fi + +# Adds a symlink to the index without clobbering the work tree. +cache_symlink () { + sha=$(printf '%s' "$1" | git hash-object --stdin -w) && + git update-index --add --cacheinfo 120000,$sha,"$2" +} + +# Adds a regular file to the index without clobbering the work tree. +cache_file () { + sha=$(printf '%s' "$1" | git hash-object --stdin -w) && + git update-index --add --cacheinfo 100644,$sha,"$2" +} + +test_expect_success 'symlink nested through another symlink resolves' ' + test_when_finished "rm -rf chained" && + mkdir chained && + ( + cd chained && + git init -q && + + cache_symlink realdir leading && + cache_symlink leading/sub nested && + cache_file content realdir/sub/file && + + git checkout -- . && + + # "cmd.exe /c dir" marks directory symlinks as + # and file symlinks as ; unlike opening a path + # through the symlink, this distinguishes the two even + # though both resolve identically for plain reads. + cmd.exe //c dir . >dir-listing && + grep "SYMLINKD.*nested" dir-listing + ) +' + +test_done From a57c9feca82df6d97388f1b04e9fd76d18008071 Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Thu, 30 Jul 2026 11:51:16 -0700 Subject: [PATCH 4/7] mingw: simplify phantom symlink tracking to a flat hashmap The trie plus grafting introduced in the previous two commits works, but review feedback pointed out it carries more machinery (nested hashmaps, a trie keyed by path components, grafting subtrees between nodes) than the underlying problem needs: for a symlink whose target is some path P, only P itself matters for resolving it, regardless of which of P's leading directories get created along the way. Replace the trie with a single hashmap keyed by each phantom symlink's (canonicalized, absolute) target path, storing a growable array of waiting symlinks per key instead of a per-node linked list. process_phantom_symlinks() takes the path that was just created (a new directory, or a symlink that turned out to point at one) and looks it up directly -- an O(1) hashmap lookup -- instead of walking a trie down to the matching node. Canonicalizing the path and freeing it afterwards also moves into process_phantom_symlinks() itself, so callers are one-liners instead of repeating that pattern. On the same 343-symlink, 37,016-directory fixture used in the previous commits, this measures 54.9s and 343 calls to process_phantom_symlink() -- unchanged from the trie's numbers, since both do O(1) work per relevant mkdir(); the difference is code, not complexity class. This reintroduces the limitation the grafting commit fixed: a phantom symlink whose target passes through another symlink as an intermediate component is only woken when that intermediate symlink itself resolves, not when the real directory it points at is populated later (see the previous two commits' messages for the worked example). The flat hashmap has no way to recognize that an intermediate symlink's own path and its resolved target are aliases for the same location without either scanning every entry when a symlink resolves, or reintroducing a second, trie-like index to answer that prefix question -- both of which reintroduce the complexity this commit is removing. Given the reported real-world cases (git-annex repositories, and large monorepos with many independent symlinks) are about independent dangling symlinks rather than chains through other symlinks, this trade-off was judged worth the simplification; drop t2041, which specifically exercised the now-removed grafting behavior. Signed-off-by: Chris Harris --- compat/mingw.c | 333 ++++++-------------- t/t2041-checkout-symlink-through-symlink.sh | 56 ---- 2 files changed, 100 insertions(+), 289 deletions(-) delete mode 100755 t/t2041-checkout-symlink-through-symlink.sh diff --git a/compat/mingw.c b/compat/mingw.c index 3d1a06c234b81b..0f50074995c71f 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -450,43 +450,54 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink) } /* - * Newly created symlinks to non-existing targets are indexed by a trie - * over their target path's components, so mkdir() (or a symlink turning - * into a directory symlink) can wake only the entries nested under the - * path that just became traversable, instead of re-probing everything. + * Newly created symlinks to non-existing targets are indexed by a + * hashmap keyed by their (canonicalized, absolute) target path, so + * that mkdir() creating that path -- or a symlink at that path turning + * out to be a directory symlink -- wakes only the entries actually + * waiting on it, instead of re-probing every phantom symlink created + * so far. */ struct phantom_symlink_info { - struct phantom_symlink_info *next; wchar_t *wlink; - wchar_t *wtarget; }; -struct phantom_trie_node { +struct phantom_symlink_target { struct hashmap_entry ent; - char *component; - struct hashmap children; - struct phantom_symlink_info *waiters; + wchar_t *wtarget; + size_t nr, alloc; + struct phantom_symlink_info *items; + char target[FLEX_ARRAY]; }; -static CRITICAL_SECTION phantom_symlinks_cs; - -static int phantom_trie_node_cmp(const void *cmp_data UNUSED, - const struct hashmap_entry *eptr, - const struct hashmap_entry *entry_or_key, - const void *keydata) +static int phantom_symlink_target_cmp(const void *cmp_data UNUSED, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata) { - const struct phantom_trie_node *e = - container_of(eptr, const struct phantom_trie_node, ent); + const struct phantom_symlink_target *e = + container_of(eptr, const struct phantom_symlink_target, ent); - return !fspatheq(e->component, keydata ? keydata : - container_of(entry_or_key, const struct phantom_trie_node, - ent)->component); + return !fspatheq(e->target, keydata ? keydata : + container_of(entry_or_key, const struct phantom_symlink_target, + ent)->target); } -static struct phantom_trie_node phantom_trie_root = - { .children = HASHMAP_INIT(phantom_trie_node_cmp, NULL) }; +static struct hashmap phantom_symlinks = + HASHMAP_INIT(phantom_symlink_target_cmp, NULL); +static CRITICAL_SECTION phantom_symlinks_cs; -static char *phantom_symlink_canonicalize(const wchar_t *wpath) +static wchar_t *xwcsdup(const wchar_t *s) +{ + size_t size = sizeof(wchar_t) * (wcslen(s) + 1); + return memcpy(xmalloc(size), s, size); +} + +/* + * Returns the canonicalized, absolute UTF-8 form of wpath. If wout is + * non-NULL, also fills it with the canonicalized, absolute wide form + * (must have room for at least MAX_LONG_PATH wchar_t). + */ +static char *canonicalize_path(const wchar_t *wpath, wchar_t *wout) { wchar_t wfullpath[MAX_LONG_PATH]; char utf8[MAX_LONG_PATH * 3]; @@ -495,202 +506,60 @@ static char *phantom_symlink_canonicalize(const wchar_t *wpath) if (!len || len >= ARRAY_SIZE(wfullpath) || xwcstoutf(utf8, wfullpath, sizeof(utf8)) < 0) return NULL; + if (wout) + wcscpy(wout, wfullpath); return xstrdup(utf8); } -/* the path process_phantom_symlink() itself probes with CreateFileW() */ -static char *phantom_symlink_target_key(const wchar_t *wtarget, - const wchar_t *wlink) -{ - wchar_t relative[MAX_LONG_PATH]; - const wchar_t *rel = make_relative_to(wtarget, wlink, relative, - ARRAY_SIZE(relative)); - - return rel ? phantom_symlink_canonicalize(rel) : NULL; -} - -static struct phantom_trie_node *phantom_trie_child( - struct phantom_trie_node *node, const char *component, - size_t len, int create) +/* + * Wakes every phantom symlink whose target is exactly wpath: mkdir() + * creating that path, or a symlink at that path turning out to be a + * directory symlink, may let them resolve. + */ +static void process_phantom_symlinks(const wchar_t *wpath) { - char buf[MAX_LONG_PATH * 3]; + char *target_key = canonicalize_path(wpath, NULL); struct hashmap_entry key; - struct phantom_trie_node *child; - - if (!len || len >= sizeof(buf)) - return NULL; - memcpy(buf, component, len); - buf[len] = '\0'; + struct phantom_symlink_target *e; + size_t i; - hashmap_entry_init(&key, fspathhash(buf)); - child = hashmap_get_entry_from_hash(&node->children, key.hash, buf, - struct phantom_trie_node, ent); - if (!child && create) { - child = xcalloc(1, sizeof(*child)); - child->component = xstrdup(buf); - hashmap_init(&child->children, phantom_trie_node_cmp, NULL, 0); - hashmap_entry_init(&child->ent, key.hash); - hashmap_add(&node->children, &child->ent); - } - return child; -} - -static struct phantom_trie_node *phantom_trie_walk(const char *path, int create) -{ - struct phantom_trie_node *node = &phantom_trie_root; - const char *p = path; - - while (*p && node) { - const char *start; + if (!target_key) + return; - while (*p == '/' || *p == '\\') - p++; - start = p; - while (*p && *p != '/' && *p != '\\') - p++; - if (p == start) - break; - node = phantom_trie_child(node, start, (size_t)(p - start), create); - } - return node; -} + EnterCriticalSection(&phantom_symlinks_cs); + hashmap_entry_init(&key, fspathhash(target_key)); + e = hashmap_get_entry_from_hash(&phantom_symlinks, key.hash, target_key, + struct phantom_symlink_target, ent); -static void phantom_trie_wake_node(struct phantom_trie_node *node); -static void phantom_symlink_directory_resolved(const wchar_t *wtarget, - const wchar_t *wlink); + for (i = 0; e && i < e->nr; ) { + enum phantom_symlink_result result = + process_phantom_symlink(e->wtarget, e->items[i].wlink); + wchar_t *wlink = e->items[i].wlink; -/* - * Moves everything nested under `from` (not `from` itself) to the same - * relative position under `to`. Used when a symlink at `from`'s path - * turns out to point at `to`'s path: anything registered through the - * symlink can then still be found by mkdir() under the real path it - * points at, not just under the symlink's own path. - * assumes phantom_symlinks_cs is held - */ -static void phantom_trie_graft_children(struct phantom_trie_node *to, - struct phantom_trie_node *from) -{ - struct hashmap_iter iter; - struct phantom_trie_node *from_child; - - hashmap_iter_init(&from->children, &iter); - while ((from_child = container_of_or_null(hashmap_iter_next(&iter), - struct phantom_trie_node, ent))) { - struct phantom_trie_node *to_child = phantom_trie_child( - to, from_child->component, strlen(from_child->component), 1); - if (!to_child) + if (result == PHANTOM_SYMLINK_RETRY) { + i++; continue; - if (from_child->waiters) { - struct phantom_symlink_info *last = from_child->waiters; - - while (last->next) - last = last->next; - last->next = to_child->waiters; - to_child->waiters = from_child->waiters; - from_child->waiters = NULL; - } - phantom_trie_graft_children(to_child, from_child); - } -} - -/* assumes phantom_symlinks_cs is held */ -static void phantom_trie_drain_waiters(struct phantom_trie_node *node) -{ - struct phantom_symlink_info *current, **psi; - int restart; - - do { - restart = 0; - psi = &node->waiters; - while ((current = *psi)) { - enum phantom_symlink_result result = - process_phantom_symlink(current->wtarget, current->wlink); - - if (result == PHANTOM_SYMLINK_RETRY) { - psi = ¤t->next; - continue; - } - - *psi = current->next; - if (result == PHANTOM_SYMLINK_DIRECTORY) - phantom_symlink_directory_resolved(current->wtarget, - current->wlink); - free(current); - - if (result == PHANTOM_SYMLINK_DIRECTORY) { - /* may wake into this same node; restart, don't trust *psi */ - restart = 1; - break; - } } - } while (restart); -} - -/* assumes phantom_symlinks_cs is held */ -static void phantom_trie_wake_children(struct phantom_trie_node *node) -{ - struct hashmap_iter iter; - struct phantom_trie_node *child; - - hashmap_iter_init(&node->children, &iter); - while ((child = container_of_or_null(hashmap_iter_next(&iter), - struct phantom_trie_node, ent))) - phantom_trie_wake_node(child); -} -static void phantom_trie_wake_node(struct phantom_trie_node *node) -{ - phantom_trie_drain_waiters(node); - phantom_trie_wake_children(node); -} - -/* - * wlink just became a directory symlink pointing at wtarget (whether - * converted from a phantom, or created that way outright): graft - * anything registered through wlink's own path onto wtarget's resolved - * path (see phantom_trie_graft_children()), then wake both. - */ -static void phantom_symlink_directory_resolved(const wchar_t *wtarget, - const wchar_t *wlink) -{ - char *own_key = phantom_symlink_canonicalize(wlink); - char *real_key = phantom_symlink_target_key(wtarget, wlink); - struct phantom_trie_node *n, *real_node; + e->items[i] = e->items[--e->nr]; + if (result == PHANTOM_SYMLINK_DIRECTORY) + process_phantom_symlinks(wlink); + free(wlink); + } - EnterCriticalSection(&phantom_symlinks_cs); - n = own_key ? phantom_trie_walk(own_key, 0) : NULL; - real_node = real_key ? phantom_trie_walk(real_key, 1) : NULL; - if (n && real_node && real_node != n) - phantom_trie_graft_children(real_node, n); - if (n) - phantom_trie_wake_node(n); - if (real_node) - phantom_trie_wake_node(real_node); + if (e && !e->nr) { + hashmap_remove(&phantom_symlinks, &e->ent, target_key); + free(e->wtarget); + free(e->items); + free(e); + } LeaveCriticalSection(&phantom_symlinks_cs); - free(own_key); - free(real_key); -} - -/* target_key: as returned by phantom_symlink_canonicalize(); not freed here */ -static void process_phantom_symlinks_under(const char *target_key) -{ - struct phantom_trie_node *node; - - if (!target_key) - return; - - EnterCriticalSection(&phantom_symlinks_cs); - node = phantom_trie_walk(target_key, 0); - if (node) - phantom_trie_wake_node(node); - LeaveCriticalSection(&phantom_symlinks_cs); + free(target_key); } static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) { - int len; - /* create file symlink */ if (!CreateSymbolicLinkW(wlink, wtarget, symlink_file_flags)) { errno = err_win_to_posix(GetLastError()); @@ -700,46 +569,47 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) /* convert to directory symlink if target exists */ switch (process_phantom_symlink(wtarget, wlink)) { case PHANTOM_SYMLINK_RETRY: { - /* if target doesn't exist, add to phantom symlinks trie */ - wchar_t wfullpath[MAX_LONG_PATH]; - struct phantom_symlink_info *psi; - struct phantom_trie_node *node; - char *target_key; + /* the path process_phantom_symlink() itself probes */ + wchar_t relative[MAX_LONG_PATH], wfulltarget[MAX_LONG_PATH]; + wchar_t wfulllink[MAX_LONG_PATH]; + const wchar_t *rel = make_relative_to(wtarget, wlink, relative, + ARRAY_SIZE(relative)); + char *target_key = rel ? canonicalize_path(rel, wfulltarget) : NULL; + struct hashmap_entry key; + struct phantom_symlink_target *e; + int len; + + if (!target_key) + break; /* convert to absolute path to be independent of cwd */ - len = GetFullPathNameW(wlink, MAX_LONG_PATH, wfullpath, NULL); - if (!len || len >= MAX_LONG_PATH) { + len = GetFullPathNameW(wlink, ARRAY_SIZE(wfulllink), wfulllink, NULL); + if (!len || len >= ARRAY_SIZE(wfulllink)) { errno = err_win_to_posix(GetLastError()); + free(target_key); return -1; } - target_key = phantom_symlink_target_key(wtarget, wlink); - if (!target_key) - break; - - /* over-allocate and fill phantom_symlink_info structure */ - psi = xmalloc(sizeof(struct phantom_symlink_info) + - sizeof(wchar_t) * (len + wcslen(wtarget) + 2)); - psi->wlink = (wchar_t *)(psi + 1); - wcscpy(psi->wlink, wfullpath); - psi->wtarget = psi->wlink + len + 1; - wcscpy(psi->wtarget, wtarget); - EnterCriticalSection(&phantom_symlinks_cs); - node = phantom_trie_walk(target_key, 1); - if (node) { - psi->next = node->waiters; - node->waiters = psi; - } else { - free(psi); + hashmap_entry_init(&key, fspathhash(target_key)); + e = hashmap_get_entry_from_hash(&phantom_symlinks, key.hash, + target_key, + struct phantom_symlink_target, ent); + if (!e) { + FLEX_ALLOC_STR(e, target, target_key); + e->wtarget = xwcsdup(wfulltarget); + hashmap_entry_init(&e->ent, key.hash); + hashmap_add(&phantom_symlinks, &e->ent); } + ALLOC_GROW(e->items, e->nr + 1, e->alloc); + e->items[e->nr++].wlink = xwcsdup(wfulllink); LeaveCriticalSection(&phantom_symlinks_cs); free(target_key); break; } case PHANTOM_SYMLINK_DIRECTORY: /* if we created a dir symlink, wake others waiting on it */ - phantom_symlink_directory_resolved(wtarget, wlink); + process_phantom_symlinks(wlink); break; default: break; @@ -976,11 +846,8 @@ int mingw_mkdir(const char *path, int mode UNUSED) return -1; ret = _wmkdir(wpath); - if (!ret) { - char *created = phantom_symlink_canonicalize(wpath); - process_phantom_symlinks_under(created); - free(created); - } + if (!ret) + process_phantom_symlinks(wpath); if (!ret && needs_hiding(path)) return set_hidden_flag(wpath, 1); return ret; @@ -3716,7 +3583,7 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch break; /* There may be dangling phantom symlinks that point at this * one, which should now morph into directory symlinks. */ - phantom_symlink_directory_resolved(wtarget, wlink); + process_phantom_symlinks(wlink); return 0; default: BUG("unhandled symlink type"); diff --git a/t/t2041-checkout-symlink-through-symlink.sh b/t/t2041-checkout-symlink-through-symlink.sh deleted file mode 100755 index fd7771be3e3652..00000000000000 --- a/t/t2041-checkout-symlink-through-symlink.sh +++ /dev/null @@ -1,56 +0,0 @@ -#!/bin/sh - -test_description='checkout a symlink nested through another symlink on Windows - -A phantom symlink may target a path that goes through another -symlink. Ensures that such a symlink is still upgraded to a directory -symlink once its real target appears, even when that target directory -is only populated after the leading symlink has already resolved.' - -# Tell MSYS to create native symlinks. Without this flag test-lib's -# prerequisite detection for SYMLINKS doesn't detect the right thing. -MSYS=winsymlinks:nativestrict && export MSYS - -. ./test-lib.sh - -if ! test_have_prereq MINGW,SYMLINKS -then - skip_all='skipping $0: MinGW-only test, which requires symlink support.' - test_done -fi - -# Adds a symlink to the index without clobbering the work tree. -cache_symlink () { - sha=$(printf '%s' "$1" | git hash-object --stdin -w) && - git update-index --add --cacheinfo 120000,$sha,"$2" -} - -# Adds a regular file to the index without clobbering the work tree. -cache_file () { - sha=$(printf '%s' "$1" | git hash-object --stdin -w) && - git update-index --add --cacheinfo 100644,$sha,"$2" -} - -test_expect_success 'symlink nested through another symlink resolves' ' - test_when_finished "rm -rf chained" && - mkdir chained && - ( - cd chained && - git init -q && - - cache_symlink realdir leading && - cache_symlink leading/sub nested && - cache_file content realdir/sub/file && - - git checkout -- . && - - # "cmd.exe /c dir" marks directory symlinks as - # and file symlinks as ; unlike opening a path - # through the symlink, this distinguishes the two even - # though both resolve identically for plain reads. - cmd.exe //c dir . >dir-listing && - grep "SYMLINKD.*nested" dir-listing - ) -' - -test_done From 16ff38a0629c83a0782493b525e6751d776a1be4 Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Thu, 6 Aug 2026 08:39:59 -0700 Subject: [PATCH 5/7] path-trie: add a trie mapping paths to entry lists Add a small data structure that maps file system paths to lists of intrusive entries, keyed componentwise, so that -- unlike a flat hashmap keyed by whole paths -- prefix questions can be answered: all entries registered at or below a path can be removed in one operation (path_trie_drain()) or moved onto another path (path_trie_move()). Both operations are needed when a path turns out to be an alias for another path, as with symbolic links on Windows: a symlink whose type cannot be determined yet may have a target that passes through another such symlink, and once the latter resolves, everything registered through it must be findable via the real path it points at. The trie records each entry's current registration path in the entry itself, and path_trie_move() rewrites it, so a drained entry can always be re-registered where it was last filed. Component comparison is optionally case-insensitive, chosen at init time. This will be used by the Windows-specific phantom symlink tracking in compat/mingw.c in the next commit; the data structure itself is platform-independent and comes with unit tests. Signed-off-by: Chris Harris --- Makefile | 2 + path-trie.c | 253 +++++++++++++++++++++++++++++++++++++ path-trie.h | 89 +++++++++++++ t/unit-tests/u-path-trie.c | 224 ++++++++++++++++++++++++++++++++ 4 files changed, 568 insertions(+) create mode 100644 path-trie.c create mode 100644 path-trie.h create mode 100644 t/unit-tests/u-path-trie.c diff --git a/Makefile b/Makefile index 87505e5df83ebf..90146a1822a551 100644 --- a/Makefile +++ b/Makefile @@ -1247,6 +1247,7 @@ LIB_OBJS += parse-options.o LIB_OBJS += patch-delta.o LIB_OBJS += patch-ids.o LIB_OBJS += path.o +LIB_OBJS += path-trie.o LIB_OBJS += path-walk.o LIB_OBJS += pathspec.o LIB_OBJS += pkt-line.o @@ -1540,6 +1541,7 @@ CLAR_TEST_SUITES += u-odb-inmemory CLAR_TEST_SUITES += u-oid-array CLAR_TEST_SUITES += u-oidmap CLAR_TEST_SUITES += u-oidtree +CLAR_TEST_SUITES += u-path-trie CLAR_TEST_SUITES += u-prio-queue CLAR_TEST_SUITES += u-reftable-basics CLAR_TEST_SUITES += u-reftable-block diff --git a/path-trie.c b/path-trie.c new file mode 100644 index 00000000000000..1da273f2820484 --- /dev/null +++ b/path-trie.c @@ -0,0 +1,253 @@ +#include "git-compat-util.h" +#include "path-trie.h" +#include "strbuf.h" + +struct path_trie_node { + struct hashmap_entry ent; /* in the parent node's `children` */ + struct hashmap children; + struct path_trie_entry *entries; + char component[FLEX_ARRAY]; +}; + +static int node_cmp(const void *cmp_data, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata) +{ + const unsigned int icase = *(const unsigned int *)cmp_data; + const struct path_trie_node *e = + container_of(eptr, const struct path_trie_node, ent); + const char *key = keydata ? keydata : + container_of(entry_or_key, const struct path_trie_node, + ent)->component; + + return icase ? strcasecmp(e->component, key) : + strcmp(e->component, key); +} + +static unsigned int component_hash(const struct path_trie *trie, + const char *component) +{ + return trie->icase ? strihash(component) : strhash(component); +} + +static struct path_trie_node *make_node(struct path_trie *trie, + const char *component) +{ + struct path_trie_node *node; + + FLEX_ALLOC_STR(node, component, component); + hashmap_init(&node->children, node_cmp, &trie->icase, 0); + hashmap_entry_init(&node->ent, component_hash(trie, component)); + return node; +} + +static struct path_trie_node *get_child(struct path_trie *trie, + struct path_trie_node *node, + const char *component, int create) +{ + unsigned int hash = component_hash(trie, component); + struct path_trie_node *child = hashmap_get_entry_from_hash( + &node->children, hash, component, + struct path_trie_node, ent); + + if (!child && create) { + child = make_node(trie, component); + hashmap_add(&node->children, &child->ent); + } + return child; +} + +/* + * Walks the trie to the node for `path`, optionally creating missing + * nodes on the way. Returns NULL if the node does not exist (and + * `create` is not set), or if `path` contains no components at all. + */ +static struct path_trie_node *walk(struct path_trie *trie, const char *path, + int create) +{ + struct path_trie_node *node = trie->root; + struct strbuf component = STRBUF_INIT; + const char *p = path; + + while (*p && node) { + const char *start; + + while (is_dir_sep(*p)) + p++; + if (!*p) + break; + start = p; + while (*p && !is_dir_sep(*p)) + p++; + + strbuf_reset(&component); + strbuf_add(&component, start, p - start); + node = get_child(trie, node, component.buf, create); + } + + strbuf_release(&component); + return node == trie->root ? NULL : node; +} + +void path_trie_init(struct path_trie *trie, int icase) +{ + trie->icase = !!icase; + trie->root = make_node(trie, ""); +} + +static void free_node(struct path_trie_node *node) +{ + struct hashmap_iter iter; + struct path_trie_node *child; + + hashmap_for_each_entry(&node->children, &iter, child, ent) + free_node(child); + hashmap_clear(&node->children); + free(node); +} + +void path_trie_clear(struct path_trie *trie) +{ + if (trie->root) { + free_node(trie->root); + trie->root = NULL; + } +} + +void path_trie_add(struct path_trie *trie, const char *path, + struct path_trie_entry *entry) +{ + struct path_trie_node *node = walk(trie, path, 1); + char *key = xstrdup(path); /* `path` may be `entry->key` itself */ + + if (!node) + BUG("cannot add to path trie under '%s'", path); + free(entry->key); + entry->key = key; + entry->next = node->entries; + node->entries = entry; +} + +/* Unlinks and returns all entries at `node` and below, prepended to `list`. */ +static struct path_trie_entry *drain_node(struct path_trie_node *node, + struct path_trie_entry *list) +{ + struct hashmap_iter iter; + struct path_trie_node *child; + + while (node->entries) { + struct path_trie_entry *e = node->entries; + + node->entries = e->next; + e->next = list; + list = e; + } + + hashmap_for_each_entry(&node->children, &iter, child, ent) + list = drain_node(child, list); + return list; +} + +struct path_trie_entry *path_trie_drain(struct path_trie *trie, + const char *path) +{ + struct path_trie_node *node = walk(trie, path, 0); + + return node ? drain_node(node, NULL) : NULL; +} + +/* + * Moves entries at `from` and below to the same position under `to`; + * `to_path` holds `to`'s path and is extended and restored around + * each recursion step, so moved entries' keys can be rewritten. + */ +static void move_node(struct path_trie *trie, struct path_trie_node *to, + struct path_trie_node *from, struct strbuf *to_path) +{ + struct hashmap_iter iter; + struct path_trie_node *from_child; + + while (from->entries) { + struct path_trie_entry *e = from->entries; + + from->entries = e->next; + e->next = to->entries; + to->entries = e; + free(e->key); + e->key = xstrdup(to_path->buf); + } + + hashmap_for_each_entry(&from->children, &iter, from_child, ent) { + struct path_trie_node *to_child = + get_child(trie, to, from_child->component, 1); + size_t len = to_path->len; + + strbuf_addch(to_path, '/'); + strbuf_addstr(to_path, from_child->component); + move_node(trie, to_child, from_child, to_path); + strbuf_setlen(to_path, len); + } +} + +static size_t component_len(const char *path) +{ + size_t len = 0; + + while (path[len] && !is_dir_sep(path[len])) + len++; + return len; +} + +/* Is `path` equal to, or nested somewhere below, `prefix`? */ +static int path_is_at_or_below(const struct path_trie *trie, + const char *path, const char *prefix) +{ + while (1) { + size_t p_len, x_len; + + while (is_dir_sep(*prefix)) + prefix++; + while (is_dir_sep(*path)) + path++; + if (!*prefix) + return 1; + if (!*path) + return 0; + + p_len = component_len(prefix); + x_len = component_len(path); + if (p_len != x_len) + return 0; + if (trie->icase ? strncasecmp(path, prefix, p_len) : + strncmp(path, prefix, p_len)) + return 0; + prefix += p_len; + path += x_len; + } +} + +void path_trie_move(struct path_trie *trie, const char *from, + const char *to) +{ + struct path_trie_node *from_node; + struct path_trie_node *to_node; + struct strbuf to_path = STRBUF_INIT; + + /* + * Moving a subtree into itself (or below itself) cannot + * terminate meaningfully; treat it as a no-op. + */ + if (path_is_at_or_below(trie, to, from)) + return; + + from_node = walk(trie, from, 0); + if (!from_node) + return; + to_node = walk(trie, to, 1); + if (!to_node) + return; + strbuf_addstr(&to_path, to); + move_node(trie, to_node, from_node, &to_path); + strbuf_release(&to_path); +} diff --git a/path-trie.h b/path-trie.h new file mode 100644 index 00000000000000..753826f2355309 --- /dev/null +++ b/path-trie.h @@ -0,0 +1,89 @@ +#ifndef PATH_TRIE_H +#define PATH_TRIE_H + +#include "hashmap.h" + +/* + * A trie over the components of file system paths, mapping each path + * to a list of caller-provided entries. + * + * Entries are intrusive, like `struct hashmap_entry`: embed a + * `struct path_trie_entry` in your own struct and use `container_of` + * to get back to it. An entry belongs to at most one trie at a time. + * + * Paths are split on directory separators (both '/' and '\\'); + * repeated separators are ignored. Components are compared + * case-insensitively if `icase` is set at init time. Callers are + * expected to pass paths in a consistent (e.g. canonicalized, + * absolute) form; the trie does not resolve '.' or '..'. + * + * Unlike a flat hashmap keyed by whole paths, a trie can answer + * prefix questions: all entries registered at or below a path can be + * removed in one operation (path_trie_drain()) or moved onto another + * path (path_trie_move()). Both are useful when a path turns out to + * be an alias for another (e.g. a symbolic link), and everything + * registered through the alias must be found via the real path from + * then on. + */ + +struct path_trie_entry { + struct path_trie_entry *next; + /* + * The path this entry is currently registered under, owned by + * the trie: set by path_trie_add(), rewritten by + * path_trie_move(). After draining, the caller may pass the + * entry back to path_trie_add() (e.g. with `entry->key` + * itself) to re-register it, or free the key -- e.g. via + * path_trie_entry_clear() -- once done with the entry. + */ + char *key; +}; + +static inline void path_trie_entry_clear(struct path_trie_entry *entry) +{ + FREE_AND_NULL(entry->key); +} + +struct path_trie_node; + +struct path_trie { + struct path_trie_node *root; + unsigned int icase; +}; + +void path_trie_init(struct path_trie *trie, int icase); + +/* + * Removes all nodes from the trie. Entries themselves are not freed + * (the trie does not own them); drain first if they need releasing. + */ +void path_trie_clear(struct path_trie *trie); + +/* + * Adds an entry at `path`, recording the path in `entry->key` + * (replacing -- and releasing -- any previous key, so passing + * `entry->key` itself as `path` re-registers a drained entry). + */ +void path_trie_add(struct path_trie *trie, const char *path, + struct path_trie_entry *entry); + +/* + * Removes and returns all entries registered at `path` or nested + * anywhere below it, linked through their `next` fields (in no + * particular order). Returns NULL if there are none. + */ +struct path_trie_entry *path_trie_drain(struct path_trie *trie, + const char *path); + +/* + * Moves every entry registered at or below `from` to the + * corresponding path with the `from` prefix replaced by `to`, e.g. + * moving "a/b" to "x" moves entries at "a/b/c" to "x/c", updating + * each moved entry's `key` accordingly. Entries already present + * under `to` are kept. Moving a path into itself or below itself is + * a no-op. + */ +void path_trie_move(struct path_trie *trie, const char *from, + const char *to); + +#endif /* PATH_TRIE_H */ diff --git a/t/unit-tests/u-path-trie.c b/t/unit-tests/u-path-trie.c new file mode 100644 index 00000000000000..688c4347c693a6 --- /dev/null +++ b/t/unit-tests/u-path-trie.c @@ -0,0 +1,224 @@ +#include "unit-test.h" +#include "path-trie.h" + +struct test_entry { + struct path_trie_entry ent; + const char *tag; +}; + +static struct test_entry *entry(const char *tag) +{ + struct test_entry *e = xcalloc(1, sizeof(*e)); + + e->tag = tag; + return e; +} + +static size_t drain_count(struct path_trie *trie, const char *path) +{ + struct path_trie_entry *list = path_trie_drain(trie, path); + size_t n = 0; + + while (list) { + struct path_trie_entry *next = list->next; + + path_trie_entry_clear(list); + free(container_of(list, struct test_entry, ent)); + list = next; + n++; + } + return n; +} + +static int drained_tags_contain(struct path_trie_entry *list, const char *tag) +{ + for (; list; list = list->next) { + struct test_entry *e = + container_of(list, struct test_entry, ent); + + if (!strcmp(e->tag, tag)) + return 1; + } + return 0; +} + +static void free_drained(struct path_trie_entry *list) +{ + while (list) { + struct path_trie_entry *next = list->next; + + path_trie_entry_clear(list); + free(container_of(list, struct test_entry, ent)); + list = next; + } +} + +void test_path_trie__drain_empty(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + cl_assert_equal_p(path_trie_drain(&trie, "a/b"), NULL); + path_trie_clear(&trie); +} + +void test_path_trie__drain_exact_path(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b/c", &entry("x")->ent); + cl_assert_equal_i(drain_count(&trie, "a/b/c"), 1); + cl_assert_equal_i(drain_count(&trie, "a/b/c"), 0); + path_trie_clear(&trie); +} + +void test_path_trie__drain_covers_subtree(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b", &entry("shallow")->ent); + path_trie_add(&trie, "a/b/c/d", &entry("deep")->ent); + path_trie_add(&trie, "a/other", &entry("sibling")->ent); + cl_assert_equal_i(drain_count(&trie, "a/b"), 2); + cl_assert_equal_i(drain_count(&trie, "a"), 1); + path_trie_clear(&trie); +} + +void test_path_trie__drain_does_not_cover_ancestors(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a", &entry("above")->ent); + cl_assert_equal_i(drain_count(&trie, "a/b"), 0); + cl_assert_equal_i(drain_count(&trie, "a"), 1); + path_trie_clear(&trie); +} + +void test_path_trie__multiple_entries_per_path(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b", &entry("one")->ent); + path_trie_add(&trie, "a/b", &entry("two")->ent); + path_trie_add(&trie, "a/b", &entry("three")->ent); + cl_assert_equal_i(drain_count(&trie, "a/b"), 3); + path_trie_clear(&trie); +} + +void test_path_trie__separators_are_normalized(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b/c", &entry("slash")->ent); + path_trie_add(&trie, "a//b///c", &entry("doubled")->ent); + cl_assert_equal_i(drain_count(&trie, "/a/b/c/"), 2); + path_trie_clear(&trie); +} + +void test_path_trie__case_sensitivity(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b", &entry("lower")->ent); + cl_assert_equal_i(drain_count(&trie, "A/B"), 0); + cl_assert_equal_i(drain_count(&trie, "a/b"), 1); + path_trie_clear(&trie); + + path_trie_init(&trie, 1); + path_trie_add(&trie, "a/b", &entry("lower")->ent); + cl_assert_equal_i(drain_count(&trie, "A/B"), 1); + path_trie_clear(&trie); +} + +void test_path_trie__move_relocates_subtree(void) +{ + struct path_trie trie; + struct path_trie_entry *drained; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "link/sub", &entry("via-link")->ent); + path_trie_add(&trie, "link/sub/deeper", &entry("nested")->ent); + + path_trie_move(&trie, "link", "real"); + + cl_assert_equal_i(drain_count(&trie, "link"), 0); + drained = path_trie_drain(&trie, "real/sub"); + cl_assert(drained != NULL); + cl_assert(drained_tags_contain(drained, "via-link")); + cl_assert(drained_tags_contain(drained, "nested")); + free_drained(drained); + path_trie_clear(&trie); +} + +void test_path_trie__move_merges_with_existing(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "from/x", &entry("moved")->ent); + path_trie_add(&trie, "to/x", &entry("already-there")->ent); + + path_trie_move(&trie, "from", "to"); + + cl_assert_equal_i(drain_count(&trie, "to/x"), 2); + path_trie_clear(&trie); +} + +void test_path_trie__move_missing_source_is_noop(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "to/x", &entry("existing")->ent); + path_trie_move(&trie, "does/not/exist", "to"); + cl_assert_equal_i(drain_count(&trie, "to"), 1); + path_trie_clear(&trie); +} + +void test_path_trie__move_into_itself_is_noop(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b", &entry("kept")->ent); + path_trie_move(&trie, "a", "a/b/c"); + path_trie_move(&trie, "a", "a"); + cl_assert_equal_i(drain_count(&trie, "a/b"), 1); + path_trie_clear(&trie); +} + +void test_path_trie__move_to_ancestor(void) +{ + struct path_trie trie; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "a/b/c", &entry("moves-up")->ent); + path_trie_move(&trie, "a/b", "a"); + cl_assert_equal_i(drain_count(&trie, "a/c"), 1); + path_trie_clear(&trie); +} + +void test_path_trie__move_rewrites_keys(void) +{ + struct path_trie trie; + struct path_trie_entry *drained; + + path_trie_init(&trie, 0); + path_trie_add(&trie, "link/sub", &entry("aliased")->ent); + path_trie_move(&trie, "link", "real"); + + drained = path_trie_drain(&trie, "real"); + cl_assert(drained != NULL); + cl_assert_equal_s(drained->key, "real/sub"); + + /* re-registering by the entry's own key must be safe */ + path_trie_add(&trie, drained->key, drained); + cl_assert_equal_i(drain_count(&trie, "real/sub"), 1); + path_trie_clear(&trie); +} From cc83bc5038f5bf5c8b2a81a8e127da8032f24061 Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Thu, 6 Aug 2026 08:40:46 -0700 Subject: [PATCH 6/7] mingw: track phantom symlinks in a path trie The flat hashmap introduced two commits ago wakes only the phantom symlinks whose target is exactly the path that was just created. That misses targets that pass through another symlink: given a symlink "leading" -> "realdir" and a symlink "nested" -> "leading/sub", "nested" is registered under "leading/sub", but the directory that eventually appears is created under "realdir/sub" -- a different key aliasing the same location -- so "nested" is never woken and stays a file symlink. The old unconditional full-list rescan caught this case by brute force. Switch the tracking to the path trie added in the previous commit. Whenever a symlink resolves to a directory, everything registered at or below its own path is moved (path_trie_move()) to the corresponding path under its target, so later mkdir()s under the real path find those entries; the trie rewrites each moved entry's registration key, so an entry that still cannot resolve is re-filed where a future wake will look for it. Waking is a simple worklist: drain everything at or below the created path, probe each entry, and queue the target of every symlink that resolved to a directory, since other phantom symlinks may point through it. Re-add the t2041 regression test for exactly this chained-symlink scenario (it passes with the old full-list rescan and with this trie, and fails with the flat hashmap). On the 343-symlink, 37,016-directory fixture from the earlier commits, checkout takes 52.2s -- unchanged from the flat hashmap (54.9s) within noise, and still down from 330s before this series. Signed-off-by: Chris Harris --- compat/mingw.c | 211 +++++++++++--------- t/t2041-checkout-symlink-through-symlink.sh | 56 ++++++ 2 files changed, 177 insertions(+), 90 deletions(-) create mode 100755 t/t2041-checkout-symlink-through-symlink.sh diff --git a/compat/mingw.c b/compat/mingw.c index 0f50074995c71f..973d58656bc3b3 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -9,7 +9,7 @@ #include "dir.h" #include "environment.h" #include "gettext.h" -#include "hashmap.h" +#include "path-trie.h" #include "repository.h" #include "run-command.h" #include "strbuf.h" @@ -451,53 +451,41 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink) /* * Newly created symlinks to non-existing targets are indexed by a - * hashmap keyed by their (canonicalized, absolute) target path, so - * that mkdir() creating that path -- or a symlink at that path turning - * out to be a directory symlink -- wakes only the entries actually - * waiting on it, instead of re-probing every phantom symlink created - * so far. + * path trie keyed by their (canonicalized, absolute) target path, so + * that mkdir() creating that path -- or a symlink at that path + * turning out to be a directory symlink -- wakes only the entries + * actually waiting on it, instead of re-probing every phantom + * symlink created so far. A trie rather than a flat map because a + * phantom symlink's target may pass through *another* symlink; once + * that one resolves, everything registered through it is moved to + * the corresponding real path (path_trie_move()), where later + * mkdir()s can find it. */ struct phantom_symlink_info { + struct path_trie_entry ent; wchar_t *wlink; -}; - -struct phantom_symlink_target { - struct hashmap_entry ent; wchar_t *wtarget; - size_t nr, alloc; - struct phantom_symlink_info *items; - char target[FLEX_ARRAY]; }; -static int phantom_symlink_target_cmp(const void *cmp_data UNUSED, - const struct hashmap_entry *eptr, - const struct hashmap_entry *entry_or_key, - const void *keydata) -{ - const struct phantom_symlink_target *e = - container_of(eptr, const struct phantom_symlink_target, ent); +static struct path_trie phantom_symlinks; +static CRITICAL_SECTION phantom_symlinks_cs; - return !fspatheq(e->target, keydata ? keydata : - container_of(entry_or_key, const struct phantom_symlink_target, - ent)->target); +static void free_phantom_symlink(struct phantom_symlink_info *psi) +{ + path_trie_entry_clear(&psi->ent); + free(psi->wlink); + free(psi->wtarget); + free(psi); } -static struct hashmap phantom_symlinks = - HASHMAP_INIT(phantom_symlink_target_cmp, NULL); -static CRITICAL_SECTION phantom_symlinks_cs; - static wchar_t *xwcsdup(const wchar_t *s) { size_t size = sizeof(wchar_t) * (wcslen(s) + 1); return memcpy(xmalloc(size), s, size); } -/* - * Returns the canonicalized, absolute UTF-8 form of wpath. If wout is - * non-NULL, also fills it with the canonicalized, absolute wide form - * (must have room for at least MAX_LONG_PATH wchar_t). - */ -static char *canonicalize_path(const wchar_t *wpath, wchar_t *wout) +/* Returns the canonicalized, absolute UTF-8 form of wpath. */ +static char *canonicalize_path(const wchar_t *wpath) { wchar_t wfullpath[MAX_LONG_PATH]; char utf8[MAX_LONG_PATH * 3]; @@ -506,56 +494,110 @@ static char *canonicalize_path(const wchar_t *wpath, wchar_t *wout) if (!len || len >= ARRAY_SIZE(wfullpath) || xwcstoutf(utf8, wfullpath, sizeof(utf8)) < 0) return NULL; - if (wout) - wcscpy(wout, wfullpath); return xstrdup(utf8); } /* - * Wakes every phantom symlink whose target is exactly wpath: mkdir() - * creating that path, or a symlink at that path turning out to be a - * directory symlink, may let them resolve. + * The canonicalized, absolute UTF-8 form of wtarget, resolved against + * wlink's directory if relative: the path process_phantom_symlink() + * itself probes, and thus the path whose creation can resolve the + * symlink. */ -static void process_phantom_symlinks(const wchar_t *wpath) +static char *canonicalize_target(const wchar_t *wtarget, const wchar_t *wlink) { - char *target_key = canonicalize_path(wpath, NULL); - struct hashmap_entry key; - struct phantom_symlink_target *e; - size_t i; - - if (!target_key) - return; + wchar_t relative[MAX_LONG_PATH]; + const wchar_t *rel = make_relative_to(wtarget, wlink, relative, + ARRAY_SIZE(relative)); - EnterCriticalSection(&phantom_symlinks_cs); - hashmap_entry_init(&key, fspathhash(target_key)); - e = hashmap_get_entry_from_hash(&phantom_symlinks, key.hash, target_key, - struct phantom_symlink_target, ent); + return rel ? canonicalize_path(rel) : NULL; +} - for (i = 0; e && i < e->nr; ) { - enum phantom_symlink_result result = - process_phantom_symlink(e->wtarget, e->items[i].wlink); - wchar_t *wlink = e->items[i].wlink; +/* + * Wakes every phantom symlink registered at or below `key` (a + * canonicalized, absolute UTF-8 path; ownership is taken): the path + * just came into existence, which may let them resolve. Every + * resolved directory symlink queues its own target for waking in + * turn, since other phantom symlinks may point through it. + */ +static void process_phantom_symlinks_at(char *key) +{ + char **queue = NULL; + size_t queue_nr = 0, queue_alloc = 0; - if (result == PHANTOM_SYMLINK_RETRY) { - i++; - continue; - } + ALLOC_GROW(queue, queue_nr + 1, queue_alloc); + queue[queue_nr++] = key; - e->items[i] = e->items[--e->nr]; - if (result == PHANTOM_SYMLINK_DIRECTORY) - process_phantom_symlinks(wlink); - free(wlink); - } + EnterCriticalSection(&phantom_symlinks_cs); + while (queue_nr) { + char *current = queue[--queue_nr]; + struct path_trie_entry *drained = + path_trie_drain(&phantom_symlinks, current); + + while (drained) { + struct phantom_symlink_info *psi = container_of( + drained, struct phantom_symlink_info, ent); + + drained = drained->next; + + switch (process_phantom_symlink(psi->wtarget, + psi->wlink)) { + case PHANTOM_SYMLINK_RETRY: + path_trie_add(&phantom_symlinks, psi->ent.key, + &psi->ent); + break; + case PHANTOM_SYMLINK_DIRECTORY: { + /* + * This symlink is now a directory symlink; + * anything registered through its own path + * is reachable via its target from now on, + * and may be able to resolve. + */ + char *own = canonicalize_path(psi->wlink); - if (e && !e->nr) { - hashmap_remove(&phantom_symlinks, &e->ent, target_key); - free(e->wtarget); - free(e->items); - free(e); + if (own) { + path_trie_move(&phantom_symlinks, own, + psi->ent.key); + free(own); + } + ALLOC_GROW(queue, queue_nr + 1, queue_alloc); + queue[queue_nr++] = xstrdup(psi->ent.key); + free_phantom_symlink(psi); + break; + } + default: + free_phantom_symlink(psi); + break; + } + } + free(current); } LeaveCriticalSection(&phantom_symlinks_cs); + free(queue); +} - free(target_key); +static void process_phantom_symlinks(const wchar_t *wpath) +{ + char *key = canonicalize_path(wpath); + + if (key) + process_phantom_symlinks_at(key); +} + +/* A directory symlink wlink -> wtarget was created (or so resolved). */ +static void directory_symlink_created(const wchar_t *wtarget, + const wchar_t *wlink) +{ + char *own = canonicalize_path(wlink); + char *target_key = canonicalize_target(wtarget, wlink); + + if (own && target_key) { + EnterCriticalSection(&phantom_symlinks_cs); + path_trie_move(&phantom_symlinks, own, target_key); + LeaveCriticalSection(&phantom_symlinks_cs); + } + free(own); + if (target_key) + process_phantom_symlinks_at(target_key); } static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) @@ -569,14 +611,9 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) /* convert to directory symlink if target exists */ switch (process_phantom_symlink(wtarget, wlink)) { case PHANTOM_SYMLINK_RETRY: { - /* the path process_phantom_symlink() itself probes */ - wchar_t relative[MAX_LONG_PATH], wfulltarget[MAX_LONG_PATH]; wchar_t wfulllink[MAX_LONG_PATH]; - const wchar_t *rel = make_relative_to(wtarget, wlink, relative, - ARRAY_SIZE(relative)); - char *target_key = rel ? canonicalize_path(rel, wfulltarget) : NULL; - struct hashmap_entry key; - struct phantom_symlink_target *e; + char *target_key = canonicalize_target(wtarget, wlink); + struct phantom_symlink_info *psi; int len; if (!target_key) @@ -590,26 +627,19 @@ static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) return -1; } + psi = xcalloc(1, sizeof(*psi)); + psi->wlink = xwcsdup(wfulllink); + psi->wtarget = xwcsdup(wtarget); + EnterCriticalSection(&phantom_symlinks_cs); - hashmap_entry_init(&key, fspathhash(target_key)); - e = hashmap_get_entry_from_hash(&phantom_symlinks, key.hash, - target_key, - struct phantom_symlink_target, ent); - if (!e) { - FLEX_ALLOC_STR(e, target, target_key); - e->wtarget = xwcsdup(wfulltarget); - hashmap_entry_init(&e->ent, key.hash); - hashmap_add(&phantom_symlinks, &e->ent); - } - ALLOC_GROW(e->items, e->nr + 1, e->alloc); - e->items[e->nr++].wlink = xwcsdup(wfulllink); + path_trie_add(&phantom_symlinks, target_key, &psi->ent); LeaveCriticalSection(&phantom_symlinks_cs); free(target_key); break; } case PHANTOM_SYMLINK_DIRECTORY: /* if we created a dir symlink, wake others waiting on it */ - process_phantom_symlinks(wlink); + directory_symlink_created(wtarget, wlink); break; default: break; @@ -3583,7 +3613,7 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch break; /* There may be dangling phantom symlinks that point at this * one, which should now morph into directory symlinks. */ - process_phantom_symlinks(wlink); + directory_symlink_created(wtarget, wlink); return 0; default: BUG("unhandled symlink type"); @@ -4536,6 +4566,7 @@ int wmain(int argc, const wchar_t **wargv) /* initialize critical section for waitpid pinfo_t list */ InitializeCriticalSection(&pinfo_cs); InitializeCriticalSection(&phantom_symlinks_cs); + path_trie_init(&phantom_symlinks, 1); /* initialize critical section for fscache */ InitializeCriticalSection(&fscache_cs); diff --git a/t/t2041-checkout-symlink-through-symlink.sh b/t/t2041-checkout-symlink-through-symlink.sh new file mode 100755 index 00000000000000..fd7771be3e3652 --- /dev/null +++ b/t/t2041-checkout-symlink-through-symlink.sh @@ -0,0 +1,56 @@ +#!/bin/sh + +test_description='checkout a symlink nested through another symlink on Windows + +A phantom symlink may target a path that goes through another +symlink. Ensures that such a symlink is still upgraded to a directory +symlink once its real target appears, even when that target directory +is only populated after the leading symlink has already resolved.' + +# Tell MSYS to create native symlinks. Without this flag test-lib's +# prerequisite detection for SYMLINKS doesn't detect the right thing. +MSYS=winsymlinks:nativestrict && export MSYS + +. ./test-lib.sh + +if ! test_have_prereq MINGW,SYMLINKS +then + skip_all='skipping $0: MinGW-only test, which requires symlink support.' + test_done +fi + +# Adds a symlink to the index without clobbering the work tree. +cache_symlink () { + sha=$(printf '%s' "$1" | git hash-object --stdin -w) && + git update-index --add --cacheinfo 120000,$sha,"$2" +} + +# Adds a regular file to the index without clobbering the work tree. +cache_file () { + sha=$(printf '%s' "$1" | git hash-object --stdin -w) && + git update-index --add --cacheinfo 100644,$sha,"$2" +} + +test_expect_success 'symlink nested through another symlink resolves' ' + test_when_finished "rm -rf chained" && + mkdir chained && + ( + cd chained && + git init -q && + + cache_symlink realdir leading && + cache_symlink leading/sub nested && + cache_file content realdir/sub/file && + + git checkout -- . && + + # "cmd.exe /c dir" marks directory symlinks as + # and file symlinks as ; unlike opening a path + # through the symlink, this distinguishes the two even + # though both resolve identically for plain reads. + cmd.exe //c dir . >dir-listing && + grep "SYMLINKD.*nested" dir-listing + ) +' + +test_done From d5f6dcc236df6aee31334d84ffb0c5eed55f9046 Mon Sep 17 00:00:00 2001 From: Chris Harris Date: Thu, 6 Aug 2026 09:30:58 -0700 Subject: [PATCH 7/7] path-trie: pass only icase to path_is_at_or_below() The helper only consults the trie's icase flag, not the trie itself; narrow the parameter to say so. Signed-off-by: Chris Harris --- path-trie.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/path-trie.c b/path-trie.c index 1da273f2820484..6f8134f1fee145 100644 --- a/path-trie.c +++ b/path-trie.c @@ -200,8 +200,8 @@ static size_t component_len(const char *path) } /* Is `path` equal to, or nested somewhere below, `prefix`? */ -static int path_is_at_or_below(const struct path_trie *trie, - const char *path, const char *prefix) +static int path_is_at_or_below(const char *path, const char *prefix, + unsigned int icase) { while (1) { size_t p_len, x_len; @@ -219,7 +219,7 @@ static int path_is_at_or_below(const struct path_trie *trie, x_len = component_len(path); if (p_len != x_len) return 0; - if (trie->icase ? strncasecmp(path, prefix, p_len) : + if (icase ? strncasecmp(path, prefix, p_len) : strncmp(path, prefix, p_len)) return 0; prefix += p_len; @@ -238,7 +238,7 @@ void path_trie_move(struct path_trie *trie, const char *from, * Moving a subtree into itself (or below itself) cannot * terminate meaningfully; treat it as a no-op. */ - if (path_is_at_or_below(trie, to, from)) + if (path_is_at_or_below(to, from, trie->icase)) return; from_node = walk(trie, from, 0);