Add possibility to pass config options to local node and down to init()

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ipfsd-ctl",
-  "version": "0.14.0",
+  "version": "0.17.0",
   "description": "simple controls for an ipfs node",
   "main": "lib/index.js",
   "jsxnext:main": "src/index.js",
@@ -10,8 +10,8 @@
     "test": "aegir-test --env node",
     "build": "aegir-build --env node",
     "release": "aegir-release --env node",
-    "release:minor": "aegir-release --type minor --env node",
-    "release:major": "aegir-release --type major --env node",
+    "release-minor": "aegir-release --type minor --env node",
+    "release-major": "aegir-release --type major --env node",
     "coverage-publish": "aegir-coverage publish"
   },
   "engines": {
@@ -31,30 +31,34 @@
     "Bret Comnes <[email protected]>",
     "David Dias <[email protected]>",
     "FrauBienenstich <[email protected]>",
+    "Friedel Ziegelmayer <[email protected]>",
     "Harlan T Wood <[email protected]>",
-    "Juan Benet <[email protected]>",
     "Kristoffer Ström <[email protected]>",
     "Lars-Magnus Skog <[email protected]>",
+    "Richard Littauer <[email protected]>",
     "Stephen Whitmore <[email protected]>",
-    "dignifiedquire <dignifiedquire@gmail.com>",
+    "edsilv <e.silverton@gmail.com>",
     "greenkeeperio-bot <[email protected]>",
     "haad <[email protected]>",
-    "haadcode <[email protected]>"
+    "haadcode <[email protected]>",
+    "jbenet <[email protected]>"
   ],
   "license": "MIT",
   "dependencies": {
-    "go-ipfs-dep": "0.4.1",
-    "ipfs-api": "^4.1.0",
-    "multiaddr": "^2.0.0",
-    "rimraf": "^2.4.5",
+    "bl": "^1.1.2",
+    "go-ipfs-dep": "0.4.4",
+    "ipfs-api": "^9.0.0",
+    "multiaddr": "^2.0.3",
+    "once": "^1.4.0",
+    "rimraf": "^2.5.4",
     "run-series": "^1.1.4",
     "shutdown": "^0.2.4",
     "subcomandante": "^1.0.5"
   },
   "devDependencies": {
-    "aegir": "^8.0.1",
+    "aegir": "^9.1.1",
     "mkdirp": "^0.5.1",
-    "pre-commit": "^1.1.2"
+    "pre-commit": "^1.1.3"
   },
   "repository": {
     "type": "git",

src/index.js

@@ -13,15 +13,19 @@ module.exports = {
   version (done) {
     (new Node()).version(done)
   },
-  local (path, done) {
+  local (path, opts, done) {
+    if (typeof opts === 'function') {
+      done = opts
+      opts = {}
+    }
     if (!done) {
       done = path
       path = process.env.IPFS_PATH ||
         join(process.env.HOME ||
              process.env.USERPROFILE, '.ipfs')
     }
     process.nextTick(() => {
-      done(null, new Node(path))
+      done(null, new Node(path, opts))
     })
   },
   disposableApi (opts, done) {

src/node.js

@@ -9,17 +9,20 @@ const rimraf = require('rimraf')
 const shutdown = require('shutdown')
 const path = require('path')
 const join = path.join
-
-const rootPath = process.env.testpath ? process.env.testpath : __dirname
+const bl = require('bl')
+const once = require('once')
 
 const ipfsDefaultPath = findIpfsExecutable()
 
 const GRACE_PERIOD = 7500 // amount of ms to wait before sigkill
 
 function findIpfsExecutable () {
-  let npm3Path = path.join(rootPath, '../../', '/go-ipfs-dep/go-ipfs/ipfs')
+  const rootPath = process.env.testpath ? process.env.testpath : __dirname
 
-  let npm2Path = path.join(rootPath, '../', 'node_modules/go-ipfs-dep/go-ipfs/ipfs')
+  const appRoot = path.join(rootPath, '..')
+  const depPath = path.join('go-ipfs-dep', 'go-ipfs', 'ipfs')
+  const npm3Path = path.join(appRoot, '../', depPath)
+  const npm2Path = path.join(appRoot, 'node_modules', depPath)
 
   try {
     fs.statSync(npm3Path)
@@ -43,8 +46,9 @@ function configureNode (node, conf, done) {
 
 // Consistent error handling
 function parseConfig (path, done) {
+  const file = fs.readFileSync(join(path, 'config'))
+
   try {
-    const file = fs.readFileSync(join(path, 'config'))
     const parsed = JSON.parse(file)
     done(null, parsed)
   } catch (err) {
@@ -66,21 +70,22 @@ module.exports = class Node {
   }
 
   _run (args, envArg, done) {
-    let result = ''
     run(this.exec, args, envArg)
       .on('error', done)
-      .on('data', (data) => {
-        result += data
-      })
-      .on('end', () => done(null, result.trim()))
+      .pipe(bl((err, result) => {
+        if (err) {
+          return done(err)
+        }
+
+        done(null, result.toString().trim())
+      }))
   }
 
   init (initOpts, done) {
     if (!done) {
       done = initOpts
       initOpts = {}
     }
-    let buf = ''
 
     const keySize = initOpts.keysize || 2048
 
@@ -91,24 +96,29 @@ module.exports = class Node {
 
     run(this.exec, ['init', '-b', keySize], {env: this.env})
       .on('error', done)
-      .on('data', (data) => {
-        buf += data
-      })
-      .on('end', () => {
+      .pipe(bl((err, buf) => {
+        if (err) return done(err)
+
         configureNode(this, this.opts, (err) => {
-          if (err) return done(err)
+          if (err) {
+            return done(err)
+          }
+
           this.clean = false
           this.initialized = true
+
           done(null, this)
         })
-      })
+      }))
 
     if (this.disposable) {
       shutdown.addHandler('disposable', 1, this.shutdown.bind(this))
     }
   }
 
   // cleanup tmp files
+  // TODO: this is a bad name for a function. a user may call this expecting
+  // something similar to "stopDaemon()". consider changing it. - @jbenet
   shutdown (done) {
     if (!this.clean && this.disposable) {
       rimraf(this.path, (err) => {
@@ -118,51 +128,117 @@ module.exports = class Node {
     }
   }
 
-  startDaemon (done) {
-    parseConfig(this.path, (err, conf) => {
+  startDaemon (flags, done) {
+    if (typeof flags === 'function' && typeof done === 'undefined') {
+      done = flags
+      flags = []
+    }
+
+    const node = this
+    parseConfig(node.path, (err, conf) => {
       if (err) return done(err)
 
-      this.subprocess = run(this.exec, ['daemon'], {env: this.env})
-        .on('error', (err) => {
-          if (String(err).match('daemon is running')) {
-            // we're good
-            done(null, ipfs(conf.Addresses.API))
-          } else if (String(err).match('non-zero exit code')) {
-            // ignore when kill -9'd
-          } else {
-            done(err)
-          }
-        })
-        .on('data', (data) => {
-          const match = String(data).trim().match(/API server listening on (.*)/)
-          if (match) {
-            this.apiAddr = match[1]
-            const addr = multiaddr(this.apiAddr).nodeAddress()
-            const api = ipfs(this.apiAddr)
-            api.apiHost = addr.address
-            api.apiPort = addr.port
-            done(null, api)
-          }
-        })
+      let stdout = ''
+      let args = ['daemon'].concat(flags || [])
+
+      // strategy:
+      // - run subprocess
+      // - listen for API addr on stdout (success)
+      // - or an early exit or error (failure)
+      node.subprocess = run(node.exec, args, {env: node.env})
+      node.subprocess.on('error', onErr)
+        .on('data', onData)
+
+      // done2 is called to call done after removing the event listeners
+      let done2 = (err, val) => {
+        node.subprocess.removeListener('data', onData)
+        node.subprocess.removeListener('error', onErr)
+        if (err) {
+          node.killProcess(() => {}) // we failed. kill, just to be sure...
+        }
+        done(err, val)
+        done2 = () => {} // in case it gets called twice
+      }
+
+      function onErr (err) {
+        if (String(err).match('daemon is running')) {
+          // we're good
+          done2(null, ipfs(conf.Addresses.API))
+
+          // TODO: I don't think this case is OK at all...
+          // When does the daemon outout "daemon is running" ?? seems old.
+          // Someone should check on this... - @jbenet
+        } else if (String(err).match('non-zero exit code')) {
+          // exited with an error on startup, before we removed listeners
+          done2(err)
+        } else {
+          done2(err)
+        }
+      }
+
+      function onData (data) {
+        data = String(data)
+        stdout += data
+
+        if (!data.trim().match(/Daemon is ready/)) {
+          return // not ready yet, keep waiting.
+        }
+
+        const apiM = stdout.match(/API server listening on (.*)\n/)
+        if (apiM) {
+          // found the API server listening. extract the addr.
+          node.apiAddr = apiM[1]
+        } else {
+          // daemon ready but no API server? seems wrong...
+          done2(new Error('daemon ready without api'))
+        }
+
+        const gatewayM = stdout.match(/Gateway \((readonly|writable)\) server listening on (.*)\n/)
+        if (gatewayM) {
+          // found the Gateway server listening. extract the addr.
+          node.gatewayAddr = gatewayM[1]
+        }
+
+        const addr = multiaddr(node.apiAddr).nodeAddress()
+        const api = ipfs(node.apiAddr)
+        api.apiHost = addr.address
+        api.apiPort = addr.port
+
+        // We are happyly listening, so let's not hide other errors
+        node.subprocess.removeListener('error', onErr)
+
+        done2(null, api)
+      }
     })
   }
 
   stopDaemon (done) {
-    if (!done) done = () => {}
-    if (!this.subprocess) return done(null)
+    if (!done) {
+      done = () => {}
+    }
 
-    this.subprocess.kill('SIGTERM')
+    if (!this.subprocess) {
+      return done()
+    }
+
+    this.killProcess(done)
+  }
 
+  killProcess (done) {
+    // need a local var for the closure, as we clear the var.
+    const subprocess = this.subprocess
     const timeout = setTimeout(() => {
-      this.subprocess.kill('SIGKILL')
-      done(null)
+      subprocess.kill('SIGKILL')
+      done()
     }, GRACE_PERIOD)
 
-    this.subprocess.on('close', () => {
+    subprocess.on('close', () => {
       clearTimeout(timeout)
-      done(null)
+      this.subprocess = null
+      done()
     })
 
+    subprocess.kill('SIGTERM')
     this.subprocess = null
   }
 
@@ -180,9 +256,10 @@ module.exports = class Node {
   }
 
   setConfig (key, value, done) {
+    done = once(done)
     run(this.exec, ['config', key, value, '--json'], {env: this.env})
       .on('error', done)
-      .on('data', (data) => {})
+      .on('data', () => {})
       .on('end', () => done())
   }
 

test/fixtures/test.txt

@@ -0,0 +1 @@
+hello world

test/index.spec.js

@@ -1,4 +1,5 @@
 /* eslint-env mocha */
+/* eslint max-nested-callbacks: ["error", 8] */
 'use strict'
 
 const ipfsd = require('../src')
@@ -364,8 +365,29 @@ describe('ipfs-api version', function () {
 
       const added = res[res.length - 1]
       assert(added)
-      assert.equal(added.Hash, 'Qmafmh1Cw3H1bwdYpaaj5AbCW4LkYyUWaM7Nykpn5NZoYL')
+      assert.equal(added.Hash, 'Qmd4VbGayymErsAE1E6FYD4SnDgbL8z3P2h9jRQCaBaDSV')
       done()
     })
   })
 })
+
+describe('node startDaemon', () => {
+  it('allows passing flags', (done) => {
+    ipfsd.disposable((err, node) => {
+      if (err) throw err
+      node.startDaemon(['--should-not-exist'], (err, ignore) => {
+        if (!err) {
+          throw new Error('should have errored')
+        }
+
+        let errStr = 'Unrecognized option \'should-not-exist\''
+
+        if (String(err).indexOf(errStr) >= 0) {
+          done() // correct error
+        }
+
+        throw err
+      })
+    })
+  })
+})