From 7925b64f9119ce446da13aba5b2b009db08b09e2 Mon Sep 17 00:00:00 2001 From: zkochan Date: Sat, 17 Sep 2016 19:00:02 +0300 Subject: [PATCH] fix(install): GitHub API rate limit issue GitHub API usage removed. The tarball is used from codeload.github.com Close #361, PR #363 BREAKING CHANGE: The folder names of packages fetched from GitHub changed to not contain information that would require an API request to get. Old folder name: is-negative@github+kevva+is-negative+HEAD New folder name: github+kevva+is-negative+HEAD Users are forced to redownload their stores to avoid orphan folders after the naming change. --- package.json | 2 +- src/api/initCmd.ts | 28 +++++++++++++++----- src/resolve/github.ts | 61 ++++++------------------------------------- test/index.ts | 28 +++++++------------- 4 files changed, 41 insertions(+), 78 deletions(-) diff --git a/package.json b/package.json index d8d16fb9bb..32e0715620 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "pnpm", "description": "A fast implementation of npm install", - "version": "0.32.1", + "version": "0.33.0", "author": "Rico Sta. Cruz ", "bin": { "pnpm": "lib/bin/pnpm.js" diff --git a/src/api/initCmd.ts b/src/api/initCmd.ts index 962ed19833..c8c3313c87 100644 --- a/src/api/initCmd.ts +++ b/src/api/initCmd.ts @@ -5,6 +5,7 @@ import lockfile = require('lockfile') const lock = thenify(lockfile.lock) const unlock = thenify(lockfile.unlock) import semver = require('semver') +import {stripIndent} from 'common-tags' import requireJson from '../fs/requireJson' import writeJson from '../fs/writeJson' import expandTilde from '../fs/expandTilde' @@ -104,13 +105,28 @@ export default async function (opts: StrictBasicOptions): Promise=0.28')) { - throw new Error(`The store structure was changed. - Remove it and run pnpm again. - More info about what was changed at: https://github.com/rstacruz/pnpm/issues/276 - TIPS: - If you have a shared store, remove both the node_modules and the shared shore. - Otherwise just run \`rm -rf node_modules\``) + const msg = structureChangeMsg('More info about what was changed at: https://github.com/rstacruz/pnpm/issues/276') + throw new Error(msg) } + if (!semver.satisfies(storeVersion, '>=0.33')) { + const msg = structureChangeMsg(stripIndent` + The change was needed to fix the GitHub rate limit issue: + Issue: https://github.com/rstacruz/pnpm/issues/361 + PR: https://github.com/rstacruz/pnpm/pull/363 + `) + throw new Error(msg) + } +} + +function structureChangeMsg (moreInfo: string): string { + return stripIndent` + The store structure was changed. + Remove it and run pnpm again. + ${moreInfo} + TIPS: + If you have a shared store, remove both the node_modules and the shared shore. + Otherwise just run \`rm -rf node_modules\` + ` } async function readGlobalPkg (globalPath: string) { diff --git a/src/resolve/github.ts b/src/resolve/github.ts index 75ac92df36..9d6bd33e16 100644 --- a/src/resolve/github.ts +++ b/src/resolve/github.ts @@ -1,71 +1,26 @@ -import pkgFullName, {delimiter} from '../pkgFullName' +import {delimiter} from '../pkgFullName' import {HostedPackageSpec, ResolveOptions, ResolveResult} from '.' -import {Package} from '../api/initCmd' /** * Resolves a 'hosted' package hosted on 'github'. */ -const PARSE_GITHUB_RE = /^github:([^\/]+)\/([^#]+)(#(.+))?$/ - export default async function resolveGithub (spec: HostedPackageSpec, opts: ResolveOptions): Promise { - const getJSON = opts.got.getJSON const ghSpec = parseGithubSpec(spec) - ghSpec.ref = await resolveRef(ghSpec) - const resPkg: Package = await resolvePackageJson(ghSpec) return { - fullname: pkgFullName({ - name: resPkg.name, - version: ['github', ghSpec.owner, ghSpec.repo, ghSpec.ref].join(delimiter) - }), + fullname: ['github', ghSpec.owner, ghSpec.repo, ghSpec.ref].join(delimiter), dist: { location: 'remote', - tarball: [ - 'https://api.github.com/repos', - ghSpec.owner, - ghSpec.repo, - 'tarball', - ghSpec.ref - ].join('/') + tarball: `https://codeload.github.com/${ghSpec.owner}/${ghSpec.repo}/tar.gz/${ghSpec.ref}` } } - - type GitHubContentResponse = { - content: string - } - - async function resolvePackageJson (spec: GitHubSpec) { - const url = [ - 'https://api.github.com/repos', - spec.owner, - spec.repo, - 'contents/package.json?ref=' + spec.ref - ].join('/') - const body = await getJSON(url) - const content = new Buffer(body.content, 'base64').toString('utf8') - return JSON.parse(content) - } - - type GitHubRepoResponse = { - sha: string - } - - async function resolveRef (spec: GitHubSpec) { - const url = [ - 'https://api.github.com/repos', - spec.owner, - spec.repo, - 'commits', - spec.ref - ].join('/') - const body = await getJSON(url) - return body.sha - } } -function parseGithubSpec (pkg: HostedPackageSpec): GitHubSpec { - const m = PARSE_GITHUB_RE.exec(pkg.hosted.shortcut) +const PARSE_GITHUB_RE = /^github:([^\/]+)\/([^#]+)(#(.+))?$/ + +function parseGithubSpec (spec: HostedPackageSpec): GitHubSpec { + const m = PARSE_GITHUB_RE.exec(spec.hosted.shortcut) if (!m) { - throw new Error('cannot parse: ' + pkg.hosted.shortcut) + throw new Error('cannot parse: ' + spec.hosted.shortcut) } const owner = m[1] const repo = m[2] diff --git a/test/index.ts b/test/index.ts index 9eea9d8297..79144de36a 100644 --- a/test/index.ts +++ b/test/index.ts @@ -285,22 +285,19 @@ test('nested local dependency of a local dependency', t => { .catch(t.end) }) -// Skipping on CI as failing frequently there, due to environment issues -if (!process.env.CI) { - test('from a github repo', t => { - prepare() - install(['kevva/is-negative'], { quiet: true }) - .then(() => { - const localPkg = require( - path.join(process.cwd(), 'node_modules', 'is-negative')) +test('from a github repo', t => { + prepare() + install(['kevva/is-negative'], { quiet: true }) + .then(() => { + const localPkg = require( + path.join(process.cwd(), 'node_modules', 'is-negative')) - t.ok(localPkg, 'isNegative() is available') + t.ok(localPkg, 'isNegative() is available') - t.end() - }) - .catch(t.end) + t.end() }) -} + .catch(t.end) +}) test('shrinkwrap compatibility', t => { prepare({ dependencies: { rimraf: '*' } }) @@ -770,11 +767,6 @@ test('tarball local package', t => { }) test("don't fail when peer dependency is fetched from GitHub", t => { - if (process.env.CI) { - t.skip('cannot run on CI because of exceeding rate limit') - return t.end() - } - prepare() install([local('test-pnpm-peer-deps')], { quiet: true }) .then(() => t.end())