feat: instead of detecting firstPush, we now look for a gk-lockfile c…

…ommit on the branch

ci-services/bitrise.js

@@ -1,5 +1,3 @@
-const gitHelpers = require('../lib/git-helpers')
-
 const env = process.env
 
 // http://devcenter.bitrise.io/faq/available-environment-variables/
@@ -19,9 +17,6 @@ module.exports = {
   repoSlug: parseRepoSlug(env.GIT_REPOSITORY_URL),
   // The name of the current branch
   branchName: env.BITRISE_GIT_BRANCH,
-  // Is this the first push on this branch
-  // i.e. the Greenkeeper commit
-  firstPush: gitHelpers.getNumberOfCommitsOnBranch(env.BITRISE_GIT_BRANCH) === 1,
   // Is this a regular build
   correctBuild: env.PR === 'false',
   // Should the lockfile be uploaded from this build

ci-services/buildkite.js

@@ -7,7 +7,6 @@ const env = process.env
 module.exports = {
   repoSlug: gitHelpers.getRepoSlug(env.BUILDKITE_REPO),
   branchName: env.BUILDKITE_BRANCH,
-  firstPush: gitHelpers.getNumberOfCommitsOnBranch(env.BUILDKITE_BRANCH) === 1,
   correctBuild: env.BUILDKITE_PULL_REQUEST === 'false',
   uploadBuild: true
 }

ci-services/circleci.js

@@ -7,7 +7,6 @@ const env = process.env
 module.exports = {
   repoSlug: `${env.CIRCLE_PROJECT_USERNAME}/${env.CIRCLE_PROJECT_REPONAME}`,
   branchName: env.CIRCLE_BRANCH,
-  firstPush: !env.CIRCLE_PREVIOUS_BUILD_NUM,
   correctBuild: _.isEmpty(env.CI_PULL_REQUEST),
   uploadBuild: env.CIRCLE_NODE_INDEX === `${env.BUILD_LEADER_ID || 0}`
 }

ci-services/codeship.js

@@ -40,9 +40,6 @@ module.exports = {
   repoSlug: getRepoSlug(),
   // The name of the current branch
   branchName: env.CI_BRANCH,
-  // Is this the first push on this branch
-  // i.e. the Greenkeeper commit
-  firstPush: shouldUpdate(),
   // Is this a regular build (use tag: ^greenkeeper/)
   correctBuild: shouldUpdate(),
   // Should the lockfile be uploaded from this build (use tag: ^greenkeeper/)

ci-services/jenkins.js

@@ -3,12 +3,10 @@
 const env = process.env
 
 const _ = require('lodash')
-const gitHelpers = require('../lib/git-helpers')
 
 module.exports = {
   gitUrl: env.GIT_URL,
   branchName: _.drop(_.split(env.GIT_BRANCH, '/')).join('/'),
-  firstPush: env.BUILD_NUMBER === '1' || gitHelpers.getNumberOfCommitsOnBranch(env.GIT_BRANCH) === 1,
   correctBuild: true, // assuming pull requests are not build
   uploadBuild: true // assuming 1 build per branch
 }

ci-services/semaphoreci.js

@@ -7,7 +7,6 @@ const env = process.env
 module.exports = {
   repoSlug: env.SEMAPHORE_REPO_SLUG,
   branchName: env.BRANCH_NAME,
-  firstPush: env.SEMAPHORE_BUILD_NUMBER === '1',
   correctBuild: _.isEmpty(env.PULL_REQUEST_NUMBER),
   uploadBuild: env.SEMAPHORE_CURRENT_JOB === '1'
 }

ci-services/teamcity.js

@@ -29,7 +29,6 @@ function shouldUpload () {
 module.exports = {
   repoSlug: gitHelpers.getRepoSlug(env.VCS_ROOT_URL),
   branchName: env.VCS_ROOT_BRANCH,
-  firstPush: shouldUpload(),
   correctBuild: !isPullRequest(),
   uploadBuild: shouldUpload()
 }

ci-services/travis.js

@@ -7,9 +7,6 @@ module.exports = {
   repoSlug: env.TRAVIS_REPO_SLUG,
   // The name of the current branch
   branchName: env.TRAVIS_BRANCH,
-  // Is this the first push on this branch
-  // i.e. the Greenkeeper commit
-  firstPush: !env.TRAVIS_COMMIT_RANGE,
   // Is this a regular build
   correctBuild: env.TRAVIS_PULL_REQUEST === 'false',
   // Should the lockfile be uploaded from this build

ci-services/wercker.js

@@ -1,13 +1,10 @@
 'use strict'
 
-const gitHelpers = require('../lib/git-helpers')
-
 const env = process.env
 
 module.exports = {
   repoSlug: `${env.WERCKER_GIT_OWNER}/${env.WERCKER_GIT_REPOSITORY}`,
   branchName: env.WERCKER_GIT_BRANCH,
-  firstPush: gitHelpers.getNumberOfCommitsOnBranch(env.WERCKER_GIT_BRANCH) === 1,
   correctBuild: env.WERCKER_GIT_DOMAIN === 'github.com',
 
   // In wercker, only add the upload step to the pipeline you'd want to upload from

lib/git-helpers.js

@@ -23,5 +23,15 @@ module.exports = {
     return (
       `${parsed[1]}/${parsed[2]}`
     )
+  },
+  hasLockfileCommit: function hasLockfileCommit (info) {
+    // TODO: this assumes the GitHub default branch is `master`
+    //       we may have to make this a config option for folks that use a different branch name
+    //       as git doesn’t track where a branch is forked from, and while we do have access to
+    //       GitHub here, we don’t have any GitHub API calls here yet, so it might be easier
+    //       to make this a config option. This is going to be a semver-major update anyway,
+    //       so we have a chance for good documentation
+    const hasCommit = _.trim(exec(`git log --oneline master...${info.branchName} | grep 'chore(package): update lockfile'`))
+    return hasCommit !== ''
   }
 }

lib/update-lockfile.js

@@ -40,7 +40,6 @@ module.exports.updateLockfile = function updateLockfile (dependency, options) {
         exec('npm5 -v')
         npmBin = 'npm5'
       } catch (err) {}
-
       exec(`${npmBin} install${args}`)
     }
   }

test/update.js

@@ -35,68 +35,68 @@ test('monorepo: root package', () => {
   prepare()
   expect.assertions(2)
   runUpdateInSubdir('fixtures/root-package')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(12)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(13)
 })
 
 test('monorepo: no package.json', () => {
   prepare()
   expect.assertions(1)
   runUpdateInSubdir('fixtures/no-package')
-  expect(exec.callCount).toEqual(0)
+  expect(exec.callCount).toEqual(1)
 })
 
 test('monorepo: root and one sub package', () => {
   prepare()
   expect.assertions(3)
   runUpdateInSubdir('fixtures/root-and-one-sub')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(9).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(17)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(10).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(18)
 })
 
 test('monorepo: root and two sub package', () => {
   prepare()
   expect.assertions(4)
   runUpdateInSubdir('fixtures/root-and-two-sub')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(9).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(14).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(22)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(10).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(15).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(23)
 })
 
 test('monorepo: root and two sub package at different levels', () => {
   prepare()
   expect.assertions(4)
   runUpdateInSubdir('fixtures/root-and-two-diff-sub')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(9).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(14).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(22)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(10).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(15).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(23)
 })
 
 test('monorepo: no root and one sub package', () => {
   prepare()
   expect.assertions(2)
   runUpdateInSubdir('fixtures/no-root-and-one-sub')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(12)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(13)
 })
 
 test('monorepo: no root and two sub package', () => {
   prepare()
   expect.assertions(3)
   runUpdateInSubdir('fixtures/no-root-and-two-sub')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(9).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(17)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(10).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(18)
 })
 
 test('monorepo: no root and two sub package at different levels', () => {
   prepare()
   expect.assertions(3)
   runUpdateInSubdir('fixtures/no-root-and-two-diff-sub')
-  expect(exec.getCall(4).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.getCall(9).calledWith('npm install -S [email protected]')).toBeTruthy()
-  expect(exec.callCount).toEqual(17)
+  expect(exec.getCall(5).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.getCall(10).calledWith('npm install -S [email protected]')).toBeTruthy()
+  expect(exec.callCount).toEqual(18)
 })

update.js

@@ -11,6 +11,7 @@ const fg = require('fast-glob')
 
 const config = require('./lib/config')
 const extractDependency = require('./lib/extract-dependency')
+const hasLockfileCommit = require('./lib/git-helpers').hasLockfileCommit
 
 const lockfile = require('./lib/update-lockfile')
 const updateLockfile = lockfile.updateLockfile
@@ -30,10 +31,6 @@ module.exports = function update () {
     return console.error(`'${info.branchName}' is not a Greenkeeper branch`)
   }
 
-  if (!info.firstPush) {
-    return console.error('Only running on first push of a new branch')
-  }
-
   if (!info.correctBuild) {
     return console.error('This build should not update the lockfile. It could be a PR, not a branch build.')
   }
@@ -42,6 +39,10 @@ module.exports = function update () {
     return console.error('No branch details set, so assuming not a Greenkeeper branch')
   }
 
+  if (hasLockfileCommit(info)) {
+    return console.error('greenkeeper-lockfile already has a commit on this branch')
+  }
+
   const allPackageFiles = fg.sync('./**/package.json')
   const doCommit = allPackageFiles.reduce((didChange, pkgJson) => {
     const lockfilePath = path.dirname(pkgJson)

upload.js

@@ -8,6 +8,7 @@ const fs = require('fs')
 
 const config = require('./lib/config')
 const info = require('./ci-services')()
+const hasLockfileCommit = require('./lib/git-helpers').hasLockfileCommit
 
 const env = process.env
 
@@ -32,14 +33,14 @@ module.exports = function upload () {
     return console.error('Not running on the initial Greenkeeper branch. Will only run on Greenkeeper branches that update a specific dependency')
   }
 
-  if (!info.firstPush) {
-    return console.error('Only running on first push of a new branch')
-  }
-
   if (!info.uploadBuild) {
     return console.error('Only uploading on one build job')
   }
 
+  if (hasLockfileCommit(info)) {
+    return console.error('greenkeeper-lockfile already has a commit on this branch')
+  }
+
   let remote = `[email protected]:${info.repoSlug}`
   if (info.gitUrl) remote = info.gitUrl