Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: ipfs version flags + ipfs repo version #1181

Merged
merged 11 commits into from
Jan 25, 2018
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"hapi": "^16.6.2",
"hapi-set-header": "^1.0.2",
"hoek": "^5.0.2",
"ipfs-api": "^17.3.0",
"ipfs-api": "^17.5.0",
"ipfs-bitswap": "~0.18.0",
"ipfs-block": "~0.6.1",
"ipfs-block-service": "~0.13.0",
Expand Down
2 changes: 1 addition & 1 deletion src/cli/commands/repo/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = {
builder: {},

handler (argv) {
argv.ipfs.repo.version(function (err, version) {
argv.ipfs.repo.version((err, version) => {
if (err) {
throw err
}
Expand Down
32 changes: 26 additions & 6 deletions src/cli/commands/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,46 @@ module.exports = {
number: {
alias: 'n',
type: 'boolean',
default: false
default: false,
describe: 'Print only the version number'
},
commit: {
type: 'boolean',
default: false
default: false,
describe: `Include the version's commit hash`
},
repo: {
type: 'boolean',
default: false
default: false,
describe: `Print only the repo's version number`
},
all: {
type: 'boolean',
default: false,
describe: 'Print everything we have'
}
},

handler (argv) {
// TODO: handle argv.{repo|commit|number}
argv.ipfs.version((err, version) => {
argv.ipfs.version((err, ipfs) => {
if (err) {
throw err
}

print(`js-ipfs version: ${version.version}`)
const withCommit = argv.all || argv.commit
const parsedVersion = `${ipfs.version}${withCommit ? `-${ipfs.commit}` : ''}`

if (argv.repo) {
// go-ipfs prints only the number, even without the --number flag.
print(ipfs.repo)
} else if (argv.number) {
print(parsedVersion)
} else if (argv.all) {
print(`js-ipfs version: ${parsedVersion}`)
print(`Repo version: ${ipfs.repo}`)
} else {
print(`js-ipfs version: ${parsedVersion}`)
}
})
}
}
15 changes: 11 additions & 4 deletions src/core/components/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,24 @@
const pkg = require('../../../package.json')
const promisify = require('promisify-es6')

// TODO add the commit hash of the current ipfs version to the response.
module.exports = function version (self) {
return promisify((opts, callback) => {
if (typeof opts === 'function') {
callback = opts
opts = {}
}

callback(null, {
version: pkg.version,
repo: '',
commit: ''
self.repo.version((err, repoVersion) => {
if (err) {
throw err
}

callback(null, {
version: pkg.version,
repo: repoVersion,
commit: ''
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonKrone found the issue. Here you are assuming that IPFS has always been init when it is not the case always. the Sharness test is checking the version with ipfs offline and without init.

Copy link
Contributor Author

@JonKrone JonKrone Jan 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! In order to avoid the init constraint, go-ipfs accesses the repo version directly from a constant exposed by go-ipfs/repo so the repo version is available offline and without an initialized repo. Analogous to const repoVersion = require('ipfs-repo').version

Unfortunately, there's no similar field exposed by the js-ipfs-repo package. We could read the js-ipfs-repo/constants file directly like we're doing with pkg.version above or add the version as an export of the js-ipfs-repo package.

If we need to be able to read the version w/o init, I think an export is the way to go and replace the dependence of core/components/repo.version on an initialized _repo with just require('ipfs-repo').version. Should we then add this version constant to the repo SPEC?

Feels like a big change to not use the repo API, what are your thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can totally expose the constant, however, I do not think is the best approach. If we use the version from the module, then we might have a false positive where the module is signaling the latest repo version than the one that is inited.

I feel that the right way to do it is:

  • if repo inited, then read the version from the repo
  • if the repo is not inited, then read the version from the constant

Copy link
Contributor Author

@JonKrone JonKrone Jan 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I agree! I looked into what happens when the version is outdated and found that starting the daemon with an outdated ~./jsipfs/version throws during startup. So even running js-ipfs --help fails. test: echo "5" > ~/.jsipfs/version && jsipfs --help.

If that's expected behavior, I can't think of a case where you can run a cli command on an initialized but outdated repo, and we could safely report the constant version.

I think you have the right of it, though, and we should aim for reporting the version of ~/.jsipfs/version if the repo is initialized and maybe at some point in the future, change startup so that we delay opening the repo connection or make accessing the version independent of opening a repo?

either way: ipfs/js-ipfs-repo#158

})
})
}
16 changes: 16 additions & 0 deletions src/http/api/resources/repo.js
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
'use strict'

const boom = require('boom')

exports = module.exports

exports.version = (request, reply) => {
const ipfs = request.server.app.ipfs

ipfs.repo.version((err, version) => {
if (err) {
return reply(boom.badRequest(err))
}

reply(version)
})
}
2 changes: 1 addition & 1 deletion src/http/api/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = (server) => {
require('./bootstrap')(server)
require('./block')(server)
require('./object')(server)
// require('./repo')(server)
require('./repo')(server)
require('./config')(server)
require('./swarm')(server)
require('./bitswap')(server)
Expand Down
5 changes: 2 additions & 3 deletions src/http/api/routes/repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@

const resources = require('./../resources')

// TODO
module.exports = (server) => {
const api = server.select('API')

api.route({
method: '*',
path: '/api/v0/repo',
handler: resources.repo
path: '/api/v0/repo/version',
handler: resources.repo.version
})
}
28 changes: 28 additions & 0 deletions test/cli/repo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* eslint-env mocha */
'use strict'

const fs = require('fs')
const path = require('path')
const expect = require('chai').expect
const runOnAndOff = require('../utils/on-and-off')

function getRepoVersion (repoPath) {
const versionPath = path.join(repoPath, 'version')
return String(fs.readFileSync(versionPath))
}

describe('repo', () => runOnAndOff((thing) => {
let ipfs
let repoVersion

before(() => {
ipfs = thing.ipfs
repoVersion = getRepoVersion(ipfs.repoPath)
})

it('get the repo version', () => {
return ipfs('repo version').then((out) => {
expect(out).to.eql(`${repoVersion}\n`)
})
})
}))
37 changes: 37 additions & 0 deletions test/cli/version.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
/* eslint-env mocha */
'use strict'

const fs = require('fs')
const path = require('path')
const expect = require('chai').expect
const pkgversion = require('../../package.json').version
const runOnAndOff = require('../utils/on-and-off')

function getRepoVersion (repoPath) {
const versionPath = path.join(repoPath, 'version')
return String(fs.readFileSync(versionPath))
}

describe('version', () => runOnAndOff((thing) => {
let ipfs
let repoVersion

before(() => {
ipfs = thing.ipfs
repoVersion = getRepoVersion(ipfs.repoPath)
})

it('get the version', () => {
Expand All @@ -19,4 +28,32 @@ describe('version', () => runOnAndOff((thing) => {
)
})
})

it('handles --number', () => {
return ipfs('version --number').then(out =>
expect(out).to.eql(`${pkgversion}\n`)
)
})

it('handles --commit', () => {
return ipfs('version --commit').then(out =>
expect(out).to.eql(`js-ipfs version: ${pkgversion}-\n`)
)
})

it('handles --all', () => {
return ipfs('version --all').then(out =>
expect(out).to.include(
`js-ipfs version: ${pkgversion}-
Repo version: ${repoVersion}
`
)
)
})

it('handles --repo', () => {
return ipfs('version --repo').then(out => {
expect(out).to.eql(`${repoVersion}\n`)
})
})
}))