fix: better handling of symlink (Windows,...) (#1272)

Add finer handling of symlink as it is not supported on Windows.

Fixes #1268

---------

Signed-off-by: Jeff MAURY <[email protected]>
Co-authored-by: coyotte508 <[email protected]>

Author: jeffmaury

packages/hub/src/lib/download-file-to-cache-dir.spec.ts

@@ -7,6 +7,7 @@ import type { Stats } from "node:fs";
 import { getHFHubCachePath, getRepoFolderName } from "./cache-management";
 import { toRepoId } from "../utils/toRepoId";
 import { downloadFileToCacheDir } from "./download-file-to-cache-dir";
+import { createSymlink } from "../utils/symlink";
 
 vi.mock("node:fs/promises", () => ({
 	writeFile: vi.fn(),
@@ -21,6 +22,10 @@ vi.mock("./paths-info", () => ({
 	pathsInfo: vi.fn(),
 }));
 
+vi.mock("../utils/symlink", () => ({
+	createSymlink: vi.fn(),
+}));
+
 const DUMMY_REPO: RepoId = {
 	name: "hello-world",
 	type: "model",
@@ -196,7 +201,7 @@ describe("downloadFileToCacheDir", () => {
 		expect(vi.mocked(lstat).mock.calls[0][0]).toBe(expectedBlob);
 
 		// symlink should have been created
-		expect(symlink).toHaveBeenCalledOnce();
+		expect(createSymlink).toHaveBeenCalledOnce();
 		// no download done
 		expect(fetchMock).not.toHaveBeenCalled();
 
@@ -283,6 +288,6 @@ describe("downloadFileToCacheDir", () => {
 		// 2. should rename the incomplete to the blob expected name
 		expect(rename).toHaveBeenCalledWith(incomplete, expectedBlob);
 		// 3. should create symlink pointing to blob
-		expect(symlink).toHaveBeenCalledWith(expectedBlob, expectPointer);
+		expect(createSymlink).toHaveBeenCalledWith(expectedBlob, expectPointer);
 	});
 });

packages/hub/src/lib/download-file-to-cache-dir.ts

@@ -1,11 +1,12 @@
 import { getHFHubCachePath, getRepoFolderName } from "./cache-management";
 import { dirname, join } from "node:path";
-import { writeFile, rename, symlink, lstat, mkdir, stat } from "node:fs/promises";
+import { writeFile, rename, lstat, mkdir, stat } from "node:fs/promises";
 import type { CommitInfo, PathInfo } from "./paths-info";
 import { pathsInfo } from "./paths-info";
 import type { CredentialsParams, RepoDesignation } from "../types/public";
 import { toRepoId } from "../utils/toRepoId";
 import { downloadFile } from "./download-file";
+import { createSymlink } from "../utils/symlink";
 
 export const REGEX_COMMIT_HASH: RegExp = new RegExp("^[0-9a-f]{40}$");
 
@@ -107,7 +108,7 @@ export async function downloadFileToCacheDir(
 	// shortcut the download if needed
 	if (await exists(blobPath)) {
 		// create symlinks in snapshot folder to blob object
-		await symlink(blobPath, pointerPath);
+		await createSymlink(blobPath, pointerPath);
 		return pointerPath;
 	}
 
@@ -127,6 +128,6 @@ export async function downloadFileToCacheDir(
 	// rename .incomplete file to expect blob
 	await rename(incomplete, blobPath);
 	// create symlinks in snapshot folder to blob object
-	await symlink(blobPath, pointerPath);
+	await createSymlink(blobPath, pointerPath);
 	return pointerPath;
 }

packages/hub/src/utils/symlink.spec.ts

@@ -0,0 +1,63 @@
+import { describe, test, expect, vi, beforeEach } from "vitest";
+import * as fs from "node:fs/promises";
+import { createSymlink } from "./symlink";
+import * as path from "node:path";
+import type { FileHandle } from "node:fs/promises";
+
+vi.mock("node:fs/promises", () => ({
+	rm: vi.fn(),
+	symlink: vi.fn(),
+	rename: vi.fn(),
+	copyFile: vi.fn(),
+	mkdir: vi.fn(),
+	mkdtemp: vi.fn(),
+	open: vi.fn(),
+}));
+
+vi.mock("node:os", () => ({
+	platform: vi.fn(),
+}));
+
+describe("createSymlink", () => {
+	const src = "/path/to/src";
+	const dst = "/path/to/dst";
+
+	beforeEach(() => {
+		vi.resetAllMocks();
+		vi.mocked(fs.mkdtemp).mockImplementation(async (prefix) => `${prefix}/temp`);
+		vi.mocked(fs.open).mockResolvedValue({ close: vi.fn() } as unknown as FileHandle);
+	});
+
+	test("should remove existing destination", async () => {
+		await createSymlink(dst, src);
+		expect(fs.rm).toHaveBeenCalledWith(dst);
+	});
+
+	describe("symlink not supported (Windows)", () => {
+		beforeEach(() => {
+			vi.mocked(fs.symlink).mockRejectedValue(new Error("Symlink not supported"));
+		});
+
+		test("should copyfile", async () => {
+			await createSymlink(dst, src);
+			expect(fs.copyFile).toHaveBeenCalledWith(path.resolve(src), path.resolve(dst));
+		});
+
+		test("should rename file if new_blob is true", async () => {
+			await createSymlink(dst, src, true);
+			expect(fs.rename).toHaveBeenCalledWith(path.resolve(src), path.resolve(dst));
+		});
+	});
+
+	describe("symlink supported", () => {
+		test("should symlink", async () => {
+			await createSymlink(dst, src);
+			expect(fs.symlink).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src));
+		});
+
+		test("should symlink if new_blob is true", async () => {
+			await createSymlink(dst, src, true);
+			expect(fs.symlink).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src));
+		});
+	});
+});

packages/hub/src/utils/symlink.ts

@@ -0,0 +1,63 @@
+/**
+ * Heavily inspired by https://github.com/huggingface/huggingface_hub/blob/fcfd14361bd03f23f82efced1aa65a7cbfa4b922/src/huggingface_hub/file_download.py#L517
+ */
+
+import * as fs from "node:fs/promises";
+import * as path from "node:path";
+import * as os from "node:os";
+
+function expandUser(path: string): string {
+	if (path.startsWith("~")) {
+		return path.replace("~", os.homedir());
+	}
+	return path;
+}
+
+/**
+ * Create a symbolic link named dst pointing to src.
+ *
+ * By default, it will try to create a symlink using a relative path. Relative paths have 2 advantages:
+ * - If the cache_folder is moved (example: back-up on a shared drive), relative paths within the cache folder will
+ *  not break.
+ * - Relative paths seems to be better handled on Windows. Issue was reported 3 times in less than a week when
+ *   changing from relative to absolute paths. See https://github.com/huggingface/huggingface_hub/issues/1398,
+ *   https://github.com/huggingface/diffusers/issues/2729 and https://github.com/huggingface/transformers/pull/22228.
+ *   NOTE: The issue with absolute paths doesn't happen on admin mode.
+ * When creating a symlink from the cache to a local folder, it is possible that a relative path cannot be created.
+ * This happens when paths are not on the same volume. In that case, we use absolute paths.
+ *
+ * The result layout looks something like
+ *     └── [ 128]  snapshots
+ *         ├── [ 128]  2439f60ef33a0d46d85da5001d52aeda5b00ce9f
+ *         │   ├── [  52]  README.md -> ../../../blobs/d7edf6bd2a681fb0175f7735299831ee1b22b812
+ *         │   └── [  76]  pytorch_model.bin -> ../../../blobs/403450e234d65943a7dcf7e05a771ce3c92faa84dd07db4ac20f592037a1e4bd
+ *
+ * If symlinks cannot be created on this platform (most likely to be Windows), the workaround is to avoid symlinks by
+ * having the actual file in `dst`. If it is a new file (`new_blob=True`), we move it to `dst`. If it is not a new file
+ * (`new_blob=False`), we don't know if the blob file is already referenced elsewhere. To avoid breaking existing
+ * cache, the file is duplicated on the disk.
+ *
+ * In case symlinks are not supported, a warning message is displayed to the user once when loading `huggingface_hub`.
+ * The warning message can be disabled with the `HF_HUB_DISABLE_SYMLINKS_WARNING` environment variable.
+ */
+export async function createSymlink(dst: string, src: string, new_blob?: boolean): Promise<void> {
+	try {
+		await fs.rm(dst);
+	} catch (_e: unknown) {
+		/* empty */
+	}
+	const abs_src = path.resolve(expandUser(src));
+	const abs_dst = path.resolve(expandUser(dst));
+
+	try {
+		await fs.symlink(abs_dst, abs_src);
+	} catch (_e: unknown) {
+		if (new_blob) {
+			console.info(`Symlink not supported. Moving file from ${abs_src} to ${abs_dst}`);
+			await fs.rename(abs_src, abs_dst);
+		} else {
+			console.info(`Symlink not supported. Copying file from ${abs_src} to ${abs_dst}`);
+			await fs.copyFile(abs_src, abs_dst);
+		}
+	}
+}

packages/hub/vitest-browser.config.mts

@@ -5,6 +5,7 @@ export default defineConfig({
 		exclude: [
 			...configDefaults.exclude,
 			"src/utils/FileBlob.spec.ts",
+			"src/utils/symlink.spec.ts",
 			"src/lib/cache-management.spec.ts",
 			"src/lib/download-file-to-cache-dir.spec.ts",
 			"src/lib/snapshot-download.spec.ts",