From a896fdf01c64b7629a81f52052ce5ca221928762 Mon Sep 17 00:00:00 2001 From: Daniel Hiltgen Date: Wed, 7 Oct 2026 12:53:35 -0700 Subject: [PATCH] manifest: avoid symlinks on Windows (#18852) Store named manifests as regular files on Windows, where symlinks can be created successfully but fail when opened. Repair existing symlinks lazily on read using the verified blob and existing copy path. Keep locking consistent across platforms and avoid reacquiring the store lock during pruning. Fixes #18847 --- manifest/manifest.go | 40 +++++++++++++++--- manifest/manifest_test.go | 89 +++++++++++++++++++++++++++++++++++++++ manifest/paths.go | 11 +++-- 3 files changed, 130 insertions(+), 10 deletions(-) diff --git a/manifest/manifest.go b/manifest/manifest.go index 61c808324..f476743b4 100644 --- a/manifest/manifest.go +++ b/manifest/manifest.go @@ -416,9 +416,9 @@ func ReferencedBlobDigestsForName(n model.Name) ([]string, error) { return referencedBlobDigestsForData(digest, data) } -// RetainedBlobDigests returns the complete set of blob digests reachable from +// retainedBlobDigestsLocked returns the complete set of blob digests reachable from // all named manifest objects in the local store. -func RetainedBlobDigests() (map[string]struct{}, error) { +func retainedBlobDigestsLocked() (map[string]struct{}, error) { refs, err := namedManifestRefs(true) if err != nil { return nil, err @@ -426,7 +426,7 @@ func RetainedBlobDigests() (map[string]struct{}, error) { retained := make(map[string]struct{}) for n, ref := range refs { - data, _, digest, err := readVerifiedManifest(ref.path, ref.root) + data, _, digest, err := readVerifiedManifestLocked(ref.path, ref.root) if err != nil { slog.Warn("bad manifest", "name", n, "error", err) continue @@ -592,7 +592,7 @@ func RemoveUnreferencedBlobs(candidates ...string) ([]string, error) { } func removeUnreferencedBlobs(candidates ...string) ([]string, error) { - inUse, err := RetainedBlobDigests() + inUse, err := retainedBlobDigestsLocked() if err != nil { return nil, err } @@ -1248,6 +1248,11 @@ func linkManifest(name model.Name, digest string) error { return err } + // Avoid symlinks on windows + if runtime.GOOS == "windows" { + return copyManifestFile(blobPath, manifestPath) + } + if rel, err := filepath.Rel(filepath.Dir(manifestPath), blobPath); err == nil { tempName, err := tempManifestPath(filepath.Dir(manifestPath)) if err != nil { @@ -1373,6 +1378,12 @@ func replaceManifestPath(tempName, dst string) error { // Regular-file manifests are treated as legacy/copy fallback manifests and are // opened without mutating the local store. func OpenVerifiedManifest(path, root string) (*os.File, string, error) { + manifestStoreMu.Lock() + defer manifestStoreMu.Unlock() + return openVerifiedManifestLocked(path, root) +} + +func openVerifiedManifestLocked(path, root string) (*os.File, string, error) { resolvedRoot, err := filepath.EvalSymlinks(root) if err != nil { return nil, "", err @@ -1403,7 +1414,11 @@ func OpenVerifiedManifest(path, root string) (*os.File, string, error) { return nil, "", fmt.Errorf("manifest symlink target %q does not match blob %q", target, blobPath) } - f, err := os.Open(path) + readPath := path + if runtime.GOOS == "windows" { + readPath = blobPath + } + f, err := os.Open(readPath) if err != nil { return nil, "", err } @@ -1416,6 +1431,13 @@ func OpenVerifiedManifest(path, root string) (*os.File, string, error) { return nil, "", err } + if runtime.GOOS == "windows" { + // Replace symlink with copy on windows + if err := copyManifestFile(blobPath, path); err != nil { + slog.Warn("could not replace manifest symlink", "path", path, "error", err) + } + } + return f, digest, nil } @@ -1496,7 +1518,13 @@ func readManifestPath(path, root string) ([]byte, error) { } func readVerifiedManifest(path, root string) ([]byte, os.FileInfo, string, error) { - f, digest, err := OpenVerifiedManifest(path, root) + manifestStoreMu.Lock() + defer manifestStoreMu.Unlock() + return readVerifiedManifestLocked(path, root) +} + +func readVerifiedManifestLocked(path, root string) ([]byte, os.FileInfo, string, error) { + f, digest, err := openVerifiedManifestLocked(path, root) if err != nil { return nil, nil, "", err } diff --git a/manifest/manifest_test.go b/manifest/manifest_test.go index 0fa72b515..1c54c1dda 100644 --- a/manifest/manifest_test.go +++ b/manifest/manifest_test.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "slices" "strings" "testing" @@ -107,6 +108,15 @@ func TestWriteManifestStoresManifestAsBlob(t *testing.T) { if err != nil { t.Fatal(err) } + if runtime.GOOS == "windows" { + info, err := os.Lstat(manifestPath) + if err != nil { + t.Fatal(err) + } + if !info.Mode().IsRegular() { + t.Fatalf("Windows manifest mode = %v, want regular file", info.Mode()) + } + } manifestData, err := os.ReadFile(manifestPath) if err != nil { t.Fatal(err) @@ -138,6 +148,85 @@ func TestWriteManifestStoresManifestAsBlob(t *testing.T) { } } +func TestReadManifestSymlink(t *testing.T) { + for _, action := range []string{"read", "parse", "prune", "corrupt"} { + t.Run(action, func(t *testing.T) { + t.Setenv("OLLAMA_MODELS", t.TempDir()) + name := model.ParseName("example") + data := createManifestListData(t, + createManifestForTest("sha256:"+strings.Repeat("a", 64), "sha256:"+strings.Repeat("b", 64), RunnerGGML), + createManifestForTest("sha256:"+strings.Repeat("c", 64), "sha256:"+strings.Repeat("d", 64), RunnerLlamaCPP), + ) + digest := writeManifestBlobForTest(t, data) + blobPath, err := BlobsPath(digest) + if err != nil { + t.Fatal(err) + } + path, err := V2PathForName(name) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + target, err := filepath.Rel(filepath.Dir(path), blobPath) + if err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, path); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + + if action == "corrupt" { + if err := os.WriteFile(blobPath, []byte("corrupt"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := ReadManifestData(name); err == nil || !strings.Contains(err.Error(), "digest mismatch") { + t.Fatalf("ReadManifestData error = %v, want digest mismatch", err) + } + if got, err := os.Readlink(path); err != nil || got != target { + t.Fatalf("invalid manifest link changed: target = %q, error = %v", got, err) + } + return + } + + switch action { + case "parse": + if _, err := ParseNamedManifest(name); err != nil { + t.Fatal(err) + } + case "prune": + if removed, err := RemoveUnreferencedBlobs(digest); err != nil || len(removed) != 0 { + t.Fatalf("pruning linked manifest: removed = %v, error = %v", removed, err) + } + default: + got, err := ReadManifestData(name) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, data) { + t.Fatal("repair changed the manifest list") + } + } + info, err := os.Lstat(path) + if err != nil { + t.Fatal(err) + } + if runtime.GOOS == "windows" { + if !info.Mode().IsRegular() { + t.Fatalf("repaired manifest mode = %v, want regular file", info.Mode()) + } + } else if got, err := os.Readlink(path); err != nil || got != target { + t.Fatalf("manifest link changed: target = %q, error = %v", got, err) + } + got, err := os.ReadFile(path) + if err != nil || !bytes.Equal(got, data) { + t.Fatalf("repaired file differs from manifest list: %v", err) + } + }) + } +} + func TestV2ManifestPathEscapesHostPort(t *testing.T) { t.Setenv("OLLAMA_MODELS", t.TempDir()) diff --git a/manifest/paths.go b/manifest/paths.go index 79af2e7f4..cf3bfa256 100644 --- a/manifest/paths.go +++ b/manifest/paths.go @@ -119,10 +119,13 @@ func resolveManifestPath(n model.Name) (string, string, error) { if err != nil { return "", "", err } - // Stat, not Lstat: a v2 entry is a symlink, and a dangling one (for - // example after a pre-v2 daemon garbage-collected its manifest blob) - // must fall through to the legacy manifest instead of being selected. - if _, err := os.Stat(v2Path); err == nil { + // Resolve Windows links explicitly, preserving fallback for dangling links. + if runtime.GOOS == "windows" { + _, err = filepath.EvalSymlinks(v2Path) + } else { + _, err = os.Stat(v2Path) + } + if err == nil { root, err := V2Path() return v2Path, root, err } else if !os.IsNotExist(err) {