From c5a711af0f9cf725d6167bb363f788005fc6f5ff Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Sun, 2 Apr 2017 21:34:05 -0400 Subject: [PATCH] refactor(CLI): make use of `mountutils` to unmount drives (#1209) `mountutils` is a native C++ NodeJS addon the Etcher team has been working on to solve several issues we've been having with third party unmount programs. We'll see how this little module behaves after some real world usage. I'm confident that it will fix the issues linked in this commit. This commit also upgrades `npm` to 4.4.4 in Appveyor, given there is a known building issue on Windows that is solved in a recent version. Change-Type: minor Changelog-Entry: Fix several unmount related issues in all platforms. See: https://github.com/resin-io-modules/mountutils Fixes: https://github.com/resin-io/etcher/issues/1177 Fixes: https://github.com/resin-io/etcher/issues/985 Fixes: https://github.com/resin-io/etcher/issues/750 Signed-off-by: Juan Cruz Viotti --- appveyor.yml | 1 + lib/cli/unmount.js | 108 -------------------------------------- lib/cli/writer.js | 6 +-- npm-shrinkwrap.json | 17 ++++++ package.json | 6 +-- tests/cli/unmount.spec.js | 64 ---------------------- 6 files changed, 24 insertions(+), 178 deletions(-) delete mode 100644 lib/cli/unmount.js delete mode 100644 tests/cli/unmount.spec.js diff --git a/appveyor.yml b/appveyor.yml index 8dc7794f..805ed689 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -21,6 +21,7 @@ environment: install: - ps: Install-Product node $env:nodejs_version x64 + - npm install -g npm@4.4.4 - npm install -g bower rimraf asar - choco install nsis -version 2.51 - choco install jq diff --git a/lib/cli/unmount.js b/lib/cli/unmount.js deleted file mode 100644 index 9131e149..00000000 --- a/lib/cli/unmount.js +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2016 resin.io - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -const _ = require('lodash'); -const Bluebird = require('bluebird'); -const childProcess = Bluebird.promisifyAll(require('child_process')); -const os = require('os'); - -/** - * @summary Unmount command templates - * @namespace COMMAND_TEMPLATES - * - * We make sure that the commands declared here exit - * successfully even if the drive is not mounted. - */ -const COMMAND_TEMPLATES = { - - /** - * @property {String} darwin - * @memberof COMMAND_TEMPLATES - * @private - */ - darwin: '/usr/sbin/diskutil unmountDisk force <%= device %>', - - /** - * @property {String} linux - * @memberof COMMAND_TEMPLATES - * @private - * - * @description - * If trying to unmount the raw device in Linux, we get: - * > umount: /dev/sdN: not mounted - * Therefore we use the ?* glob to make sure umount processes - * the partitions of sdN independently (even if they contain multiple digits) - * but not the raw device. - * We also redirect stderr to /dev/null to ignore warnings - * if a device is already unmounted. - * Finally, we also wrap the command in a boolean expression - * that always evaluates to true to ignore the return code, - * which is non zero when a device was already unmounted. - */ - linux: 'umount <%= device %>?* 2>/dev/null || /bin/true' - -}; - -/** - * @summary Get UNIX unmount command - * @function - * @public - * - * @param {String} operatingSystem - operating system slug - * @param {Object} drive - drive object - * @returns {String} command - * - * @example - * const drivelist = require('drivelist'); - * const os = require('os'); - * - * drivelist.list((drives) => { - * const command = unmount.getUNIXUnmountCommand(os.platform(), drives[0]); - * }); - */ -exports.getUNIXUnmountCommand = (operatingSystem, drive) => { - return _.template(COMMAND_TEMPLATES[operatingSystem])(drive); -}; - -/** - * @summary Unmount drive - * @function - * @public - * - * @param {Object} drive - drive object - * @returns {Promise} - * - * @example - * const Bluebird = require('bluebird'); - * const drivelist = Bluebird.promisifyAll(require('drivelist')); - * - * drivelist.listAsync().each(unmount.unmountDrive); - */ -exports.unmountDrive = (drive) => { - const platform = os.platform(); - - if (platform === 'win32') { - const removedrive = Bluebird.promisifyAll(require('removedrive')); - return Bluebird.each(drive.mountpoints, (mountpoint) => { - return removedrive.ejectAsync(mountpoint.path); - }); - } - - const command = exports.getUNIXUnmountCommand(platform, drive); - return childProcess.execAsync(command); -}; diff --git a/lib/cli/writer.js b/lib/cli/writer.js index 6fa4e7d8..eca53cfd 100644 --- a/lib/cli/writer.js +++ b/lib/cli/writer.js @@ -19,8 +19,8 @@ const imageWrite = require('etcher-image-write'); const Bluebird = require('bluebird'); const fs = Bluebird.promisifyAll(require('fs')); +const mountutils = Bluebird.promisifyAll(require('mountutils')); const os = require('os'); -const unmount = require('./unmount'); const imageStream = require('../image-stream'); const errors = require('../shared/errors'); const constraints = require('../shared/drive-constraints'); @@ -64,7 +64,7 @@ exports.writeImage = (imagePath, drive, options, onProgress) => { return Bluebird.resolve(); } - return unmount.unmountDrive(drive); + return mountutils.unmountDiskAsync(drive.device); }).then(() => { return fs.openAsync(drive.raw, 'rs+'); }).then((driveFileDescriptor) => { @@ -110,7 +110,7 @@ exports.writeImage = (imagePath, drive, options, onProgress) => { return Bluebird.resolve(); } - return unmount.unmountDrive(drive); + return mountutils.unmountDiskAsync(drive.device); }); }); }); diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index b5ce339c..d998e33d 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -445,6 +445,11 @@ "resolved": "https://registry.npmjs.org/binary/-/binary-0.3.0.tgz", "dev": true }, + "bindings": { + "version": "1.2.1", + "from": "bindings@>=1.2.1 <2.0.0", + "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.2.1.tgz" + }, "bl": { "version": "0.9.5", "from": "bl@>=0.9.0 <0.10.0", @@ -4791,6 +4796,18 @@ "from": "moment-duration-format@>=1.3.0 <2.0.0", "resolved": "https://registry.npmjs.org/moment-duration-format/-/moment-duration-format-1.3.0.tgz" }, + "mountutils": { + "version": "1.0.2", + "from": "mountutils@1.0.2", + "resolved": "https://registry.npmjs.org/mountutils/-/mountutils-1.0.2.tgz", + "dependencies": { + "nan": { + "version": "2.5.1", + "from": "nan@>=2.5.1 <3.0.0", + "resolved": "https://registry.npmjs.org/nan/-/nan-2.5.1.tgz" + } + } + }, "ms": { "version": "0.7.2", "from": "ms@0.7.2", diff --git a/package.json b/package.json index c975cddd..6fe7ef0e 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "url": "git@github.com:resin-io/etcher.git" }, "scripts": { - "test": "npm run lint && electron-mocha --recursive --renderer tests/gui -R spec && electron-mocha --recursive tests/cli tests/shared tests/child-writer tests/image-stream -R spec", + "test": "npm run lint && electron-mocha --recursive --renderer tests/gui -R spec && electron-mocha --recursive tests/shared tests/child-writer tests/image-stream -R spec", "sass": "node-sass ./lib/gui/scss/main.scss > ./lib/gui/css/main.css", "jslint": "eslint lib tests scripts bin versionist.conf.js", "sasslint": "sass-lint lib/gui/scss", @@ -58,8 +58,7 @@ } }, "optionalDependencies": { - "elevator": "^2.1.0", - "removedrive": "^1.1.1" + "elevator": "^2.1.0" }, "dependencies": { "angular": "1.6.3", @@ -87,6 +86,7 @@ "lodash": "^4.5.1", "lodash-deep": "^2.0.0", "lzma-native": "^1.5.2", + "mountutils": "^1.0.2", "node-ipc": "^8.9.2", "node-stream-zip": "^1.3.4", "path-is-inside": "^1.0.2", diff --git a/tests/cli/unmount.spec.js b/tests/cli/unmount.spec.js deleted file mode 100644 index c02065e2..00000000 --- a/tests/cli/unmount.spec.js +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2016 resin.io - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -const m = require('mochainon'); -const unmount = require('../../lib/cli/unmount'); - -describe('CLI: Unmount', function() { - - describe('.getUNIXUnmountCommand()', function() { - - it('should return the correct command for OS X', function() { - const command = unmount.getUNIXUnmountCommand('darwin', { - device: '/dev/disk2', - description: 'DataTraveler 2.0', - size: 7823458304, - mountpoints: [ - { - path: '/Volumes/UNTITLED' - } - ], - raw: '/dev/rdisk2', - protected: false, - system: false - }); - - m.chai.expect(command).to.equal('/usr/sbin/diskutil unmountDisk force /dev/disk2'); - }); - - it('should return the correct command for GNU/Linux', function() { - const command = unmount.getUNIXUnmountCommand('linux', { - device: '/dev/sda', - description: 'DataTraveler 2.0', - size: 7823458304, - mountpoints: [ - { - path: '/media/UNTITLED' - } - ], - raw: '/dev/sda', - protected: false, - system: false - }); - - m.chai.expect(command).to.equal('umount /dev/sda?* 2>/dev/null || /bin/true'); - }); - - }); - -});