diff options
author | Forrest L Norvell <forrest@npmjs.com> | 2015-04-10 07:03:23 +0300 |
---|---|---|
committer | Forrest L Norvell <forrest@npmjs.com> | 2015-04-10 13:24:07 +0300 |
commit | 6b0f58877f37df9904490ffbaaad33862bd36dce (patch) | |
tree | 3b78a7361f2ac6ccf1644fb6276639a86a236e5c | |
parent | 33d1420bf2f629332fceb2ac7e174e63ac48f96a (diff) |
git: use paths as presented by user
* Save the lightest-possibly normalized URL into `package.json` and
package.json _from fields, because read-package-json, read-installed,
and the cache all know how to handle them now.
* Add support for github:, gist:, bitbucket:, and gitlab: shortcut
syntax. GitHub shortcuts will continue to be normalized to org/repo
instead of being saved with a github: prefix (for now).
* If presented with shortcuts, try cloning via git, SSH, and HTTPS (in
that order, skipping any methods that aren't supported by a given
hosting provider).
* No longer prompt for credentials -- it didn't work right with the
spinner, and wasn't guaranteed to work anyway. We may experiment with
doing this a better way in the future. Users can override this by
setting GIT_ASKPASS in their environment if they want to experiment
with interactive cloning, but should also set --no-spin on the npm
command line (or run `npm config set spin=false`).
-rw-r--r-- | lib/cache.js | 9 | ||||
-rw-r--r-- | lib/cache/add-remote-git.js | 521 | ||||
-rw-r--r-- | lib/cache/maybe-github.js | 35 | ||||
-rw-r--r-- | test/tap/add-remote-git-file.js | 2 | ||||
-rw-r--r-- | test/tap/git-cache-locking.js | 44 | ||||
-rw-r--r-- | test/tap/github-shortcut.js | 5 | ||||
-rw-r--r-- | test/tap/ls-l-depth-0.js | 2 | ||||
-rw-r--r-- | test/tap/maybe-github.js | 72 | ||||
-rw-r--r-- | test/tap/outdated-git.js | 35 |
9 files changed, 334 insertions, 391 deletions
diff --git a/lib/cache.js b/lib/cache.js index b3c237509..cae09b181 100644 --- a/lib/cache.js +++ b/lib/cache.js @@ -76,7 +76,6 @@ var npm = require("./npm.js") , addLocal = require("./cache/add-local.js") , addRemoteTarball = require("./cache/add-remote-tarball.js") , addRemoteGit = require("./cache/add-remote-git.js") - , maybeGithub = require("./cache/maybe-github.js") , inflight = require("inflight") , realizePackageSpecifier = require("realize-package-specifier") , npa = require("npm-package-arg") @@ -296,14 +295,8 @@ function add (args, where, cb) { }) break case "git": - addRemoteGit(p.spec, false, cb) - break case "hosted": - if (p.hosted.type === "github") { - maybeGithub(p.rawSpec, cb) - } else { - addRemoteGit(p.spec, false, cb) - } + addRemoteGit(p.rawSpec, cb) break default: if (p.name) return addNamed(p.name, p.spec, null, cb) diff --git a/lib/cache/add-remote-git.js b/lib/cache/add-remote-git.js index 84d5207dd..78f0717c6 100644 --- a/lib/cache/add-remote-git.js +++ b/lib/cache/add-remote-git.js @@ -1,73 +1,134 @@ -var mkdir = require('mkdirp') var assert = require('assert') -var git = require('../utils/git.js') +var crypto = require('crypto') var fs = require('graceful-fs') -var log = require('npmlog') var path = require('path') var url = require('url') + var chownr = require('chownr') -var crypto = require('crypto') -var npm = require('../npm.js') -var rm = require('../utils/gently-rm.js') +var dezalgo = require('dezalgo') +var hostedFromURL = require('hosted-git-info').fromUrl var inflight = require('inflight') -var getCacheStat = require('./get-stat.js') -var addLocal = require('./add-local.js') -var realizePackageSpecifier = require('realize-package-specifier') +var log = require('npmlog') +var mkdir = require('mkdirp') var normalizeGitUrl = require('normalize-git-url') -var randomBytes = require('crypto').pseudoRandomBytes // only need uniqueness +var npa = require('npm-package-arg') +var realizePackageSpecifier = require('realize-package-specifier') + +var addLocal = require('./add-local.js') +var getCacheStat = require('./get-stat.js') +var git = require('../utils/git.js') +var npm = require('../npm.js') +var rm = require('../utils/gently-rm.js') var remotes = path.resolve(npm.config.get('cache'), '_git-remotes') var templates = path.join(remotes, '_templates') var VALID_VARIABLES = [ - 'GIT_SSH', - 'GIT_SSL_NO_VERIFY', + 'GIT_ASKPASS', 'GIT_PROXY_COMMAND', - 'GIT_SSL_CAINFO' + 'GIT_SSH', + 'GIT_SSL_CAINFO', + 'GIT_SSL_NO_VERIFY' ] -module.exports = function addRemoteGit (uri, silent, cb) { +module.exports = function addRemoteGit (uri, _cb) { assert(typeof uri === 'string', 'must have git URL') - assert(typeof cb === 'function', 'must have callback') + assert(typeof _cb === 'function', 'must have callback') + var cb = dezalgo(_cb) + + // the URL comes in exactly as it was passed on the command line, or as + // normalized by normalize-package-data / read-package-json / read-installed, + // so figure out what to do with it using hosted-git-info + var parsed = hostedFromURL(uri) + if (parsed) { + // normalize GitHub syntax to org/repo (for now) + var from + if (parsed.type === 'github' && parsed.default === 'shortcut') { + from = parsed.path() + } else { + from = parsed.toString() + } + + log.verbose('addRemoteGit', from, 'is a repository hosted by', parsed.type) + + // prefer explicit URLs to pushing everything through shortcuts + if (parsed.default !== 'shortcut') { + return tryClone(from, parsed.toString(), false, cb) + } - // reconstruct the URL as it was passed in – realizePackageSpecifier - // strips off `git+` and `maybeGithub` doesn't include it. - var originalURL - if (!/^git[+:]/.test(uri)) { - originalURL = 'git+' + uri + // try git:, then git+ssh:, then git+https: before failing + tryGitProto(from, parsed, cb) } else { - originalURL = uri + // verify that this is a Git URL before continuing + parsed = npa(uri) + if (parsed.type !== 'git') { + return cb(new Error(uri + 'is not a Git or GitHub URL')) + } + + tryClone(parsed.rawSpec, uri, false, cb) + } +} + +function tryGitProto (from, hostedInfo, cb) { + var gitURL = hostedInfo.git() + if (!gitURL) return trySSH(from, hostedInfo, cb) + + log.silly('tryGitProto', 'attempting to clone', gitURL) + tryClone(from, gitURL, true, function (er) { + if (er) return trySSH(from, hostedInfo, cb) + + cb.apply(this, arguments) + }) +} + +function trySSH (from, hostedInfo, cb) { + var sshURL = hostedInfo.ssh() + if (!sshURL) return tryHTTPS(from, hostedInfo, cb) + + log.silly('trySSH', 'attempting to clone', sshURL) + tryClone(from, sshURL, true, function (er) { + if (er) return tryHTTPS(from, hostedInfo, cb) + + cb.apply(this, arguments) + }) +} + +function tryHTTPS (from, hostedInfo, cb) { + var httpsURL = hostedInfo.https() + if (!httpsURL) { + return cb(new Error(from + ' can not be cloned via Git, SSH, or HTTPS')) } - // break apart the origin URL and the branch / tag / commitish - var normalized = normalizeGitUrl(uri) - var gitURL = normalized.url + log.silly('tryHTTPS', 'attempting to clone', httpsURL) + tryClone(from, httpsURL, false, cb) +} + +function tryClone (from, combinedURL, silent, cb) { + log.silly('tryClone', 'cloning', from, 'via', combinedURL) + + var normalized = normalizeGitUrl(combinedURL) + var cloneURL = normalized.url var treeish = normalized.branch // ensure that similarly-named remotes don't collide - var repoID = gitURL.replace(/[^a-zA-Z0-9]+/g, '-') + '-' + - crypto.createHash('sha1').update(gitURL).digest('hex').slice(0, 8) + var repoID = cloneURL.replace(/[^a-zA-Z0-9]+/g, '-') + '-' + + crypto.createHash('sha1').update(cloneURL).digest('hex').slice(0, 8) var cachedRemote = path.join(remotes, repoID) - // set later, as the callback flow proceeds - var resolvedURL - var resolvedTreeish - var tmpdir - cb = inflight(repoID, cb) if (!cb) { - return log.verbose('addRemoteGit', repoID, 'already in flight; waiting') + return log.verbose('tryClone', repoID, 'already in flight; waiting') } - log.verbose('addRemoteGit', repoID, 'not in flight; caching') + log.verbose('tryClone', repoID, 'not in flight; caching') // initialize the remotes cache with the correct perms getGitDir(function (er) { if (er) return cb(er) fs.stat(cachedRemote, function (er, s) { - if (er) return mirrorRemote(finish) - if (!s.isDirectory()) return resetRemote(finish) + if (er) return mirrorRemote(from, cloneURL, treeish, cachedRemote, silent, finish) + if (!s.isDirectory()) return resetRemote(from, cloneURL, treeish, cachedRemote, finish) - validateExistingRemote(finish) + validateExistingRemote(from, cloneURL, treeish, cachedRemote, finish) }) // always set permissions on the cached remote @@ -78,235 +139,229 @@ module.exports = function addRemoteGit (uri, silent, cb) { }) } }) +} - // don't try too hard to hold on to a remote - function resetRemote (cb) { - log.info('addRemoteGit', 'resetting', cachedRemote) - rm(cachedRemote, function (er) { - if (er) return cb(er) - mirrorRemote(cb) - }) - } - - // reuse a cached remote when possible, but nuke it if it's in an - // inconsistent state - function validateExistingRemote (cb) { - git.whichAndExec( - ['config', '--get', 'remote.origin.url'], - { cwd: cachedRemote, env: gitEnv() }, - function (er, stdout, stderr) { - var originURL - if (stdout) { - originURL = stdout.trim() - log.verbose('addRemoteGit', 'remote.origin.url:', originURL) - } - - if (stderr) stderr = stderr.trim() - if (stderr || er) { - log.warn('addRemoteGit', 'resetting remote', cachedRemote, 'because of error:', stderr || er) - return resetRemote(cb) - } else if (gitURL !== originURL) { - log.warn( - 'addRemoteGit', - 'pre-existing cached repo', cachedRemote, 'points to', originURL, 'and not', gitURL - ) - return resetRemote(cb) - } +// don't try too hard to hold on to a remote +function resetRemote (from, cloneURL, treeish, cachedRemote, cb) { + log.info('resetRemote', 'resetting', cachedRemote, 'for', from) + rm(cachedRemote, function (er) { + if (er) return cb(er) + mirrorRemote(from, cloneURL, treeish, cachedRemote, false, cb) + }) +} - log.verbose('addRemoteGit', 'updating existing cached remote', cachedRemote) - updateRemote(cb) +// reuse a cached remote when possible, but nuke it if it's in an +// inconsistent state +function validateExistingRemote (from, cloneURL, treeish, cachedRemote, cb) { + git.whichAndExec( + ['config', '--get', 'remote.origin.url'], + { cwd: cachedRemote, env: gitEnv() }, + function (er, stdout, stderr) { + var originURL + if (stdout) { + originURL = stdout.trim() + log.silly('validateExistingRemote', from, 'remote.origin.url:', originURL) } - ) - } - - // make a complete bare mirror of the remote repo - // NOTE: npm uses a blank template directory to prevent weird inconsistencies - // https://github.com/npm/npm/issues/5867 - function mirrorRemote (cb) { - mkdir(cachedRemote, function (er) { - if (er) return cb(er) - var args = [ - 'clone', - '--template=' + templates, - '--mirror', - gitURL, cachedRemote - ] - git.whichAndExec( - ['clone', '--template=' + templates, '--mirror', gitURL, cachedRemote], - { cwd: cachedRemote, env: gitEnv() }, - function (er, stdout, stderr) { - if (er) { - var combined = (stdout + '\n' + stderr).trim() - var command = 'git ' + args.join(' ') + ':' - if (silent) { - log.verbose(command, combined) - } else { - log.error(command, combined) - } - return cb(er) - } - log.verbose('addRemoteGit', 'git clone ' + gitURL, stdout.trim()) - setPermissions(cb) - } - ) - }) - } - - function setPermissions (cb) { - if (process.platform === 'win32') { - log.verbose('addRemoteGit', 'skipping chownr on Windows') - resolveHead(cb) - } else { - getGitDir(function (er, cs) { - if (er) { - log.error('addRemoteGit', 'could not get cache stat') - return cb(er) - } - - chownr(cachedRemote, cs.uid, cs.gid, function (er) { - if (er) { - log.error( - 'addRemoteGit', - 'Failed to change folder ownership under npm cache for', - cachedRemote - ) - return cb(er) - } + if (stderr) stderr = stderr.trim() + if (stderr || er) { + log.warn('addRemoteGit', from, 'resetting remote', cachedRemote, 'because of error:', stderr || er) + return resetRemote(from, cloneURL, treeish, cachedRemote, cb) + } else if (cloneURL !== originURL) { + log.warn( + 'addRemoteGit', + from, + 'pre-existing cached repo', cachedRemote, 'points to', originURL, 'and not', cloneURL + ) + return resetRemote(from, cloneURL, treeish, cachedRemote, cb) + } - log.verbose('addRemoteGit', 'set permissions on', cachedRemote) - resolveHead(cb) - }) - }) + log.verbose('validateExistingRemote', from, 'is updating existing cached remote', cachedRemote) + updateRemote(from, cloneURL, treeish, cachedRemote, cb) } - } + ) +} - // always fetch the origin, even right after mirroring, because this way - // permissions will get set correctly - function updateRemote (cb) { +// make a complete bare mirror of the remote repo +// NOTE: npm uses a blank template directory to prevent weird inconsistencies +// https://github.com/npm/npm/issues/5867 +function mirrorRemote (from, cloneURL, treeish, cachedRemote, silent, cb) { + mkdir(cachedRemote, function (er) { + if (er) return cb(er) + + var args = [ + 'clone', + '--template=' + templates, + '--mirror', + cloneURL, cachedRemote + ] git.whichAndExec( - ['fetch', '-a', 'origin'], + ['clone', '--template=' + templates, '--mirror', cloneURL, cachedRemote], { cwd: cachedRemote, env: gitEnv() }, function (er, stdout, stderr) { if (er) { var combined = (stdout + '\n' + stderr).trim() - log.error('git fetch -a origin (' + gitURL + ')', combined) + var command = 'git ' + args.join(' ') + ':' + if (silent) { + log.verbose(command, combined) + } else { + log.error(command, combined) + } return cb(er) } - log.verbose('addRemoteGit', 'git fetch -a origin (' + gitURL + ')', stdout.trim()) - - setPermissions(cb) + log.verbose('mirrorRemote', from, 'git clone ' + cloneURL, stdout.trim()) + setPermissions(from, cloneURL, treeish, cachedRemote, cb) } ) - } + }) +} - // branches and tags are both symbolic labels that can be attached to different - // commits, so resolve the commitish to the current actual treeish the label - // corresponds to - // - // important for shrinkwrap - function resolveHead (cb) { - log.verbose('addRemoteGit', 'original treeish:', treeish) - var args = ['rev-list', '-n1', treeish] - git.whichAndExec( - args, - { cwd: cachedRemote, env: gitEnv() }, - function (er, stdout, stderr) { +function setPermissions (from, cloneURL, treeish, cachedRemote, cb) { + if (process.platform === 'win32') { + log.verbose('setPermissions', from, 'skipping chownr on Windows') + resolveHead(from, cloneURL, treeish, cachedRemote, cb) + } else { + getGitDir(function (er, cs) { + if (er) { + log.error('setPermissions', from, 'could not get cache stat') + return cb(er) + } + + chownr(cachedRemote, cs.uid, cs.gid, function (er) { if (er) { - log.error('git ' + args.join(' ') + ':', stderr) + log.error( + 'setPermissions', + 'Failed to change git repository ownership under npm cache for', + cachedRemote + ) return cb(er) } - resolvedTreeish = stdout.trim() - log.silly('addRemoteGit', 'resolved treeish:', resolvedTreeish) + log.verbose('setPermissions', from, 'set permissions on', cachedRemote) + resolveHead(from, cloneURL, treeish, cachedRemote, cb) + }) + }) + } +} - resolvedURL = getResolved(originalURL, resolvedTreeish) - log.verbose('addRemoteGit', 'resolved Git URL:', resolvedURL) +// always fetch the origin, even right after mirroring, because this way +// permissions will get set correctly +function updateRemote (from, cloneURL, treeish, cachedRemote, cb) { + git.whichAndExec( + ['fetch', '-a', 'origin'], + { cwd: cachedRemote, env: gitEnv() }, + function (er, stdout, stderr) { + if (er) { + var combined = (stdout + '\n' + stderr).trim() + log.error('git fetch -a origin (' + cloneURL + ')', combined) + return cb(er) + } + log.verbose('updateRemote', 'git fetch -a origin (' + cloneURL + ')', stdout.trim()) - // generate a unique filename - tmpdir = path.join( - npm.tmp, - 'git-cache-' + randomBytes(6).toString('hex'), - resolvedTreeish - ) - log.silly('addRemoteGit', 'Git working directory:', tmpdir) + setPermissions(from, cloneURL, treeish, cachedRemote, cb) + } + ) +} - mkdir(tmpdir, function (er) { - if (er) return cb(er) +// branches and tags are both symbolic labels that can be attached to different +// commits, so resolve the commitish to the current actual treeish the label +// corresponds to +// +// important for shrinkwrap +function resolveHead (from, cloneURL, treeish, cachedRemote, cb) { + log.verbose('resolveHead', from, 'original treeish:', treeish) + var args = ['rev-list', '-n1', treeish] + git.whichAndExec( + args, + { cwd: cachedRemote, env: gitEnv() }, + function (er, stdout, stderr) { + if (er) { + log.error('git ' + args.join(' ') + ':', stderr) + return cb(er) + } - cloneResolved(cb) - }) + var resolvedTreeish = stdout.trim() + log.silly('resolveHead', from, 'resolved treeish:', resolvedTreeish) + + var resolvedURL = getResolved(cloneURL, resolvedTreeish) + log.verbose('resolveHead', from, 'resolved Git URL:', resolvedURL) + + // generate a unique filename + var tmpdir = path.join( + npm.tmp, + 'git-cache-' + crypto.pseudoRandomBytes(6).toString('hex'), + resolvedTreeish + ) + log.silly('resolveHead', 'Git working directory:', tmpdir) + + mkdir(tmpdir, function (er) { + if (er) return cb(er) + + cloneResolved(from, resolvedURL, resolvedTreeish, cachedRemote, tmpdir, cb) + }) + } + ) +} + +// make a clone from the mirrored cache so we have a temporary directory in +// which we can check out the resolved treeish +function cloneResolved (from, resolvedURL, resolvedTreeish, cachedRemote, tmpdir, cb) { + var args = ['clone', cachedRemote, tmpdir] + git.whichAndExec( + args, + { cwd: cachedRemote, env: gitEnv() }, + function (er, stdout, stderr) { + stdout = (stdout + '\n' + stderr).trim() + if (er) { + log.error('git ' + args.join(' ') + ':', stderr) + return cb(er) } - ) - } + log.verbose('cloneResolved', from, 'clone', stdout) - // make a clone from the mirrored cache so we have a temporary directory in - // which we can check out the resolved treeish - function cloneResolved (cb) { - var args = ['clone', cachedRemote, tmpdir] - git.whichAndExec( - args, - { cwd: cachedRemote, env: gitEnv() }, - function (er, stdout, stderr) { - stdout = (stdout + '\n' + stderr).trim() - if (er) { - log.error('git ' + args.join(' ') + ':', stderr) - return cb(er) - } - log.verbose('addRemoteGit', 'clone', stdout) + checkoutTreeish(from, resolvedURL, resolvedTreeish, tmpdir, cb) + } + ) +} - checkoutTreeish(cb) +// there is no safe way to do a one-step clone to a treeish that isn't +// guaranteed to be a branch, so explicitly check out the treeish once it's +// cloned +function checkoutTreeish (from, resolvedURL, resolvedTreeish, tmpdir, cb) { + var args = ['checkout', resolvedTreeish] + git.whichAndExec( + args, + { cwd: tmpdir, env: gitEnv() }, + function (er, stdout, stderr) { + stdout = (stdout + '\n' + stderr).trim() + if (er) { + log.error('git ' + args.join(' ') + ':', stderr) + return cb(er) } - ) - } + log.verbose('checkoutTreeish', from, 'checkout', stdout) - // there is no safe way to do a one-step clone to a treeish that isn't - // guaranteed to be a branch, so explicitly check out the treeish once it's - // cloned - function checkoutTreeish (cb) { - var args = ['checkout', resolvedTreeish] - git.whichAndExec( - args, - { cwd: tmpdir, env: gitEnv() }, - function (er, stdout, stderr) { - stdout = (stdout + '\n' + stderr).trim() + // convince addLocal that the checkout is a local dependency + realizePackageSpecifier(tmpdir, function (er, spec) { if (er) { - log.error('git ' + args.join(' ') + ':', stderr) + log.error('addRemoteGit', 'Failed to map', tmpdir, 'to a package specifier') return cb(er) } - log.verbose('addRemoteGit', 'checkout', stdout) - // convince addLocal that the checkout is a local dependency - realizePackageSpecifier(tmpdir, function (er, spec) { - if (er) { - log.error('addRemoteGit', 'Failed to map', tmpdir, 'to a package specifier') - return cb(er) + // ensure pack logic is applied + // https://github.com/npm/npm/issues/6400 + addLocal(spec, null, function (er, data) { + if (data) { + log.silly('addRemoteGit', 'data._from:', from) + data._from = from + + log.verbose('addRemoteGit', 'data._resolved:', resolvedURL) + data._resolved = resolvedURL } - // ensure pack logic is applied - // https://github.com/npm/npm/issues/6400 - addLocal(spec, null, function (er, data) { - if (data) { - log.verbose('addRemoteGit', 'data._resolved:', resolvedURL) - data._resolved = resolvedURL - - // the spec passed to addLocal is not what the user originally requested, - // so remap - // https://github.com/npm/npm/issues/7121 - if (!data._fromGitHub) { - log.silly('addRemoteGit', 'data._from:', originalURL) - data._from = originalURL - } else { - log.silly('addRemoteGit', 'data._from:', data._from, '(GitHub)') - } - } - - cb(er, data) - }) + cb(er, data) }) - } - ) - } + }) + } + ) } function getGitDir (cb) { @@ -336,7 +391,12 @@ function gitEnv () { // git responds to env vars in some weird ways in post-receive hooks // so don't carry those along. if (gitEnv_) return gitEnv_ - gitEnv_ = {} + + // allow users to override npm's insistence on not prompting for + // passphrases, but default to just failing when credentials + // aren't available + gitEnv_ = { GIT_ASKPASS: 'echo' } + for (var k in process.env) { if (!~VALID_VARIABLES.indexOf(k) && k.match(/^GIT/)) continue gitEnv_[k] = process.env[k] @@ -350,13 +410,12 @@ function getResolved (uri, treeish) { if (!/^git[+:]/.test(parsed.protocol)) { parsed.protocol = 'git+' + parsed.protocol } - var resolved = url.format(parsed) // node incorrectly sticks a / at the start of the path We know that the host // won't change, so split and detect this // https://github.com/npm/npm/issues/3224 var spo = uri.split(parsed.host) - var spr = resolved.split(parsed.host) + var spr = url.format(parsed).split(parsed.host) if (spo[1] && spo[1].charAt(0) === ':' && spr[1] && spr[1].charAt(0) === '/') { spr[1] = spr[1].slice(1) } diff --git a/lib/cache/maybe-github.js b/lib/cache/maybe-github.js deleted file mode 100644 index 60c14a022..000000000 --- a/lib/cache/maybe-github.js +++ /dev/null @@ -1,35 +0,0 @@ -var assert = require('assert') - -var hosted = require('hosted-git-info') -var log = require('npmlog') - -var addRemoteGit = require('./add-remote-git.js') - -module.exports = function maybeGithub (p, cb) { - assert(typeof p === 'string', 'must pass package name') - assert(typeof cb === 'function', 'must pass callback') - - var parsed = hosted.fromUrl(p) - log.info('maybeGithub', 'Attempting %s from %s', p, parsed.git()) - - return addRemoteGit(parsed.git(), true, function (er, data) { - if (er) { - log.info('maybeGithub', "Couldn't clone %s", parsed.git()) - log.info('maybeGithub', 'Now attempting %s from %s', p, parsed.sshurl()) - - return addRemoteGit(parsed.sshurl(), false, function (er, data) { - if (er) return cb(er) - - success(parsed.sshurl(), data) - }) - } - - success(parsed.git(), data) - }) - - function success (u, data) { - data._from = u - data._fromGithub = true - return cb(null, data) - } -} diff --git a/test/tap/add-remote-git-file.js b/test/tap/add-remote-git-file.js index fc102faac..b2a7be903 100644 --- a/test/tap/add-remote-git-file.js +++ b/test/tap/add-remote-git-file.js @@ -28,7 +28,7 @@ test('setup', function (t) { test('cache from repo', function (t) { process.chdir(pkg) var addRemoteGit = require('../../lib/cache/add-remote-git.js') - addRemoteGit(cloneURL, false, function (er, data) { + addRemoteGit(cloneURL, function (er, data) { t.ifError(er, 'cached via git') t.equal( url.parse(data._resolved).protocol, diff --git a/test/tap/git-cache-locking.js b/test/tap/git-cache-locking.js index 39f8b279c..e08c96e1b 100644 --- a/test/tap/git-cache-locking.js +++ b/test/tap/git-cache-locking.js @@ -1,48 +1,46 @@ -var test = require("tap").test - , common = require("../common-tap") - , path = require("path") - , rimraf = require("rimraf") - , mkdirp = require("mkdirp") - , pkg = path.resolve(__dirname, "git-cache-locking") - , tmp = path.join(pkg, "tmp") - , cache = path.join(pkg, "cache") +var test = require('tap').test +var common = require('../common-tap') +var path = require('path') +var rimraf = require('rimraf') +var mkdirp = require('mkdirp') +var pkg = path.resolve(__dirname, 'git-cache-locking') +var tmp = path.join(pkg, 'tmp') +var cache = path.join(pkg, 'cache') - -test("setup", function (t) { +test('setup', function (t) { rimraf.sync(pkg) - mkdirp.sync(path.resolve(pkg, "node_modules")) + mkdirp.sync(path.resolve(pkg, 'node_modules')) t.end() }) -test("git-cache-locking: install a git dependency", function (t) { - +test('git-cache-locking: install a git dependency', function (t) { // disable git integration tests on Travis. if (process.env.TRAVIS) return t.end() // package c depends on a.git#master and b.git#master // package b depends on a.git#master common.npm([ - "install", - "git://github.com/nigelzor/npm-4503-c.git" + 'install', + 'git://github.com/nigelzor/npm-4503-c.git' ], { cwd: pkg, env: { - "npm_config_cache": cache, - "npm_config_tmp": tmp, - "npm_config_prefix": pkg, - "npm_config_global": "false", + npm_config_cache: cache, + npm_config_tmp: tmp, + npm_config_prefix: pkg, + npm_config_global: 'false', HOME: process.env.HOME, Path: process.env.PATH, PATH: process.env.PATH } - }, function (err, code) { - t.ifErr(err, "npm install finished without error") - t.equal(0, code, "npm install should succeed") + }, function (err, code, stdout, stderr) { + t.ifErr(err, 'npm install finished without error') + t.equal(0, code, 'npm install should succeed') t.end() }) }) -test("cleanup", function(t) { +test('cleanup', function (t) { rimraf.sync(pkg) t.end() }) diff --git a/test/tap/github-shortcut.js b/test/tap/github-shortcut.js index accc16f39..e95b8bdfb 100644 --- a/test/tap/github-shortcut.js +++ b/test/tap/github-shortcut.js @@ -4,8 +4,9 @@ var test = require('tap').test test('github-shortcut', function (t) { var cloneUrls = [ - ['git://github.com/foo/private.git', 'github shortcuts try git:// first'], - ['ssh://git@github.com/foo/private.git', 'github shortcuts try ssh:// urls second'] + ['git://github.com/foo/private.git', 'github shortcuts try git URLs first'], + ['git@github.com:foo/private.git', 'github shortcuts try SSH second'], + ['https://github.com/foo/private.git', 'github shortcuts try HTTPS URLs third'] ] var npm = requireInject.installGlobally('../../lib/npm.js', { 'child_process': { diff --git a/test/tap/ls-l-depth-0.js b/test/tap/ls-l-depth-0.js index 09298bdd7..5bbc1278c 100644 --- a/test/tap/ls-l-depth-0.js +++ b/test/tap/ls-l-depth-0.js @@ -19,7 +19,7 @@ var expected = '│ \n' + '└── glock@1.8.7\n' + ' an inexplicably hostile sample package\n' + - ' https://github.com/npm/glo.ck\n' + + ' git+https://github.com/npm/glo.ck.git\n' + ' https://glo.ck\n' + '\n' diff --git a/test/tap/maybe-github.js b/test/tap/maybe-github.js deleted file mode 100644 index 52a62e11b..000000000 --- a/test/tap/maybe-github.js +++ /dev/null @@ -1,72 +0,0 @@ -require("../common-tap.js") -var test = require("tap").test -var npm = require("../../lib/npm.js") - -// this is the narrowest way to replace a function in the module cache -var found = true -var remoteGitPath = require.resolve("../../lib/cache/add-remote-git.js") -require("module")._cache[remoteGitPath] = { - id: remoteGitPath, - exports: function stub(_, __, cb) { - if (found) { - cb(null, {}) - } - else { - cb(new Error("not on filesystem")) - } - } -} - -// only load maybeGithub now, so it gets the stub from cache -var maybeGithub = require("../../lib/cache/maybe-github.js") - -test("should throw with no parameters", function (t) { - t.plan(1) - - t.throws(function () { - maybeGithub() - }, "throws when called without parameters") -}) - -test("should throw with wrong parameter types", function (t) { - t.plan(2) - - t.throws(function () { - maybeGithub({}, function () {}) - }, "expects only a package name") - - t.throws(function () { - maybeGithub("npm/xxx-noexist", "ham") - }, "is always async") -}) - -test("should find an existing package on Github", function (t) { - found = true - npm.load({}, function (error) { - t.notOk(error, "bootstrapping succeeds") - t.doesNotThrow(function () { - maybeGithub("npm/npm", function (error, data) { - t.notOk(error, "no issues in looking things up") - t.ok(data, "received metadata from Github") - t.end() - }) - }) - }) -}) - -test("shouldn't find a nonexistent package on Github", function (t) { - found = false - npm.load({}, function () { - t.doesNotThrow(function () { - maybeGithub("npm/xxx-noexist", function (error, data) { - t.equal( - error.message, - "not on filesystem", - "passed through original error message" - ) - t.notOk(data, "didn't pass any metadata") - t.end() - }) - }) - }) -}) diff --git a/test/tap/outdated-git.js b/test/tap/outdated-git.js index 32bfead67..0ef243bee 100644 --- a/test/tap/outdated-git.js +++ b/test/tap/outdated-git.js @@ -11,6 +11,18 @@ var npm = require('../../') // config var pkg = path.resolve(__dirname, 'outdated-git') var cache = path.resolve(pkg, 'cache') +var json = { + name: 'outdated-git', + author: 'Rocko Artischocko', + description: 'fixture', + version: '0.0.1', + main: 'index.js', + dependencies: { + 'foo-private': 'git://github.com/robertkowalski/foo-private.git', + 'foo-private-credentials': 'git://user:pass@github.com/robertkowalski/foo-private.git', + 'foo-github': 'robertkowalski/foo' + } +} test('setup', function (t) { setup() @@ -22,11 +34,11 @@ test('discovers new versions in outdated', function (t) { t.plan(5) npm.load({cache: cache, registry: common.registry, loglevel: 'silent'}, function () { npm.commands.outdated([], function (er, d) { - t.equal('git', d[0][3]) - t.equal('git', d[0][4]) - t.equal('git://github.com/robertkowalski/foo-private.git', d[0][5]) - t.equal('git://user:pass@github.com/robertkowalski/foo-private.git', d[1][5]) - t.equal('git+https://github.com/robertkowalski/foo', d[2][5]) + t.equal(d[0][3], 'git') + t.equal(d[0][4], 'git') + t.equal(d[0][5], 'git://github.com/robertkowalski/foo-private.git') + t.equal(d[1][5], 'git://user:pass@github.com/robertkowalski/foo-private.git') + t.equal(d[2][5], 'github:robertkowalski/foo') }) }) }) @@ -36,19 +48,6 @@ test('cleanup', function (t) { t.end() }) -var json = { - name: 'outdated-git', - author: 'Rocko Artischocko', - description: 'fixture', - version: '0.0.1', - main: 'index.js', - dependencies: { - 'foo-private': 'git://github.com/robertkowalski/foo-private.git', - 'foo-private-credentials': 'git://user:pass@github.com/robertkowalski/foo-private.git', - 'foo-github': 'robertkowalski/foo' - } -} - function setup () { mkdirp.sync(cache) fs.writeFileSync(path.join(pkg, 'package.json'), JSON.stringify(json, null, 2), 'utf8') |