From a17b486e4b39c6d6ce5c7e177ecd9655f03191cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adomas=20Ven=C4=8Dkauskas?= Date: Mon, 12 Jun 2017 14:14:36 +0300 Subject: [PATCH] Don't auto-migrate data dir if target on a different drive See https://forums.zotero.org/discussion/comment/277632/#Comment_277632 --- chrome/content/zotero/xpcom/dataDirectory.js | 103 ++++++++++++++----- chrome/locale/en-US/zotero/zotero.properties | 1 + test/tests/dataDirectoryTest.js | 35 ++++++- 3 files changed, 108 insertions(+), 31 deletions(-) diff --git a/chrome/content/zotero/xpcom/dataDirectory.js b/chrome/content/zotero/xpcom/dataDirectory.js index 82a38c654..988bf2b3c 100644 --- a/chrome/content/zotero/xpcom/dataDirectory.js +++ b/chrome/content/zotero/xpcom/dataDirectory.js @@ -428,6 +428,36 @@ Zotero.DataDirectory = { }, + isNewDirOnDifferentDrive: Zotero.Promise.coroutine(function* (oldDir, newDir) { + yield this.markForMigration(oldDir, true); + let oldMarkerFile = OS.Path.join(oldDir, this.MIGRATION_MARKER); + let testPath = OS.Path.join(newDir, '..', this.MIGRATION_MARKER); + try { + // Attempt moving the marker with noCopy + yield OS.File.move(oldMarkerFile, testPath, {noCopy: true}); + } catch(e) { + yield OS.File.remove(oldMarkerFile); + + Components.classes["@mozilla.org/net/osfileconstantsservice;1"]. + getService(Components.interfaces.nsIOSFileConstantsService). + init(); + if (e instanceof OS.File.Error) { + if (typeof e.unixErrno != "undefined") { + return e.unixErrno == OS.Constants.libc.EXDEV; + } + if (typeof e.winLastError != "undefined") { + // ERROR_NOT_SAME_DEVICE is undefined + // return e.winLastError == OS.Constants.Win.ERROR_NOT_SAME_DEVICE; + return e.winLastError == 17; + } + } + throw e; + } + yield OS.File.remove(testPath); + return false; + }), + + /** * Determine if current data directory is in a legacy location */ @@ -438,6 +468,10 @@ Zotero.DataDirectory = { return false; } + if (this.newDirOnDifferentDrive) { + return false; + } + // Legacy default or set to legacy default from other program (Standalone/Z4Fx) to share data if (!Zotero.Prefs.get('useDataDir') || this.isLegacy(currentDir)) { return true; @@ -487,12 +521,24 @@ Zotero.DataDirectory = { automatic = true; // Skip automatic migration if there's a non-empty directory at the new location + // TODO: Notify user if ((yield OS.File.exists(newDir)) && !(yield Zotero.File.directoryIsEmpty(newDir))) { Zotero.debug(`${newDir} exists and is non-empty -- skipping migration`); return false; } } + // Skip migration if new dir on different drive and prompt + if (yield this.isNewDirOnDifferentDrive(dataDir, newDir)) { + Zotero.debug(`New dataDir ${newDir} is on a different drive from ${dataDir} -- skipping migration`); + Zotero.DataDirectory.newDirOnDifferentDrive = true; + + let error = Zotero.getString(`dataDir.migration.failure.full.automatic.newDirOnDifferentDrive`, Zotero.clientName) + + "\n\n" + + Zotero.getString(`dataDir.migration.failure.full.automatic.text2`, Zotero.appName); + return this.fullMigrationFailurePrompt(dataDir, newDir, error); + } + // Check for an existing pipe from other running versions of Zotero pointing at the same data // directory, and skip migration if found try { @@ -593,7 +639,7 @@ Zotero.DataDirectory = { false, null, // Don't show message in a popup in Standalone if pane isn't ready - Zotero.iStandalone + Zotero.isStandalone ); } catch (e) { @@ -607,31 +653,13 @@ Zotero.DataDirectory = { Zotero.debug("Migration failed", 1); Zotero.logError(e); - let ps = Services.prompt; - let buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) - + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING); - let index = ps.confirmEx(null, - Zotero.getString('dataDir.migration.failure.title'), - Zotero.getString(`dataDir.migration.failure.full.${mode}.text1`, ZOTERO_CONFIG.CLIENT_NAME) - + "\n\n" - + e - + "\n\n" - + Zotero.getString(`dataDir.migration.failure.full.${mode}.text2`, Zotero.appName) - + "\n\n" - + Zotero.getString('dataDir.migration.failure.full.current', oldDir) - + "\n\n" - + Zotero.getString('dataDir.migration.failure.full.recommended', newDir), - buttonFlags, - Zotero.getString('dataDir.migration.failure.full.showCurrentDirectoryAndQuit', Zotero.appName), - Zotero.getString('general.notNow'), - null, null, {} - ); - if (index == 0) { - yield Zotero.File.reveal(oldDir); - Zotero.skipLoading = true; - Zotero.Utilities.Internal.quitZotero(); - } - return; + let error = Zotero.getString(`dataDir.migration.failure.full.${mode}.text1`, Zotero.clientName) + + "\n\n" + + e; + + "\n\n" + + Zotero.getString(`dataDir.migration.failure.full.${mode}.text2`, Zotero.appName); + + return this.fullMigrationFailurePrompt(oldDir, newDir, error); } // Set data directory again @@ -700,6 +728,29 @@ Zotero.DataDirectory = { }), + fullMigrationFailurePrompt: Zotero.Promise.coroutine(function* (oldDir, newDir, error) { + let ps = Services.prompt; + let buttonFlags = (ps.BUTTON_POS_0) * (ps.BUTTON_TITLE_IS_STRING) + + (ps.BUTTON_POS_1) * (ps.BUTTON_TITLE_IS_STRING); + let index = ps.confirmEx(null, + Zotero.getString('dataDir.migration.failure.title'), + error + "\n\n" + + Zotero.getString('dataDir.migration.failure.full.current', oldDir) + + "\n\n" + + Zotero.getString('dataDir.migration.failure.full.recommended', newDir), + buttonFlags, + Zotero.getString('dataDir.migration.failure.full.showCurrentDirectoryAndQuit', Zotero.appName), + Zotero.getString('general.notNow'), + null, null, {} + ); + if (index == 0) { + yield Zotero.File.reveal(oldDir); + Zotero.skipLoading = true; + Zotero.Utilities.Internal.quitZotero(); + } + }), + + /** * Recursively moves data directory from one location to another and updates the data directory * setting in this profile and any profiles pointing to the old location diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index ca969cbff..ba37af3e8 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -146,6 +146,7 @@ dataDir.migration.failure.partial.new = New directory: %S dataDir.migration.failure.partial.showDirectoriesAndQuit = Show Directories and Quit dataDir.migration.failure.full.automatic.text1 = %S attempted to move your data directory to a new default location, but the migration could not be completed. dataDir.migration.failure.full.automatic.text2 = It is recommended that you close %S and move your data directory manually. +dataDir.migration.failure.full.automatic.newDirOnDifferentDrive = %S attempted to move your data directory to a new default location, but the old directory is on a different drive and cannot be migrated automatically. dataDir.migration.failure.full.manual.text1 = Your %S data directory could not be migrated. dataDir.migration.failure.full.manual.text2 = It is recommended that you close %S and manually move your data directory to the new default location. dataDir.migration.failure.full.firefoxOpen = Your data directory cannot be migrated while Zotero for Firefox is open. Please close Firefox and try again. diff --git a/test/tests/dataDirectoryTest.js b/test/tests/dataDirectoryTest.js index a7166e6c5..6c226982c 100644 --- a/test/tests/dataDirectoryTest.js +++ b/test/tests/dataDirectoryTest.js @@ -39,10 +39,12 @@ describe("Zotero.DataDirectory", function () { stubs.canMigrate = sinon.stub(Zotero.DataDirectory, "canMigrate").returns(true); // A pipe always exists during tests, since Zotero is running stubs.pipeExists = sinon.stub(Zotero.IPC, "pipeExists").returns(Zotero.Promise.resolve(false)); + stubs.setDataDir = sinon.stub(Zotero.DataDirectory, "set"); + stubs.isNewDirOnDifferentDrive = sinon.stub(Zotero.DataDirectory, 'isNewDirOnDifferentDrive').resolves(true); }); beforeEach(function* () { - stubs.setDataDir = sinon.stub(Zotero.DataDirectory, "set"); + stubs.setDataDir.reset(); }); afterEach(function* () { @@ -50,13 +52,14 @@ describe("Zotero.DataDirectory", function () { yield removeDir(newDir); Zotero.DataDirectory._cache(false); yield Zotero.DataDirectory.init(); - - stubs.setDataDir.restore(); }); after(function* () { - stubs.canMigrate.restore(); - stubs.pipeExists.restore(); + for (let key in stubs) { + try { + stubs[key].restore(); + } catch(e) {} + } }); // Force non-mv mode @@ -183,6 +186,28 @@ describe("Zotero.DataDirectory", function () { yield assert.eventually.isFalse(Zotero.DataDirectory.checkForMigration(oldDir, newDir)); }); + it("should skip automatic migration and show prompt if target directory is on a different drive", function* () { + resetCommandMode(); + resetFunctionMode(); + + yield populateDataDirectory(oldDir); + yield OS.File.remove(oldMigrationMarker); + + stubs.isNewDirOnDifferentDrive.resolves(true); + + var promise = waitForDialog(function (dialog) { + assert.include( + dialog.document.documentElement.textContent, + Zotero.getString(`dataDir.migration.failure.full.automatic.newDirOnDifferentDrive`, Zotero.clientName) + ); + }, 'cancel'); + + yield assert.eventually.isNotOk(Zotero.DataDirectory.checkForMigration(oldDir, newDir)); + yield promise; + + stubs.isNewDirOnDifferentDrive.resolves(false); + }); + add("should show error on partial failure", function (automatic) { return function* () { yield populateDataDirectory(oldDir, null, automatic);