From 43c0c130bfd8a499522c6d1a3d6b234f52212f1f Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Mon, 30 Apr 2018 16:45:26 +0100 Subject: [PATCH 01/10] chore: add vscode and eslint to gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index a68f6491c0..06eea61ff3 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,8 @@ test/repo-tests* **/bundle.js docs +.vscode +.eslintrc # Logs logs *.log From 6924e353971d5a2db4d10fa38e2a718f402a1430 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Mon, 30 Apr 2018 17:23:38 +0100 Subject: [PATCH 02/10] feat(cli): improve error handling and help messages --- src/cli/bin.js | 178 +++++++++++++++++++++++-------------- src/cli/commands/daemon.js | 22 ++--- src/cli/commands/id.js | 16 ++-- src/cli/commands/init.js | 32 +++---- src/cli/utils.js | 32 ++----- src/core/boot.js | 10 ++- src/core/index.js | 61 ++++++++++++- src/http/index.js | 1 - 8 files changed, 209 insertions(+), 143 deletions(-) diff --git a/src/cli/bin.js b/src/cli/bin.js index 2ccf2f66a0..2ea1c9a824 100755 --- a/src/cli/bin.js +++ b/src/cli/bin.js @@ -2,104 +2,144 @@ 'use strict' -const yargs = require('yargs') +const yargs = require('yargs/yargs') const updateNotifier = require('update-notifier') const readPkgUp = require('read-pkg-up') -const fs = require('fs') -const path = require('path') -const utils = require('./utils') -const print = utils.print +const { disablePrinting, print, getNodeOrAPI } = require('./utils') +const addCmd = require('./commands/files/add') +const catCmd = require('./commands/files/cat') +const getCmd = require('./commands/files/get') const pkg = readPkgUp.sync({cwd: __dirname}).pkg + updateNotifier({ pkg, updateCheckInterval: 1000 * 60 * 60 * 24 * 7 // 1 week }).notify() -const args = process.argv.slice(2) +const MSG_USAGE = `Usage: + ipfs - Global p2p merkle-dag filesystem. + + ipfs [options] ...` +const MSG_EPILOGUE = `Use 'ipfs --help' to learn more about each command. + +ipfs uses a repository in the local file system. By default, the repo is +located at ~/.ipfs. To change the repo location, set the $IPFS_PATH +environment variable: + + export IPFS_PATH=/path/to/ipfsrepo -// Determine if the first argument is a sub-system command -const commandNames = fs.readdirSync(path.join(__dirname, 'commands')) -const isCommand = commandNames.includes(`${args[0]}.js`) +EXIT STATUS -const cli = yargs +The CLI will exit with one of the following values: + +0 Successful execution. +1 Failed executions. +` +const MSG_NO_CMD = 'You need at least one command before moving on' + +const argv = process.argv.slice(2) +let args = {} +let cli = yargs(argv) + .usage(MSG_USAGE) .option('silent', { desc: 'Write no output', type: 'boolean', default: false, - coerce: ('silent', silent => silent ? utils.disablePrinting() : silent) + coerce: disablePrinting + }) + .option('debug', { + desc: 'Show debug output', + type: 'boolean', + default: false, + alias: 'D' }) .option('pass', { desc: 'Pass phrase for the keys', type: 'string', default: '' }) - .commandDir('commands', { - // Only include the commands for the sub-system we're using, or include all - // if no sub-system command has been passed. - include (path, filename) { - if (!isCommand) return true - return `${args[0]}.js` === filename - } + .option('api', { + desc: 'Use a specific API instance.', + type: 'string' }) - .epilog(utils.ipfsPathHelp) - .demandCommand(1) - .fail((msg, err, yargs) => { - if (err) { - throw err // preserve stack + .commandDir('commands') + // NOTE: This creates an alias of + // `jsipfs files {add, get, cat}` to `jsipfs {add, get, cat}`. + // This will stay until https://github.com/ipfs/specs/issues/98 is resolved. + .command(addCmd) + .command(catCmd) + .command(getCmd) + .demandCommand(1, MSG_NO_CMD) + .alias('help', 'h') + .epilogue(MSG_EPILOGUE) + .strict() + // .recommendCommands() + .completion() + +if (argv[0] === 'daemon' || argv[0] === 'init' || argv[0] === 'id') { + args = cli.fail((msg, err, yargs) => { + if (err instanceof Error && err.message && !msg) { + msg = err.message } - if (args.length > 0) { - print(msg) + // Cli specific error messages + if (err && err.code === 'ERR_REPO_NOT_INITIALIZED') { + msg = `No IPFS repo found in ${err.path}. +please run: 'ipfs init'` } - yargs.showHelp() - }) + // Show help and error message + if (!args.silent) { + yargs.showHelp() + console.error('Error: ' + msg) + } -// If not a sub-system command then load the top level aliases -if (!isCommand) { - // NOTE: This creates an alias of - // `jsipfs files {add, get, cat}` to `jsipfs {add, get, cat}`. - // This will stay until https://github.com/ipfs/specs/issues/98 is resolved. - const addCmd = require('./commands/files/add') - const catCmd = require('./commands/files/cat') - const getCmd = require('./commands/files/get') - const aliases = [addCmd, catCmd, getCmd] - aliases.forEach((alias) => { - cli.command(alias.command, alias.describe, alias.builder, alias.handler) - }) -} + // Write to stderr when debug is on + if (err && args.debug) { + console.error(err) + } -// Need to skip to avoid locking as these commands -// don't require a daemon -if (args[0] === 'daemon' || args[0] === 'init') { - cli - .help() - .strict() - .completion() - .parse(args) + process.exit(1) + }).argv } else { - // here we have to make a separate yargs instance with - // only the `api` option because we need this before doing - // the final yargs parse where the command handler is invoked.. - yargs().option('api').parse(process.argv, (err, argv, output) => { - if (err) { - throw err - } - utils.getIPFS(argv, (err, ipfs, cleanup) => { - if (err) { throw err } - - cli - .help() - .strict() - .completion() - .parse(args, { ipfs: ipfs }, (err, argv, output) => { - if (output) { print(output) } - - cleanup(() => { - if (err) { throw err } + yargs() + .option('pass', { + desc: 'Pass phrase for the keys', + type: 'string', + default: '' + }) + .option('api', { + desc: 'Use a specific API instance.', + type: 'string' + }) + .parse(argv, (err, parsedArgv, output) => { + if (err) { + console.error(err) + } else { + getNodeOrAPI(parsedArgv) + .then(node => { + args = cli + .parse(argv, { ipfs: node }, (err, parsedArgv, output) => { + if (output) { + print(output) + } + if (node && node._repo && !node._repo.closed) { + node._repo.close(err => { + if (err) { + console.error(err) + } + }) + } + if (err && parsedArgv.debug) { + console.error(err) + } + }) + }) + .catch(err => { + console.error(err) + process.exit(1) }) - }) + } }) - }) } diff --git a/src/cli/commands/daemon.js b/src/cli/commands/daemon.js index 6c6e4d9d30..aa0d16cd67 100644 --- a/src/cli/commands/daemon.js +++ b/src/cli/commands/daemon.js @@ -2,10 +2,9 @@ const HttpAPI = require('../../http') const utils = require('../utils') +const promisify = require('promisify-es6') const print = utils.print -let httpAPI - module.exports = { command: 'daemon', @@ -31,21 +30,8 @@ module.exports = { handler (argv) { print('Initializing daemon...') - const repoPath = utils.getRepoPath() - httpAPI = new HttpAPI(process.env.IPFS_PATH, null, argv) - - httpAPI.start((err) => { - if (err && err.code === 'ENOENT' && err.message.match(/Uninitalized repo/i)) { - print('Error: no initialized ipfs repo found in ' + repoPath) - print('please run: jsipfs init') - process.exit(1) - } - if (err) { - throw err - } - print('Daemon is ready') - }) - + const httpAPI = new HttpAPI(process.env.IPFS_PATH, null, argv) + const start = promisify(httpAPI.start) const cleanup = () => { print(`Received interrupt signal, shutting down..`) httpAPI.stop((err) => { @@ -60,5 +46,7 @@ module.exports = { process.on('SIGTERM', cleanup) process.on('SIGINT', cleanup) process.on('SIGHUP', cleanup) + + return start().then(() => print('Daemon is ready')) } } diff --git a/src/cli/commands/id.js b/src/cli/commands/id.js index 81ed38eef4..24b7c028af 100644 --- a/src/cli/commands/id.js +++ b/src/cli/commands/id.js @@ -1,5 +1,6 @@ 'use strict' -const print = require('../utils').print + +const {print, getNodeOrAPI} = require('../utils') module.exports = { command: 'id', @@ -15,12 +16,11 @@ module.exports = { handler (argv) { // TODO: handle argv.format - argv.ipfs.id((err, id) => { - if (err) { - throw err - } - - print(JSON.stringify(id, '', 2)) - }) + return getNodeOrAPI(argv) + .then(node => Promise.all([Promise.resolve(node), node.id()])) + .then(([node, id]) => { + print(JSON.stringify(id, '', 2)) + node.stop() + }) } } diff --git a/src/cli/commands/init.js b/src/cli/commands/init.js index 36525419b4..ed0c5faa4e 100644 --- a/src/cli/commands/init.js +++ b/src/cli/commands/init.js @@ -2,8 +2,7 @@ const Repo = require('ipfs-repo') const IPFS = require('../../core') -const utils = require('../utils') -const print = utils.print +const { ipfsPathHelp, getRepoPath, print } = require('../utils') module.exports = { command: 'init', @@ -12,7 +11,7 @@ module.exports = { builder (yargs) { return yargs - .epilog(utils.ipfsPathHelp) + .epilog(ipfsPathHelp) .option('bits', { type: 'number', alias: 'b', @@ -22,33 +21,26 @@ module.exports = { .option('emptyRepo', { alias: 'e', type: 'boolean', - describe: "Don't add and pin help files to the local storage" + describe: 'Don\'t add and pin help files to the local storage' }) }, handler (argv) { - const path = utils.getRepoPath() + const path = getRepoPath() print(`initializing ipfs node at ${path}`) - const node = new IPFS({ + return IPFS.createNodePromise({ repo: new Repo(path), init: false, start: false - }) - - node.init({ - bits: argv.bits, - emptyRepo: argv.emptyRepo, - pass: argv.pass, - log: print - }, (err) => { - if (err) { - if (err.code === 'EACCES') { - err.message = `EACCES: permission denied, stat $IPFS_PATH/version` - } - throw err - } + }).then(node => { + return node.init({ + bits: argv.bits, + emptyRepo: argv.emptyRepo, + pass: argv.pass, + log: print + }) }) } } diff --git a/src/cli/utils.js b/src/cli/utils.js index f993e0caef..4a9481b6bd 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -38,14 +38,13 @@ function getAPICtl (apiAddr) { return APIctl(apiAddr) } -exports.getIPFS = (argv, callback) => { +exports.getNodeOrAPI = (argv) => { + log('get node or api async') if (argv.api || isDaemonOn()) { - return callback(null, getAPICtl(argv.api), (cb) => cb()) + return Promise.resolve(getAPICtl(argv.api)) } - - // Required inline to reduce startup time - const IPFS = require('../core') - const node = new IPFS({ + const {createReadyNodePromise} = require('../core') + return IPFS.createReadyNodePromise({ repo: exports.getRepoPath(), init: false, start: false, @@ -54,22 +53,6 @@ exports.getIPFS = (argv, callback) => { pubsub: true } }) - - const cleanup = (cb) => { - if (node && node._repo && !node._repo.closed) { - node._repo.close(() => cb()) - } else { - cb() - } - } - - node.on('error', (err) => { - throw err - }) - - node.once('ready', () => { - callback(null, node, cleanup) - }) } exports.getRepoPath = () => { @@ -77,7 +60,10 @@ exports.getRepoPath = () => { } let visible = true -exports.disablePrinting = () => { visible = false } +exports.disablePrinting = (silent) => { + visible = !silent + return silent +} exports.print = (msg, newline) => { if (newline === undefined) { diff --git a/src/core/boot.js b/src/core/boot.js index cc7c14970e..b99469c0a7 100644 --- a/src/core/boot.js +++ b/src/core/boot.js @@ -72,7 +72,7 @@ module.exports = (self) => { // No repo, but need should init one if (doInit && !hasRepo) { tasks.push((cb) => self.init(initOptions, cb)) - // we know we will have a repo for all follwing tasks + // we know we will have a repo for all following tasks // if the above succeeds hasRepo = true } @@ -98,8 +98,12 @@ module.exports = (self) => { // Need to start up the node if (doStart) { if (!hasRepo) { - console.log('WARNING, trying to start ipfs node on uninitialized repo, maybe forgot to set "init: true"') - return done(new Error('Uninitalized repo')) + return done( + Object.assign(new Error('repo is not initialized yet'), { + code: 'ERR_REPO_NOT_INITIALIZED', + path: self._repo.path + }) + ) } else { tasks.push((cb) => self.start(cb)) } diff --git a/src/core/index.js b/src/core/index.js index 46f8c9bb2c..fc79ab1a11 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -138,8 +138,65 @@ class IPFS extends EventEmitter { } } -exports = module.exports = IPFS +module.exports = IPFS -exports.createNode = (options) => { +IPFS.createNode = (options) => { return new IPFS(options) } + +/** + * Static factory method to create a node wrapped with a Promise + * + * The Promise does *NOT* wait for the Ready Event to resolve with a IPFS instance + * and rejects with Error Events. + * + * @param {object} options + * @returns {IPFS} + */ +IPFS.createNodePromise = (options) => { + return new Promise((resolve, reject) => { + const node = new IPFS(options) + + node.on('error', err => { + reject(err) + }) + + resolve(node) + }) +} + +/** + * Static factory method to create a ready node wrapped with a Promise +* + * The Promise waits for the Ready Event to resolve with a IPFS instance + * and rejects with Error Events or a repo not initialized. + * + * @param {Object} options IPFS constructor options + * @returns {IPFS} + */ +IPFS.createReadyNodePromise = (options = {}) => { + return new Promise((resolve, reject) => { + const node = new IPFS(options) + + node.once('error', err => { + reject(err) + }) + + node.once('ready', () => { + /** + * We shouldn't need to do this but until we get a unified error from ipfs-repo + * we catch it here + */ + node._repo._isInitialized((err) => { + if (err) { + reject(Object.assign(new Error('repo is not initialized yet'), + { + code: 'ERR_REPO_NOT_INITIALIZED', + path: node._repo.path + })) + } + resolve(node) + }) + }) + }) +} diff --git a/src/http/index.js b/src/http/index.js index 93a5ec172e..178e867f37 100644 --- a/src/http/index.js +++ b/src/http/index.js @@ -81,7 +81,6 @@ function HttpApi (repo, config, cliArgs) { this.node.once('error', (err) => { this.log('error starting core', err) - err.code = 'ENOENT' cb(err) }) this.node.once('start', cb) From 40111afd3684f7645c223138a86a9dbe2f754627 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Mon, 30 Apr 2018 17:27:52 +0100 Subject: [PATCH 03/10] test(cli): fix tests for new error messages --- test/cli/config.js | 2 +- test/cli/daemon.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/cli/config.js b/test/cli/config.js index 2150c39e2c..7b96344f8e 100644 --- a/test/cli/config.js +++ b/test/cli/config.js @@ -58,7 +58,7 @@ describe('config', () => runOnAndOff((thing) => { }) it('set a config key with invalid json', () => { - return ipfs.fail('config foo {"bar:0} --json') + return ipfs.fail('config foo {\\"bar:0} --json') }) it('get a config key value', () => { diff --git a/test/cli/daemon.js b/test/cli/daemon.js index d60d591279..91ebc11c1b 100644 --- a/test/cli/daemon.js +++ b/test/cli/daemon.js @@ -118,10 +118,10 @@ describe('daemon', () => { it('gives error if user hasn\'t run init before', function (done) { this.timeout(100 * 1000) - const expectedError = 'no initialized ipfs repo found in ' + repoPath + const expectedError = 'No IPFS repo found in ' + repoPath ipfs('daemon').catch((err) => { - expect(err.stdout).to.have.string(expectedError) + expect(err.stderr).to.have.string(expectedError) done() }) }) From 6dbb8346b254f56144f87e18fa8306de21905a7a Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Tue, 8 May 2018 19:19:46 +0100 Subject: [PATCH 04/10] fix: fix version cmd and repo component --- package.json | 2 +- src/cli/bin.js | 2 +- src/cli/commands/version.js | 40 ++++++++++++------------ src/cli/utils.js | 9 +++--- src/core/boot.js | 10 +----- src/core/components/repo.js | 10 +----- src/core/index.js | 53 +++++++++----------------------- test/http-api/interface/index.js | 2 +- 8 files changed, 44 insertions(+), 84 deletions(-) diff --git a/package.json b/package.json index 707ca981ea..4de4f0a7f7 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ "ipfs-block": "~0.7.1", "ipfs-block-service": "~0.14.0", "ipfs-multipart": "~0.1.0", - "ipfs-repo": "0.20.0", + "ipfs-repo": "~0.21.0", "ipfs-unixfs": "~0.1.14", "ipfs-unixfs-engine": "~0.29.0", "ipld": "~0.17.0", diff --git a/src/cli/bin.js b/src/cli/bin.js index 2ea1c9a824..12e24447ea 100755 --- a/src/cli/bin.js +++ b/src/cli/bin.js @@ -77,7 +77,7 @@ let cli = yargs(argv) // .recommendCommands() .completion() -if (argv[0] === 'daemon' || argv[0] === 'init' || argv[0] === 'id') { +if (['daemon', 'init', 'id', 'version'].includes(argv[0])) { args = cli.fail((msg, err, yargs) => { if (err instanceof Error && err.message && !msg) { msg = err.message diff --git a/src/cli/commands/version.js b/src/cli/commands/version.js index 6bb8447a2f..60ffef094e 100644 --- a/src/cli/commands/version.js +++ b/src/cli/commands/version.js @@ -1,7 +1,7 @@ 'use strict' const os = require('os') -const print = require('../utils').print +const {print, getNodeOrAPI} = require('../utils') module.exports = { command: 'version', @@ -33,27 +33,25 @@ module.exports = { }, handler (argv) { - argv.ipfs.version((err, data) => { - if (err) { - throw err - } + return getNodeOrAPI(argv, {forceInitialized: false}) + .then(node => node.version()) + .then(data => { + const withCommit = argv.all || argv.commit + const parsedVersion = `${data.version}${withCommit ? `-${data.commit}` : ''}` - const withCommit = argv.all || argv.commit - const parsedVersion = `${data.version}${withCommit ? `-${data.commit}` : ''}` - - if (argv.repo) { + if (argv.repo) { // go-ipfs prints only the number, even without the --number flag. - print(data.repo) - } else if (argv.number) { - print(parsedVersion) - } else if (argv.all) { - print(`js-ipfs version: ${parsedVersion}`) - print(`Repo version: ${data.repo}`) - print(`System version: ${os.arch()}/${os.platform()}`) - print(`Node.js version: ${process.version}`) - } else { - print(`js-ipfs version: ${parsedVersion}`) - } - }) + print(data.repo) + } else if (argv.number) { + print(parsedVersion) + } else if (argv.all) { + print(`js-ipfs version: ${parsedVersion}`) + print(`Repo version: ${data.repo}`) + print(`System version: ${os.arch()}/${os.platform()}`) + print(`Node.js version: ${process.version}`) + } else { + print(`js-ipfs version: ${parsedVersion}`) + } + }) } } diff --git a/src/cli/utils.js b/src/cli/utils.js index 4a9481b6bd..1ca67d915e 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -38,13 +38,14 @@ function getAPICtl (apiAddr) { return APIctl(apiAddr) } -exports.getNodeOrAPI = (argv) => { +exports.getNodeOrAPI = (argv, stateOptions = {forceInitialized: true}) => { log('get node or api async') if (argv.api || isDaemonOn()) { return Promise.resolve(getAPICtl(argv.api)) } - const {createReadyNodePromise} = require('../core') - return IPFS.createReadyNodePromise({ + + const {createNodePromise} = require('../core') + return IPFS.createNodePromise({ repo: exports.getRepoPath(), init: false, start: false, @@ -52,7 +53,7 @@ exports.getNodeOrAPI = (argv) => { EXPERIMENTAL: { pubsub: true } - }) + }, stateOptions) } exports.getRepoPath = () => { diff --git a/src/core/boot.js b/src/core/boot.js index b99469c0a7..1fd81e19ba 100644 --- a/src/core/boot.js +++ b/src/core/boot.js @@ -36,15 +36,7 @@ module.exports = (self) => { } ], (err, res) => { if (err) { - // If the error is that no repo exists, - // which happens when the version file is not found - // we just want to signal that no repo exist, not - // fail the whole process. - // TODO: improve datastore and ipfs-repo implemenations so this error is a bit more unified - if (err.message.match(/not found/) || // indexeddb - err.message.match(/ENOENT/) || // fs - err.message.match(/No value/) // memory - ) { + if (err.code === 'ERR_REPO_NOT_INITIALIZED') { return cb(null, false) } return cb(err) diff --git a/src/core/components/repo.js b/src/core/components/repo.js index cf7004559c..158e6ac549 100644 --- a/src/core/components/repo.js +++ b/src/core/components/repo.js @@ -19,15 +19,7 @@ module.exports = function repo (self) { version: promisify((callback) => { self._repo._isInitialized(err => { if (err) { - // TODO: (dryajov) This is really hacky, there must be a better way - const match = [ - /Key not found in database \[\/version\]/, - /ENOENT/, - /not yet initialized/ - ].some((m) => { - return m.test(err.message) - }) - if (match) { + if (err.code === 'ERR_REPO_NOT_INITIALIZED') { // this repo has not been initialized return callback(null, repoVersion) } diff --git a/src/core/index.js b/src/core/index.js index fc79ab1a11..889c830d43 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -147,56 +147,33 @@ IPFS.createNode = (options) => { /** * Static factory method to create a node wrapped with a Promise * - * The Promise does *NOT* wait for the Ready Event to resolve with a IPFS instance - * and rejects with Error Events. - * - * @param {object} options - * @returns {IPFS} - */ -IPFS.createNodePromise = (options) => { - return new Promise((resolve, reject) => { - const node = new IPFS(options) - - node.on('error', err => { - reject(err) - }) - - resolve(node) - }) -} - -/** - * Static factory method to create a ready node wrapped with a Promise -* * The Promise waits for the Ready Event to resolve with a IPFS instance - * and rejects with Error Events or a repo not initialized. + * and rejects with Error Events. Rejections can be customized the with + * second param. * - * @param {Object} options IPFS constructor options + * @param {object} options - IPFS node options + * @param {object} stateOptions - Node state options to reject with error * @returns {IPFS} */ -IPFS.createReadyNodePromise = (options = {}) => { +IPFS.createNodePromise = (options = {}, stateOptions = {}) => { return new Promise((resolve, reject) => { const node = new IPFS(options) - node.once('error', err => { + node.on('error', err => { reject(err) }) node.once('ready', () => { - /** - * We shouldn't need to do this but until we get a unified error from ipfs-repo - * we catch it here - */ - node._repo._isInitialized((err) => { - if (err) { - reject(Object.assign(new Error('repo is not initialized yet'), - { - code: 'ERR_REPO_NOT_INITIALIZED', - path: node._repo.path - })) - } + if (stateOptions.forceInitialized) { + node._repo._isInitialized((err) => { + if (err) { + reject(err) + } + resolve(node) + }) + } else { resolve(node) - }) + } }) }) } diff --git a/test/http-api/interface/index.js b/test/http-api/interface/index.js index 7239f09e3a..3cc82bccc6 100644 --- a/test/http-api/interface/index.js +++ b/test/http-api/interface/index.js @@ -4,7 +4,7 @@ const fs = require('fs') const path = require('path') -describe('## interface-ipfs-core over ipfs-api', () => { +describe.only('## interface-ipfs-core over ipfs-api', () => { fs.readdirSync(path.join(__dirname)) .forEach((file) => file !== 'index.js' && require(`./${file}`)) }) From c649a985b90c5362dfee219b704118f14bbe6634 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Mon, 14 May 2018 15:35:04 +0100 Subject: [PATCH 05/10] fix: resolve conflits with the new inline requires --- src/cli/bin.js | 37 +++++++++++++++++++++++++++---------- src/cli/utils.js | 2 +- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/cli/bin.js b/src/cli/bin.js index 12e24447ea..7bea15fcea 100755 --- a/src/cli/bin.js +++ b/src/cli/bin.js @@ -2,6 +2,8 @@ 'use strict' +const fs = require('fs') +const path = require('path') const yargs = require('yargs/yargs') const updateNotifier = require('update-notifier') const readPkgUp = require('read-pkg-up') @@ -18,16 +20,16 @@ updateNotifier({ }).notify() const MSG_USAGE = `Usage: - ipfs - Global p2p merkle-dag filesystem. +ipfs - Global p2p merkle-dag filesystem. ipfs [options] ...` -const MSG_EPILOGUE = `Use 'ipfs --help' to learn more about each command. + const MSG_EPILOGUE = `Use 'ipfs --help' to learn more about each command. ipfs uses a repository in the local file system. By default, the repo is located at ~/.ipfs. To change the repo location, set the $IPFS_PATH environment variable: - export IPFS_PATH=/path/to/ipfsrepo +export IPFS_PATH=/path/to/ipfsrepo EXIT STATUS @@ -39,6 +41,9 @@ The CLI will exit with one of the following values: const MSG_NO_CMD = 'You need at least one command before moving on' const argv = process.argv.slice(2) +const commandNames = fs.readdirSync(path.join(__dirname, 'commands')) +const isCommand = commandNames.includes(`${argv[0]}.js`) + let args = {} let cli = yargs(argv) .usage(MSG_USAGE) @@ -63,13 +68,25 @@ let cli = yargs(argv) desc: 'Use a specific API instance.', type: 'string' }) - .commandDir('commands') - // NOTE: This creates an alias of - // `jsipfs files {add, get, cat}` to `jsipfs {add, get, cat}`. - // This will stay until https://github.com/ipfs/specs/issues/98 is resolved. - .command(addCmd) - .command(catCmd) - .command(getCmd) + .commandDir('commands', { + // Only include the commands for the sub-system we're using, or include all + // if no sub-system command has been passed. + include (path, filename) { + if (!isCommand) return true + return `${argv[0]}.js` === filename + } + }) + + if(!isCommand){ + cli + // NOTE: This creates an alias of + // `jsipfs files {add, get, cat}` to `jsipfs {add, get, cat}`. + // This will stay until https://github.com/ipfs/specs/issues/98 is resolved. + .command(addCmd) + .command(catCmd) + .command(getCmd) + } + cli .demandCommand(1, MSG_NO_CMD) .alias('help', 'h') .epilogue(MSG_EPILOGUE) diff --git a/src/cli/utils.js b/src/cli/utils.js index 1ca67d915e..2b552b1b66 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -44,7 +44,7 @@ exports.getNodeOrAPI = (argv, stateOptions = {forceInitialized: true}) => { return Promise.resolve(getAPICtl(argv.api)) } - const {createNodePromise} = require('../core') + const IPFS = require('../core') return IPFS.createNodePromise({ repo: exports.getRepoPath(), init: false, From 2bdfeeecd0555e957df3f45625c7ed2c0de060a7 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Mon, 14 May 2018 17:51:43 +0100 Subject: [PATCH 06/10] chore: remove only from http-api tests --- test/http-api/interface/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/http-api/interface/index.js b/test/http-api/interface/index.js index 3cc82bccc6..7239f09e3a 100644 --- a/test/http-api/interface/index.js +++ b/test/http-api/interface/index.js @@ -4,7 +4,7 @@ const fs = require('fs') const path = require('path') -describe.only('## interface-ipfs-core over ipfs-api', () => { +describe('## interface-ipfs-core over ipfs-api', () => { fs.readdirSync(path.join(__dirname)) .forEach((file) => file !== 'index.js' && require(`./${file}`)) }) From 43bcafac9be2b6484e08859b211716dc027ae726 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Tue, 15 May 2018 12:13:09 +0100 Subject: [PATCH 07/10] fix: add back inline require in bin.js and some small stuff fix indentation in bin.js add back inline require comment in utils.js --- src/cli/bin.js | 12 +++++------- src/cli/utils.js | 1 + 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/cli/bin.js b/src/cli/bin.js index 7bea15fcea..c07ac76aa6 100755 --- a/src/cli/bin.js +++ b/src/cli/bin.js @@ -8,9 +8,6 @@ const yargs = require('yargs/yargs') const updateNotifier = require('update-notifier') const readPkgUp = require('read-pkg-up') const { disablePrinting, print, getNodeOrAPI } = require('./utils') -const addCmd = require('./commands/files/add') -const catCmd = require('./commands/files/cat') -const getCmd = require('./commands/files/get') const pkg = readPkgUp.sync({cwd: __dirname}).pkg @@ -23,7 +20,8 @@ const MSG_USAGE = `Usage: ipfs - Global p2p merkle-dag filesystem. ipfs [options] ...` - const MSG_EPILOGUE = `Use 'ipfs --help' to learn more about each command. + +const MSG_EPILOGUE = `Use 'ipfs --help' to learn more about each command. ipfs uses a repository in the local file system. By default, the repo is located at ~/.ipfs. To change the repo location, set the $IPFS_PATH @@ -82,9 +80,9 @@ let cli = yargs(argv) // NOTE: This creates an alias of // `jsipfs files {add, get, cat}` to `jsipfs {add, get, cat}`. // This will stay until https://github.com/ipfs/specs/issues/98 is resolved. - .command(addCmd) - .command(catCmd) - .command(getCmd) + .command(require('./commands/files/add')) + .command(require('./commands/files/cat')) + .command(require('./commands/files/get')) } cli .demandCommand(1, MSG_NO_CMD) diff --git a/src/cli/utils.js b/src/cli/utils.js index 2b552b1b66..d45cc3eaea 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -44,6 +44,7 @@ exports.getNodeOrAPI = (argv, stateOptions = {forceInitialized: true}) => { return Promise.resolve(getAPICtl(argv.api)) } + // Required inline to reduce startup time const IPFS = require('../core') return IPFS.createNodePromise({ repo: exports.getRepoPath(), From 34bab953dbb6da3f2633c3ce5a9bac0315027b22 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Tue, 15 May 2018 16:27:11 +0100 Subject: [PATCH 08/10] fix: remove not needed Promise.resolve and change stateOption to forceRepoInitialized forceRepoInitialized represents better to objective of the option forceInitialized could confuse we node initiliazed instead of repo --- src/cli/commands/id.js | 2 +- src/cli/commands/version.js | 6 +++--- src/cli/utils.js | 2 +- src/core/index.js | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cli/commands/id.js b/src/cli/commands/id.js index 24b7c028af..e3fa79c1b9 100644 --- a/src/cli/commands/id.js +++ b/src/cli/commands/id.js @@ -17,7 +17,7 @@ module.exports = { handler (argv) { // TODO: handle argv.format return getNodeOrAPI(argv) - .then(node => Promise.all([Promise.resolve(node), node.id()])) + .then(node => Promise.all([node, node.id()])) .then(([node, id]) => { print(JSON.stringify(id, '', 2)) node.stop() diff --git a/src/cli/commands/version.js b/src/cli/commands/version.js index 60ffef094e..a24ef3d29b 100644 --- a/src/cli/commands/version.js +++ b/src/cli/commands/version.js @@ -33,9 +33,9 @@ module.exports = { }, handler (argv) { - return getNodeOrAPI(argv, {forceInitialized: false}) - .then(node => node.version()) - .then(data => { + return getNodeOrAPI(argv, {forceRepoInitialized: false}) + .then(node => Promise.all([node, node.version()])) + .then(([node, data])=> { const withCommit = argv.all || argv.commit const parsedVersion = `${data.version}${withCommit ? `-${data.commit}` : ''}` diff --git a/src/cli/utils.js b/src/cli/utils.js index d45cc3eaea..e10a986bc1 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -38,7 +38,7 @@ function getAPICtl (apiAddr) { return APIctl(apiAddr) } -exports.getNodeOrAPI = (argv, stateOptions = {forceInitialized: true}) => { +exports.getNodeOrAPI = (argv, stateOptions = {forceRepoInitialized: true}) => { log('get node or api async') if (argv.api || isDaemonOn()) { return Promise.resolve(getAPICtl(argv.api)) diff --git a/src/core/index.js b/src/core/index.js index 889c830d43..2f218d9c7a 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -164,7 +164,7 @@ IPFS.createNodePromise = (options = {}, stateOptions = {}) => { }) node.once('ready', () => { - if (stateOptions.forceInitialized) { + if (stateOptions.forceRepoInitialized) { node._repo._isInitialized((err) => { if (err) { reject(err) From 7209da322ffb4130eaadad6ec0a21e4f949db151 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Tue, 15 May 2018 16:35:29 +0100 Subject: [PATCH 09/10] feat: add clean method to make node cleanup easier --- src/cli/commands/id.js | 2 +- src/cli/commands/version.js | 2 ++ src/core/index.js | 30 ++++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/cli/commands/id.js b/src/cli/commands/id.js index e3fa79c1b9..dee40ea2f6 100644 --- a/src/cli/commands/id.js +++ b/src/cli/commands/id.js @@ -20,7 +20,7 @@ module.exports = { .then(node => Promise.all([node, node.id()])) .then(([node, id]) => { print(JSON.stringify(id, '', 2)) - node.stop() + node.clean() }) } } diff --git a/src/cli/commands/version.js b/src/cli/commands/version.js index a24ef3d29b..775f07d407 100644 --- a/src/cli/commands/version.js +++ b/src/cli/commands/version.js @@ -52,6 +52,8 @@ module.exports = { } else { print(`js-ipfs version: ${parsedVersion}`) } + + node.clean(); }) } } diff --git a/src/core/index.js b/src/core/index.js index 2f218d9c7a..30b7cd4156 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -136,6 +136,35 @@ class IPFS extends EventEmitter { boot(this) } + + + /** + * Clean a node after usage, checks the current state and calls the needed methods + * Until _repo.close return a promise we just throw the error to perserve the stack + * + * @memberof IPFS + */ + clean() { + this.log('cleaning: state ', this.state.state()) + switch (this.state.state()) { + case 'stopped': + this._repo.close(err => { + if(err) { + throw err + } + }); + break; + case 'running': + this.stop(err => { + if(err) { + throw err + } + }) + break; + default: + break; + } + } } module.exports = IPFS @@ -160,6 +189,7 @@ IPFS.createNodePromise = (options = {}, stateOptions = {}) => { const node = new IPFS(options) node.on('error', err => { + node.clean() reject(err) }) From 7f6fccc795118b12f0271b231d88a7304b28456d Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Tue, 15 May 2018 16:36:01 +0100 Subject: [PATCH 10/10] fix: remove unneeded check in init boot always runs first and puts state in stopped state, this check doesnt make sense self.state.init() does nothing fsm even prints an error, check with DEBUG=* js-ipfs init --- src/core/components/init.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/core/components/init.js b/src/core/components/init.js index 59555f383a..d50af6cfe0 100644 --- a/src/core/components/init.js +++ b/src/core/components/init.js @@ -27,11 +27,6 @@ module.exports = function init (self) { callback(null, res) } - if (self.state.state() !== 'uninitalized') { - return done(new Error('Not able to init from state: ' + self.state.state())) - } - - self.state.init() self.log('init') opts.emptyRepo = opts.emptyRepo || false