From 67288047f3ebc3bc6d6d06c7ef62483173344867 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 3 Apr 2026 14:15:51 -0400 Subject: [PATCH] Use APFS cloning for file copies on macOS Add Zotero.File.copyFile(), which uses clonefile() on APFS with a fallback to IOUtils.copy(), and use it for all significant file copies. APFS is detected and cached for each parent folder via Zotero.File.isAPFS(), which uses statfs(). On APFS, all database backups now use the offline (close/clone/reopen) path instead of the SQLite online backup API. Cloning is nearly instant, and backup files share disk blocks via copy-on-write, saving potentially gigabytes of space. Closes #5330 --- chrome/content/zotero/xpcom/attachments.js | 2 +- chrome/content/zotero/xpcom/db.js | 12 ++- chrome/content/zotero/xpcom/file.js | 99 ++++++++++++++++++- .../content/zotero/xpcom/pdfWorker/manager.js | 2 +- test/tests/dbTest.js | 6 +- test/tests/fileTest.js | 43 ++++++++ 6 files changed, 157 insertions(+), 7 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 14dc4624af..7f6fa8c279 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -348,7 +348,7 @@ Zotero.Attachments = new function () { await OS.File.move(file.path, newPath); } else { - await OS.File.copy(file.path, newPath); + await Zotero.File.copyFile(file.path, newPath); } } // Copy entire parent directory (for HTML snapshots) diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js index b5ad85e3ed..4553876936 100644 --- a/chrome/content/zotero/xpcom/db.js +++ b/chrome/content/zotero/xpcom/db.js @@ -1043,13 +1043,19 @@ Zotero.DBConnection.prototype.backUpDatabase = async function ({ force, suffix, return this._offlineBackupPromise; } + // On APFS, use cloning for all backups -- it's nearly instant and backup files share + // disk blocks via copy-on-write, saving potentially gigabytes of space + if (online && Zotero.File.isAPFS(this._dbPath)) { + online = false; + } + var resolveOfflineBackupPromise; var success = false; if (online) { this._onlineBackupInProgress = true; } // For offline backups, start a promise that will be resolved when the backup finishes - else if (!online) { + else { this._offlineBackupPromise = new Promise(function () { resolveOfflineBackupPromise = arguments[0]; }); @@ -1114,7 +1120,7 @@ Zotero.DBConnection.prototype.backUpDatabase = async function ({ force, suffix, else { try { await this.closeDatabase(); - await IOUtils.copy(this._dbPath, tmpFile); + await Zotero.File.copyFile(this._dbPath, tmpFile); } catch (e) { Zotero.logError(e); @@ -1453,7 +1459,7 @@ Zotero.DBConnection.prototype._handleCorruptionMarker = async function () { // Copy backup file to main DB file this._debug("Restoring database '" + this._dbName + "' from backup file", 1); try { - await OS.File.copy(backupFile, file); + await Zotero.File.copyFile(backupFile, file); } catch (e) { // TODO: deal with low disk space diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js index 754b8b6ecc..d08d60e3b2 100644 --- a/chrome/content/zotero/xpcom/file.js +++ b/chrome/content/zotero/xpcom/file.js @@ -1086,11 +1086,108 @@ Zotero.File = new function () { return this.iterateDirectory(source, function (entry) { return entry.isDir ? this.copyDirectory(entry.path, OS.Path.join(target, entry.name)) - : OS.File.copy(entry.path, OS.Path.join(target, entry.name)); + : this.copyFile(entry.path, OS.Path.join(target, entry.name)); }.bind(this)) }; + var _isAPFSCache = {}; + + /** + * Check if a path is on an APFS volume + * + * @param {String} path + * @return {Boolean} + */ + this.isAPFS = function (path) { + if (!Zotero.isMac) return false; + + let dir = PathUtils.parent(path); + if (dir in _isAPFSCache) { + return _isAPFSCache[dir]; + } + + let result = false; + try { + let { ctypes } = ChromeUtils.importESModule( + "resource://gre/modules/ctypes.sys.mjs" + ); + // struct statfs -- f_fstypename is a char[16] at byte offset 72 + const STATFS_SIZE = 2168; + const FSTYPENAME_OFFSET = 72; + const FSTYPENAME_LEN = 16; + let buf = new ctypes.ArrayType(ctypes.uint8_t, STATFS_SIZE)(); + let lib = ctypes.open("/usr/lib/libSystem.B.dylib"); + try { + let statfs = lib.declare( + "statfs", + ctypes.default_abi, + ctypes.int, + ctypes.char.ptr, + ctypes.voidptr_t + ); + if (statfs(dir, buf.address()) === 0) { + let typeName = ''; + for (let i = FSTYPENAME_OFFSET; i < FSTYPENAME_OFFSET + FSTYPENAME_LEN; i++) { + if (buf[i] === 0) break; + typeName += String.fromCharCode(buf[i]); + } + result = typeName === 'apfs'; + } + } + finally { + lib.close(); + } + } + catch (e) { + Zotero.warn("Failed to check filesystem type: " + e); + } + + _isAPFSCache[dir] = result; + return result; + }; + + + /** + * Copy a file, using APFS cloning on macOS when available and falling back to a regular + * copy otherwise + * + * @param {String} source + * @param {String} target + */ + this.copyFile = async function (source, target) { + if (this.isAPFS(source)) { + try { + let { ctypes } = ChromeUtils.importESModule( + "resource://gre/modules/ctypes.sys.mjs" + ); + let lib = ctypes.open("/usr/lib/libSystem.B.dylib"); + try { + let clonefile = lib.declare( + "clonefile", + ctypes.default_abi, + ctypes.int, + ctypes.char.ptr, // src + ctypes.char.ptr, // dst + ctypes.uint32_t // flags + ); + let result = clonefile(source, target, 0); + if (result === 0) { + return; + } + } + finally { + lib.close(); + } + } + catch (e) { + Zotero.warn("clonefile() failed -- falling back to regular copy: " + e); + } + } + await IOUtils.copy(source, target); + }; + + this.createDirectoryIfMissing = function (dir) { dir = this.pathToFile(dir); if (!dir.exists() || !dir.isDirectory()) { diff --git a/chrome/content/zotero/xpcom/pdfWorker/manager.js b/chrome/content/zotero/xpcom/pdfWorker/manager.js index 929f55d1f8..b95c5bb57f 100644 --- a/chrome/content/zotero/xpcom/pdfWorker/manager.js +++ b/chrome/content/zotero/xpcom/pdfWorker/manager.js @@ -222,7 +222,7 @@ class PDFWorker { return 0; } if (!annotations.length) { - await OS.File.copy(attachmentPath, path); + await Zotero.File.copyFile(attachmentPath, path); return 0; } let buf = await IOUtils.read(attachmentPath); diff --git a/test/tests/dbTest.js b/test/tests/dbTest.js index 3d65a385bd..63bd7afcf0 100644 --- a/test/tests/dbTest.js +++ b/test/tests/dbTest.js @@ -431,6 +431,8 @@ describe("Zotero.DB", function () { }); it("should perform an offline backup if an online backup is already in progress", async function () { + // On APFS, online backups use cloning (offline path), so this doesn't apply + if (Zotero.File.isAPFS(Zotero.DB.path)) this.skip(); var promise = Zotero.DB.backUpDatabase({ suffix: 'test', online: true }); var result = await Zotero.DB.backUpDatabase({ suffix: 'test2' }); // The online backup fails @@ -439,8 +441,10 @@ describe("Zotero.DB", function () { assert.isTrue(result); assert.isTrue(await IOUtils.exists(bakFile2)); }); - + it("shouldn't perform an online backup if one is already in progress", async function () { + // On APFS, online backups use cloning (offline path), so this doesn't apply + if (Zotero.File.isAPFS(Zotero.DB.path)) this.skip(); var promise = Zotero.DB.backUpDatabase({ suffix: 'test', online: true }); var result = await Zotero.DB.backUpDatabase({ suffix: 'test2', online: true }); await promise; diff --git a/test/tests/fileTest.js b/test/tests/fileTest.js index 3daec2dfcb..e5d877807f 100644 --- a/test/tests/fileTest.js +++ b/test/tests/fileTest.js @@ -220,6 +220,49 @@ describe("Zotero.File", function () { }); + describe("#isAPFS()", function () { + it("should return true on macOS for the temp directory", function () { + if (!Zotero.isMac) this.skip(); + assert.isTrue(Zotero.File.isAPFS(Zotero.getTempDirectory().path)); + }); + + it("should return false on non-Mac platforms", function () { + if (Zotero.isMac) this.skip(); + assert.isFalse(Zotero.File.isAPFS(Zotero.getTempDirectory().path)); + }); + }); + + describe("#copyFile()", function () { + it("should copy a file", async function () { + let tmpDir = await getTempDirectory(); + let source = OS.Path.join(tmpDir, "source.txt"); + let target = OS.Path.join(tmpDir, "target.txt"); + await Zotero.File.putContentsAsync(source, "Hello world"); + + await Zotero.File.copyFile(source, target); + + assert.isTrue(await OS.File.exists(target)); + assert.equal(await Zotero.File.getContentsAsync(target), "Hello world"); + }); + + it("should copy a file to a different directory", async function () { + let tmpDir = await getTempDirectory(); + let subDir = OS.Path.join(tmpDir, "sub"); + await OS.File.makeDir(subDir); + let source = OS.Path.join(tmpDir, "source.txt"); + let target = OS.Path.join(subDir, "target.txt"); + await Zotero.File.putContentsAsync(source, "Hello world"); + + await Zotero.File.copyFile(source, target); + + assert.isTrue(await OS.File.exists(target)); + assert.equal(await Zotero.File.getContentsAsync(target), "Hello world"); + // Source should still exist + assert.isTrue(await OS.File.exists(source)); + }); + }); + + describe("#copyDirectory()", function () { it("should copy all files within a directory", async function () { var tmpDir = Zotero.getTempDirectory().path;