diff --git a/packages/hub/src/lib/download-file-to-cache-dir.spec.ts b/packages/hub/src/lib/download-file-to-cache-dir.spec.ts index 04ed7abd1e..29d17f2870 100644 --- a/packages/hub/src/lib/download-file-to-cache-dir.spec.ts +++ b/packages/hub/src/lib/download-file-to-cache-dir.spec.ts @@ -288,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(createSymlink).toHaveBeenCalledWith(expectedBlob, expectPointer); + expect(createSymlink).toHaveBeenCalledWith({ sourcePath: expectedBlob, finalPath: expectPointer }); }); }); diff --git a/packages/hub/src/lib/download-file-to-cache-dir.ts b/packages/hub/src/lib/download-file-to-cache-dir.ts index 4059979a8e..2c2ff44c59 100644 --- a/packages/hub/src/lib/download-file-to-cache-dir.ts +++ b/packages/hub/src/lib/download-file-to-cache-dir.ts @@ -108,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 createSymlink(blobPath, pointerPath); + await createSymlink({ sourcePath: blobPath, finalPath: pointerPath }); return pointerPath; } @@ -128,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 createSymlink(blobPath, pointerPath); + await createSymlink({ sourcePath: blobPath, finalPath: pointerPath }); return pointerPath; } diff --git a/packages/hub/src/utils/XetBlob.spec.ts b/packages/hub/src/utils/XetBlob.spec.ts index 3539ab3473..008e2294d2 100644 --- a/packages/hub/src/utils/XetBlob.spec.ts +++ b/packages/hub/src/utils/XetBlob.spec.ts @@ -98,7 +98,7 @@ describe("XetBlob", () => { } return fetch(url, opts); }, - internalLogging: true, + // internalLogging: true, }); const xetDownload = await blob.slice(0, 200_000).arrayBuffer(); @@ -124,7 +124,7 @@ describe("XetBlob", () => { }, hash: "7b3b6d07673a88cf467e67c1f7edef1a8c268cbf66e9dd9b0366322d4ab56d9b", size: 5_234_139_343, - internalLogging: true, + // internalLogging: true, }); const xetDownload = await blob.slice(10_000_000, 10_100_000).arrayBuffer(); diff --git a/packages/hub/src/utils/symlink.spec.ts b/packages/hub/src/utils/symlink.spec.ts index be2ad91d5c..56edaf4f33 100644 --- a/packages/hub/src/utils/symlink.spec.ts +++ b/packages/hub/src/utils/symlink.spec.ts @@ -1,63 +1,89 @@ -import { describe, test, expect, vi, beforeEach } from "vitest"; -import * as fs from "node:fs/promises"; +/* eslint-disable @typescript-eslint/consistent-type-imports */ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, expect, it, vi } from "vitest"; 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(), -})); +import { readFileSync, writeFileSync } from "node:fs"; +import { lstat, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +let failSymlink = false; +vi.mock("node:fs/promises", async (importOriginal) => ({ + ...(await importOriginal()), + symlink: async (...args: any[]) => { + if (failSymlink) { + failSymlink = false; + throw new Error("Symlink not supported"); + } -vi.mock("node:os", () => ({ - platform: vi.fn(), + // @ts-expect-error - ignore + return (await importOriginal()).symlink(...args); + }, })); -describe("createSymlink", () => { - const src = "/path/to/src"; - const dst = "/path/to/dst"; +describe("utils/symlink", () => { + it("should create a symlink", async () => { + writeFileSync(join(tmpdir(), "test.txt"), "hello world"); + await createSymlink({ + sourcePath: join(tmpdir(), "test.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), + }); - beforeEach(() => { - vi.resetAllMocks(); - vi.mocked(fs.mkdtemp).mockImplementation(async (prefix) => `${prefix}/temp`); - vi.mocked(fs.open).mockResolvedValue({ close: vi.fn() } as unknown as FileHandle); - }); + const stats = await lstat(join(tmpdir(), "test-symlink.txt")); + expect(stats.isSymbolicLink()).toBe(process.platform !== "win32"); - test("should remove existing destination", async () => { - await createSymlink(dst, src); - expect(fs.rm).toHaveBeenCalledWith(dst); + // Test file content + const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); + expect(content).toBe("hello world"); + + // Cleanup + await rm(join(tmpdir(), "test-symlink.txt")); + await rm(join(tmpdir(), "test.txt")); }); - describe("symlink not supported (Windows)", () => { - beforeEach(() => { - vi.mocked(fs.symlink).mockRejectedValue(new Error("Symlink not supported")); + it("should work when symlinking twice", async () => { + writeFileSync(join(tmpdir(), "test.txt"), "hello world"); + writeFileSync(join(tmpdir(), "test2.txt"), "hello world2"); + await createSymlink({ + sourcePath: join(tmpdir(), "test.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); - - test("should copyfile", async () => { - await createSymlink(dst, src); - expect(fs.copyFile).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); + await createSymlink({ + sourcePath: join(tmpdir(), "test2.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); - test("should rename file if new_blob is true", async () => { - await createSymlink(dst, src, true); - expect(fs.rename).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); - }); + const stats = await lstat(join(tmpdir(), "test-symlink.txt")); + expect(stats.isSymbolicLink()).toBe(process.platform !== "win32"); + + // Test file content + const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); + expect(content).toBe("hello world2"); + + // Cleanup + await rm(join(tmpdir(), "test-symlink.txt")); + await rm(join(tmpdir(), "test.txt")); + await rm(join(tmpdir(), "test2.txt")); }); - describe("symlink supported", () => { - test("should symlink", async () => { - await createSymlink(dst, src); - expect(fs.symlink).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); - }); + it("should work when symlink doesn't work (windows)", async () => { + writeFileSync(join(tmpdir(), "test.txt"), "hello world"); - test("should symlink if new_blob is true", async () => { - await createSymlink(dst, src, true); - expect(fs.symlink).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); + failSymlink = true; + await createSymlink({ + sourcePath: join(tmpdir(), "test.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); + + const stats = await lstat(join(tmpdir(), "test-symlink.txt")); + expect(stats.isSymbolicLink()).toBe(false); + + // Test file content + const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); + expect(content).toBe("hello world"); + + // Cleanup + await rm(join(tmpdir(), "test-symlink.txt")); + await rm(join(tmpdir(), "test.txt")); }); }); diff --git a/packages/hub/src/utils/symlink.ts b/packages/hub/src/utils/symlink.ts index 239b73cd18..17cedf66ad 100644 --- a/packages/hub/src/utils/symlink.ts +++ b/packages/hub/src/utils/symlink.ts @@ -36,28 +36,30 @@ function expandUser(path: string): string { * 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 { +export async function createSymlink(params: { + /** + * The path to the symlink. + */ + finalPath: string; + /** + * The path the symlink should point to. + */ + sourcePath: string; +}): Promise { + const abs_src = path.resolve(expandUser(params.sourcePath)); + const abs_dst = path.resolve(expandUser(params.finalPath)); + try { - await fs.rm(dst); - } catch (_e: unknown) { - /* empty */ + await fs.rm(abs_dst); + } catch { + // ignore } - 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_dst, abs_src); - } else { - console.info(`Symlink not supported. Copying file from ${abs_src} to ${abs_dst}`); - await fs.copyFile(abs_dst, abs_src); - } + await fs.symlink(path.relative(path.dirname(abs_dst), abs_src), abs_dst); + } catch { + console.info(`Symlink not supported. Copying file from ${abs_src} to ${abs_dst}`); + await fs.copyFile(abs_src, abs_dst); } } diff --git a/packages/hub/vitest.config.mts b/packages/hub/vitest.config.mts index 88747caaed..cc84b9c512 100644 --- a/packages/hub/vitest.config.mts +++ b/packages/hub/vitest.config.mts @@ -2,6 +2,6 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { - testTimeout: 30_000, + testTimeout: 60_000, }, });