From da81849d4b596c2a7cdc89d6c4aa99c4fdc31c63 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Wed, 24 Aug 2016 10:19:18 -0400 Subject: [PATCH] refactor: rely on `etcher-image-stream` to fetch bmap contents (#654) In order to get the bmap file contents to the Etcher CLI, we were handling extraction, writing to a temporary file, then reading again, and all sorts of other mumbo-jumbo, without realising that `etcher-image-stream` already has this information right where we need it (in the CLI's writer module) and in the way we need it (as plain text). Re-using from there hugely simplifies things. Signed-off-by: Juan Cruz Viotti --- docs/CLI.md | 1 - lib/cli/cli.js | 5 ----- lib/cli/etcher.js | 19 +++--------------- lib/cli/writer.js | 3 +-- lib/gui/models/selection-state.js | 14 ------------- lib/gui/modules/image-writer.js | 3 +-- lib/src/child-writer/index.js | 21 +++----------------- lib/src/child-writer/utils.js | 25 ------------------------ tests/gui/models/selection-state.spec.js | 16 +-------------- 9 files changed, 9 insertions(+), 98 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index e73d6771..480914e6 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -39,7 +39,6 @@ Options --check, -c validate write --robot, -r parse-able output without interactivity --log, -l output log file - --bmap, -b bmap file --yes, -y confirm non-interactively --unmount, -u unmount on success ``` diff --git a/lib/cli/cli.js b/lib/cli/cli.js index 4620d19b..6d7d7a1a 100644 --- a/lib/cli/cli.js +++ b/lib/cli/cli.js @@ -120,11 +120,6 @@ module.exports = yargs string: true, alias: 'l' }, - bmap: { - describe: 'bmap file', - string: true, - alias: 'b' - }, yes: { describe: 'confirm non-interactively', boolean: true, diff --git a/lib/cli/etcher.js b/lib/cli/etcher.js index 75ff5a28..1752a542 100644 --- a/lib/cli/etcher.js +++ b/lib/cli/etcher.js @@ -18,7 +18,6 @@ const _ = require('lodash'); const Bluebird = require('bluebird'); -const fs = Bluebird.promisifyAll(require('fs')); const visuals = require('resin-cli-visuals'); const form = require('resin-cli-form'); const drivelist = Bluebird.promisifyAll(require('drivelist')); @@ -60,19 +59,8 @@ form.run([ check: new visuals.Progress('Validating') }; - return Bluebird.props({ - drives: drivelist.listAsync(), - bmap: _.attempt(() => { - if (!options.bmap) { - return; - } - - return fs.readFileAsync(options.bmap, { - encoding: 'utf8' - }); - }) - }).then((results) => { - const selectedDrive = _.find(results.drives, { + return drivelist.listAsync().then((drives) => { + const selectedDrive = _.find(drives, { device: answers.drive }); @@ -82,8 +70,7 @@ form.run([ return writer.writeImage(options._[0], selectedDrive, { unmountOnSuccess: options.unmount, - validateWriteOnSuccess: options.check, - bmapContents: results.bmap + validateWriteOnSuccess: options.check }, (state) => { if (options.robot) { diff --git a/lib/cli/writer.js b/lib/cli/writer.js index f56f85ef..217a9127 100644 --- a/lib/cli/writer.js +++ b/lib/cli/writer.js @@ -38,7 +38,6 @@ const isWindows = os.platform() === 'win32'; * @param {Object} options - options * @param {Boolean} [options.unmountOnSuccess=false] - unmount on success * @param {Boolean} [options.validateWriteOnSuccess=false] - validate write on success - * @param {String} [options.bmapContents] - bmap file contents * @param {Function} onProgress - on progress callback (state) * * @fulfil {Boolean} - whether the operation was successful @@ -70,7 +69,7 @@ exports.writeImage = (imagePath, drive, options, onProgress) => { }, results.image, { check: options.validateWriteOnSuccess, transform: results.image.transform, - bmap: options.bmapContents + bmap: results.image.bmap }); }).then((writer) => { return new Bluebird((resolve, reject) => { diff --git a/lib/gui/models/selection-state.js b/lib/gui/models/selection-state.js index a44f6fd1..fadc6726 100644 --- a/lib/gui/models/selection-state.js +++ b/lib/gui/models/selection-state.js @@ -260,20 +260,6 @@ SelectionStateModel.service('SelectionStateModel', function(DrivesModel) { return _.get(Store.getState().toJS(), 'selection.image.logo'); }; - /** - * @summary Get image bmap - * @function - * @public - * - * @returns {String} image bmap - * - * @example - * const imageBmap = SelectionStateModel.getImageBmap(); - */ - this.getImageBmap = () => { - return _.get(Store.getState().toJS(), 'selection.image.bmap'); - }; - /** * @summary Check if there is a selected drive * @function diff --git a/lib/gui/modules/image-writer.js b/lib/gui/modules/image-writer.js index 08537be6..f6501905 100644 --- a/lib/gui/modules/image-writer.js +++ b/lib/gui/modules/image-writer.js @@ -58,8 +58,7 @@ imageWriter.service('ImageWriterService', function($q, $rootScope, SettingsModel return $q((resolve, reject) => { const child = childWriter.write(image, drive, { validateWriteOnSuccess: SettingsModel.get('validateWriteOnSuccess'), - unmountOnSuccess: SettingsModel.get('unmountOnSuccess'), - bmapContents: SelectionStateModel.getImageBmap() + unmountOnSuccess: SettingsModel.get('unmountOnSuccess') }); child.on('error', reject); child.on('done', resolve); diff --git a/lib/src/child-writer/index.js b/lib/src/child-writer/index.js index c315a560..95108250 100644 --- a/lib/src/child-writer/index.js +++ b/lib/src/child-writer/index.js @@ -17,9 +17,6 @@ 'use strict'; const EventEmitter = require('events').EventEmitter; -const _ = require('lodash'); -const Bluebird = require('bluebird'); -const fs = Bluebird.promisifyAll(require('fs')); const childProcess = require('child_process'); const rendererUtils = require('./renderer-utils'); const utils = require('./utils'); @@ -61,22 +58,10 @@ const EXIT_CODES = require('../exit-codes'); exports.write = (image, drive, options) => { const emitter = new EventEmitter(); - Bluebird.props({ - logFile: utils.getTemporaryLogFilePath(), - bmapFile: _.attempt(() => { - if (!options.bmapContents) { - return; - } - - return utils.getTemporaryBmapFilePath().tap((bmapFilePath) => { - return fs.writeFileAsync(bmapFilePath, options.bmapContents); - }); - }) - }).then((results) => { + utils.getTemporaryLogFilePath().then((logFile) => { const argv = utils.getCLIWriterArguments({ entryPoint: rendererUtils.getApplicationEntryPoint(), - logFile: results.logFile, - bmap: results.bmapFile, + logFile: logFile, image: image, device: drive.device, validateWriteOnSuccess: options.validateWriteOnSuccess, @@ -85,7 +70,7 @@ exports.write = (image, drive, options) => { // Make writer proxy inherit the temporary log file location // while keeping current environment variables intact. - process.env[CONSTANTS.TEMPORARY_LOG_FILE_ENVIRONMENT_VARIABLE] = results.logFile; + process.env[CONSTANTS.TEMPORARY_LOG_FILE_ENVIRONMENT_VARIABLE] = logFile; const child = childProcess.fork(CONSTANTS.WRITER_PROXY_SCRIPT, argv, { silent: true, diff --git a/lib/src/child-writer/utils.js b/lib/src/child-writer/utils.js index 93c7615d..3885759d 100644 --- a/lib/src/child-writer/utils.js +++ b/lib/src/child-writer/utils.js @@ -60,7 +60,6 @@ exports.getBooleanArgumentForm = (argumentName, value) => { * @param {String} options.device - device * @param {String} options.entryPoint - entry point * @param {String} [options.logFile] - log file - * @param {String} [options.bmap] - bmap file * @param {Boolean} [options.validateWriteOnSuccess] - validate write on success * @param {Boolean} [options.unmountOnSuccess] - unmount on success * @returns {String[]} arguments @@ -108,10 +107,6 @@ exports.getCLIWriterArguments = (options) => { argv.push('--log', options.logFile); } - if (options.bmap) { - argv.push('--bmap', options.bmap); - } - return argv; }; @@ -151,23 +146,3 @@ exports.getTemporaryLogFilePath = () => { postfix: '.log' }); }; - -/** - * @summary Get a temporary bmap file path - * @function - * @public - * - * @fulfil {String} - bmap path - * @returns {Promise} - * - * @example - * utils.getTemporaryBmapFilePath().then((bmapFilePath) => { - * console.log(bmapFilePath); - * }); - */ -exports.getTemporaryBmapFilePath = () => { - return tmp.fileAsync({ - prefix: `${packageJSON.name}-`, - postfix: '.bmap' - }); -}; diff --git a/tests/gui/models/selection-state.spec.js b/tests/gui/models/selection-state.spec.js index c33e9c3e..5047f414 100644 --- a/tests/gui/models/selection-state.spec.js +++ b/tests/gui/models/selection-state.spec.js @@ -55,10 +55,6 @@ describe('Browser: SelectionState', function() { m.chai.expect(SelectionStateModel.getImageLogo()).to.be.undefined; }); - it('getImageBmap() should return undefined', function() { - m.chai.expect(SelectionStateModel.getImageBmap()).to.be.undefined; - }); - it('hasDrive() should return false', function() { const hasDrive = SelectionStateModel.hasDrive(); m.chai.expect(hasDrive).to.be.false; @@ -276,8 +272,7 @@ describe('Browser: SelectionState', function() { size: 999999999, url: 'https://www.raspbian.org', name: 'Raspbian', - logo: 'Raspbian', - bmap: 'Foo Bar' + logo: 'Raspbian' }); }); @@ -430,15 +425,6 @@ describe('Browser: SelectionState', function() { }); - describe('.getImageBmap()', function() { - - it('should return the image bmap', function() { - const imageBmap = SelectionStateModel.getImageBmap(); - m.chai.expect(imageBmap).to.equal('Foo Bar'); - }); - - }); - describe('.hasImage()', function() { it('should return true', function() {