From 4a5183fc2bb5c42f3a4731bac04df18b12071e88 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 2 Nov 2016 18:59:04 -0400 Subject: [PATCH 1/6] scripts: new attempt-backport script for PRs Refs: https://github.com/nodejs/github-bot/issues/77 PR-URL: https://github.com/nodejs/github-bot/pull/90 --- lib/node-repo.js | 1 + scripts/attempt-backport.js | 205 ++++++++++++++++++++++++++++++++++++ server.js | 15 +++ 3 files changed, 221 insertions(+) create mode 100644 scripts/attempt-backport.js diff --git a/lib/node-repo.js b/lib/node-repo.js index 6be92294..82478b9c 100644 --- a/lib/node-repo.js +++ b/lib/node-repo.js @@ -93,6 +93,7 @@ function itemsInCommon (arr1, arr2) { return arr1.filter((item) => arr2.indexOf(item) !== -1) } +exports.updatePrWithLabels = updatePrWithLabels exports.resolveLabelsThenUpdatePr = deferredResolveLabelsThenUpdatePr // exposed for testability diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js new file mode 100644 index 00000000..32fe2f8c --- /dev/null +++ b/scripts/attempt-backport.js @@ -0,0 +1,205 @@ +'use strict' + +const child_process = require('child_process') +const debug = require('debug')('attempt-backport') +const request = require('request') +const updatePrWithLabels = require('../lib/node-repo').updatePrWithLabels + +const enabledRepos = ['node'] +const queue = [] +let inProgress = false + +module.exports = function (app) { + if (!global._node_repo_dir) return + + app.on('pull_request.opened', handlePrUpdate) + // Pull Request updates + app.on('pull_request.synchronize', handlePrUpdate) + + function handlePrUpdate (event, owner, repo) { + if (!~enabledRepos.indexOf(repo)) return + + const prId = event.number + const options = { owner, repo, prId, logger: event.logger } + + debug(`/${owner}/${repo}/pull/${prId} sync`) + queueAttemptBackport(options, 7, false) + queueAttemptBackport(options, 6, true) + queueAttemptBackport(options, 4, true) + + if (!inProgress) processNextBackport() + } + + // to trigger polling manually + app.get('/pr/:owner/:repo/:id', (req, res) => { + const owner = req.params.owner + const repo = req.params.repo + const prId = parseInt(req.params.id, 10) + const options = { owner, repo, prId, logger: req.log } + + if (~enabledRepos.indexOf(repo)) { + queueAttemptBackport(options, 7, false) + queueAttemptBackport(options, 6, true) + queueAttemptBackport(options, 4, true) + } + + if (!inProgress) processNextBackport() + + res.end() + }) +} + +function processNextBackport() { + const item = queue.shift() + if (!item) return + + if (typeof item !== 'function') { + debug(`item was not a function! - queue size: ${queue.length}`) + return + } else if (inProgress) { + debug(`was still in progress! - queue size: ${queue.length}`) + return + } + item() +} + +function queueAttemptBackport(options, version, isLTS) { + queue.push(function() { + debug(`processing a new backport to v${version}`) + attemptBackport(options, version, isLTS, processNextBackport) + }) +} + +function attemptBackport(options, version, isLTS, cb) { + // Start + gitAmAbort() + + function wrapCP(cmd, args, opts, callback) { + let exited = false + + if (arguments.length === 3) { + callback = opts + opts = {} + } + + opts.cwd = global._node_repo_dir + + const cp = child_process.spawn(cmd, args, opts) + const argsString = [cmd, ...args].join(' ') + + cp.on('error', function(err) { + debug(`child_process err: ${err}`) + if (!exited) onError() + }) + cp.on('exit', function(code) { + exited = true + if (!cb) { + debug(`error before exit, code: ${code}, on '${argsString}'`) + return + } else if (code > 0) { + debug(`exit code > 0: ${code}, on '${argsString}'`) + onError() + return + } + callback() + }) + // Useful when debugging. + // + // cp.stdout.on('data', (data) => { + // console.log(data.toString()) + // }) + // cp.stderr.on('data', (data) => { + // console.log(data.toString()) + // }) + + return cp + } + + function onError() { + if (!cb) return + const _cb = cb + setImmediate(() => { + debug(`backport to ${version} failed`) + if (!isLTS) updatePrWithLabels(options, [`dont-land-on-v${version}.x`]) + setImmediate(() => { + inProgress = false + _cb() + }) + }) + cb = null + } + + function gitAmAbort() { + // TODO(Fishrock123): this should probably just merge into wrapCP + let exited = false + debug(`aborting any previous backport attempt...`) + + const cp = child_process.spawn('git', ['am', '--abort'], { cwd: global._node_repo_dir }) + const argsString = 'git am --abort' + + cp.on('error', function(err) { + debug(`child_process err: ${err}`) + if (!exited) onError() + }) + cp.on('exit', function() { + exited = true + if (!cb) { + debug(`error before exit, code: ${code}, on '${argsString}'`) + return + } + gitRemoteUpdate() + }) + } + + function gitRemoteUpdate() { + debug(`updating git remotes...`) + wrapCP('git', ['remote', 'update', '-p'], gitCheckout) + } + + function gitCheckout() { + debug(`checking out upstream/v${version}.x-staging...`) + wrapCP('git', ['checkout', `upstream/v${version}.x-staging`], gitReset) + } + + function gitReset() { + debug(`resetting upstream/v${version}.x-staging...`) + wrapCP('git', ['reset', `upstream/v${version}.x-staging`, '--hard'], fetchDiff) + } + + function fetchDiff() { + debug(`fetching diff from pr ${options.prId}...`) + + const url = `https://patch-diff.githubusercontent.com/raw/${options.owner}/${options.repo}/pull/${options.prId}.patch` + + const req = request(url) + + req.on('error', function(err) { + debug(`request err: ${err}`) + return onError() + }) + req.on('response', function(response) { + if (response.statusCode !== 200) { + debug(`request non-200 status: ${response.statusCode}`) + return onError() + } + }) + + gitAttemptBackport(req) + } + + function gitAttemptBackport(req) { + debug(`attempting a backport to v${version}...`) + const cp = wrapCP('git', ['am'], { stdio: 'pipe' }, function done() { + // Success! + if (isLTS) updatePrWithLabels(options, [`lts-watch-v${version}.x`]) + + setImmediate(() => { + debug(`backport to v${version} successful`) + inProgress = false + cb() + }) + }) + + req.pipe(cp.stdin) + } +} diff --git a/server.js b/server.js index f7676129..1790f35d 100644 --- a/server.js +++ b/server.js @@ -1,5 +1,20 @@ 'use strict' +const child_process = require('child_process') + +if (process.env.NODE_REPO_DIR) { + const fs = require('fs') + global._node_repo_dir = fs.realpathSync(process.env.NODE_REPO_DIR) + const out = child_process.spawnSync('git', ['status'], { cwd: global._node_repo_dir }) + + if (out.status !== 0) { + logger.info(out.stdout) + logger.error(out.stderr) + logger.error('Bad NODE_REPO_DIR. Backport patch testing disabled.') + global._node_repo_dir = false + } +} + const app = require('./app') const logger = require('./lib/logger') From eaa098c36b5a2ef7f123d47f7569493e304e2059 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Thu, 3 Nov 2016 15:30:27 -0400 Subject: [PATCH 2/6] attempt-backport: only run PRs targeted at master Refs: https://github.com/nodejs/github-bot/issues/77 PR-URL: https://github.com/nodejs/github-bot/pull/90 --- scripts/attempt-backport.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index 32fe2f8c..b937c966 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -19,6 +19,8 @@ module.exports = function (app) { function handlePrUpdate (event, owner, repo) { if (!~enabledRepos.indexOf(repo)) return + if (event.pull_request.base.ref !== 'master') return + const prId = event.number const options = { owner, repo, prId, logger: event.logger } @@ -31,7 +33,7 @@ module.exports = function (app) { } // to trigger polling manually - app.get('/pr/:owner/:repo/:id', (req, res) => { + app.get('/attempt-backport/pr/:owner/:repo/:id', (req, res) => { const owner = req.params.owner const repo = req.params.repo const prId = parseInt(req.params.id, 10) From e54bc7b45bef4f0f38c677982bb0721219475ca9 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 4 Nov 2016 13:13:18 -0400 Subject: [PATCH 3/6] attempt-backport: use options.logger Refs: https://github.com/nodejs/github-bot/issues/77 PR-URL: https://github.com/nodejs/github-bot/pull/90 --- scripts/attempt-backport.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index b937c966..d1ab5125 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -67,7 +67,7 @@ function processNextBackport() { function queueAttemptBackport(options, version, isLTS) { queue.push(function() { - debug(`processing a new backport to v${version}`) + options.logger.debug(`processing a new backport to v${version}`) attemptBackport(options, version, isLTS, processNextBackport) }) } @@ -121,7 +121,7 @@ function attemptBackport(options, version, isLTS, cb) { if (!cb) return const _cb = cb setImmediate(() => { - debug(`backport to ${version} failed`) + options.logger.debug(`backport to ${version} failed`) if (!isLTS) updatePrWithLabels(options, [`dont-land-on-v${version}.x`]) setImmediate(() => { inProgress = false @@ -134,7 +134,7 @@ function attemptBackport(options, version, isLTS, cb) { function gitAmAbort() { // TODO(Fishrock123): this should probably just merge into wrapCP let exited = false - debug(`aborting any previous backport attempt...`) + options.logger.debug(`aborting any previous backport attempt...`) const cp = child_process.spawn('git', ['am', '--abort'], { cwd: global._node_repo_dir }) const argsString = 'git am --abort' @@ -154,22 +154,22 @@ function attemptBackport(options, version, isLTS, cb) { } function gitRemoteUpdate() { - debug(`updating git remotes...`) + options.logger.debug(`updating git remotes...`) wrapCP('git', ['remote', 'update', '-p'], gitCheckout) } function gitCheckout() { - debug(`checking out upstream/v${version}.x-staging...`) + options.logger.debug(`checking out upstream/v${version}.x-staging...`) wrapCP('git', ['checkout', `upstream/v${version}.x-staging`], gitReset) } function gitReset() { - debug(`resetting upstream/v${version}.x-staging...`) + options.logger.debug(`resetting upstream/v${version}.x-staging...`) wrapCP('git', ['reset', `upstream/v${version}.x-staging`, '--hard'], fetchDiff) } function fetchDiff() { - debug(`fetching diff from pr ${options.prId}...`) + options.logger.debug(`fetching diff from pr ${options.prId}...`) const url = `https://patch-diff.githubusercontent.com/raw/${options.owner}/${options.repo}/pull/${options.prId}.patch` @@ -190,13 +190,13 @@ function attemptBackport(options, version, isLTS, cb) { } function gitAttemptBackport(req) { - debug(`attempting a backport to v${version}...`) + options.logger.debug(`attempting a backport to v${version}...`) const cp = wrapCP('git', ['am'], { stdio: 'pipe' }, function done() { // Success! if (isLTS) updatePrWithLabels(options, [`lts-watch-v${version}.x`]) setImmediate(() => { - debug(`backport to v${version} successful`) + options.logger.debug(`backport to v${version} successful`) inProgress = false cb() }) From 323215e92b75e472f5a9e9fb6bf0d417fa3e817b Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Fri, 4 Nov 2016 13:13:41 -0400 Subject: [PATCH 4/6] attempt-backport: enable label removal for Current lines Refs: https://github.com/nodejs/github-bot/issues/77 PR-URL: https://github.com/nodejs/github-bot/pull/90 --- lib/node-repo.js | 25 +++++++++++++++++++++++++ scripts/attempt-backport.js | 10 ++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/node-repo.js b/lib/node-repo.js index 82478b9c..1707c03d 100644 --- a/lib/node-repo.js +++ b/lib/node-repo.js @@ -61,6 +61,30 @@ function updatePrWithLabels (options, labels) { }) } +function removeLabelFromPR (options, label) { + // no need to request github if we didn't resolve a label + if (!label) { + return + } + + options.logger.debug('Trying to remove label: ' + label) + + githubClient.issues.removeLabel({ + user: options.owner, + repo: options.repo, + number: options.prId, + name: label + }, (err) => { + if (err) { + if (err.code === 404) return options.logger.info('Label to remove did not exist, bailing ' + label) + + return options.logger.error(err, 'Error while removing a label') + } + + options.logger.info('Removed a label ' + label) + }) +} + function fetchExistingLabels (options, cb) { const cacheKey = `${options.owner}:${options.repo}` @@ -93,6 +117,7 @@ function itemsInCommon (arr1, arr2) { return arr1.filter((item) => arr2.indexOf(item) !== -1) } +exports.removeLabelFromPR = removeLabelFromPR exports.updatePrWithLabels = updatePrWithLabels exports.resolveLabelsThenUpdatePr = deferredResolveLabelsThenUpdatePr diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index d1ab5125..1fca9137 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -3,7 +3,9 @@ const child_process = require('child_process') const debug = require('debug')('attempt-backport') const request = require('request') -const updatePrWithLabels = require('../lib/node-repo').updatePrWithLabels +const node_repo = require('../lib/node-repo') +const updatePrWithLabels = node_repo.updatePrWithLabels +const removeLabelFromPR = node_repo.removeLabelFromPR const enabledRepos = ['node'] const queue = [] @@ -193,7 +195,11 @@ function attemptBackport(options, version, isLTS, cb) { options.logger.debug(`attempting a backport to v${version}...`) const cp = wrapCP('git', ['am'], { stdio: 'pipe' }, function done() { // Success! - if (isLTS) updatePrWithLabels(options, [`lts-watch-v${version}.x`]) + if (isLTS) { + updatePrWithLabels(options, [`lts-watch-v${version}.x`]) + } else { + removeLabelFromPR(options, `dont-land-on-v${version}.x`) + } setImmediate(() => { options.logger.debug(`backport to v${version} successful`) From 4df276e8d7bb7c1b1ed3569be3ef537e78d01bc7 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 16 Nov 2016 14:33:29 -0500 Subject: [PATCH 5/6] attempt-backport: disable label removal for now Refs: https://github.com/nodejs/github-bot/issues/77 PR-URL: https://github.com/nodejs/github-bot/pull/90 --- scripts/attempt-backport.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index 1fca9137..a78b90ca 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -5,7 +5,7 @@ const debug = require('debug')('attempt-backport') const request = require('request') const node_repo = require('../lib/node-repo') const updatePrWithLabels = node_repo.updatePrWithLabels -const removeLabelFromPR = node_repo.removeLabelFromPR +// const removeLabelFromPR = node_repo.removeLabelFromPR const enabledRepos = ['node'] const queue = [] @@ -197,9 +197,11 @@ function attemptBackport(options, version, isLTS, cb) { // Success! if (isLTS) { updatePrWithLabels(options, [`lts-watch-v${version}.x`]) - } else { - removeLabelFromPR(options, `dont-land-on-v${version}.x`) - } + }// else { + // TODO(Fishrock123): Re-enable this, but do a check first + // to make sure the label was set by the bot only. + // removeLabelFromPR(options, `dont-land-on-v${version}.x`) + //} setImmediate(() => { options.logger.debug(`backport to v${version} successful`) From 5ed1046de17f9ac449343ceed60e985d9ae2c371 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Wed, 16 Nov 2016 14:36:22 -0500 Subject: [PATCH 6/6] attempt-backport: fix lint Refs: https://github.com/nodejs/github-bot/issues/77 PR-URL: https://github.com/nodejs/github-bot/pull/90 --- scripts/attempt-backport.js | 44 ++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/scripts/attempt-backport.js b/scripts/attempt-backport.js index a78b90ca..f08b775c 100644 --- a/scripts/attempt-backport.js +++ b/scripts/attempt-backport.js @@ -53,7 +53,7 @@ module.exports = function (app) { }) } -function processNextBackport() { +function processNextBackport () { const item = queue.shift() if (!item) return @@ -67,18 +67,18 @@ function processNextBackport() { item() } -function queueAttemptBackport(options, version, isLTS) { - queue.push(function() { +function queueAttemptBackport (options, version, isLTS) { + queue.push(function () { options.logger.debug(`processing a new backport to v${version}`) attemptBackport(options, version, isLTS, processNextBackport) }) } -function attemptBackport(options, version, isLTS, cb) { +function attemptBackport (options, version, isLTS, cb) { // Start gitAmAbort() - function wrapCP(cmd, args, opts, callback) { + function wrapCP (cmd, args, opts, callback) { let exited = false if (arguments.length === 3) { @@ -91,11 +91,11 @@ function attemptBackport(options, version, isLTS, cb) { const cp = child_process.spawn(cmd, args, opts) const argsString = [cmd, ...args].join(' ') - cp.on('error', function(err) { + cp.on('error', function (err) { debug(`child_process err: ${err}`) if (!exited) onError() }) - cp.on('exit', function(code) { + cp.on('exit', function (code) { exited = true if (!cb) { debug(`error before exit, code: ${code}, on '${argsString}'`) @@ -119,7 +119,7 @@ function attemptBackport(options, version, isLTS, cb) { return cp } - function onError() { + function onError () { if (!cb) return const _cb = cb setImmediate(() => { @@ -133,19 +133,19 @@ function attemptBackport(options, version, isLTS, cb) { cb = null } - function gitAmAbort() { + function gitAmAbort () { // TODO(Fishrock123): this should probably just merge into wrapCP let exited = false - options.logger.debug(`aborting any previous backport attempt...`) + options.logger.debug('aborting any previous backport attempt...') const cp = child_process.spawn('git', ['am', '--abort'], { cwd: global._node_repo_dir }) const argsString = 'git am --abort' - cp.on('error', function(err) { + cp.on('error', function (err) { debug(`child_process err: ${err}`) if (!exited) onError() }) - cp.on('exit', function() { + cp.on('exit', function (code) { exited = true if (!cb) { debug(`error before exit, code: ${code}, on '${argsString}'`) @@ -155,33 +155,33 @@ function attemptBackport(options, version, isLTS, cb) { }) } - function gitRemoteUpdate() { - options.logger.debug(`updating git remotes...`) + function gitRemoteUpdate () { + options.logger.debug('updating git remotes...') wrapCP('git', ['remote', 'update', '-p'], gitCheckout) } - function gitCheckout() { + function gitCheckout () { options.logger.debug(`checking out upstream/v${version}.x-staging...`) wrapCP('git', ['checkout', `upstream/v${version}.x-staging`], gitReset) } - function gitReset() { + function gitReset () { options.logger.debug(`resetting upstream/v${version}.x-staging...`) wrapCP('git', ['reset', `upstream/v${version}.x-staging`, '--hard'], fetchDiff) } - function fetchDiff() { + function fetchDiff () { options.logger.debug(`fetching diff from pr ${options.prId}...`) const url = `https://patch-diff.githubusercontent.com/raw/${options.owner}/${options.repo}/pull/${options.prId}.patch` const req = request(url) - req.on('error', function(err) { + req.on('error', function (err) { debug(`request err: ${err}`) return onError() }) - req.on('response', function(response) { + req.on('response', function (response) { if (response.statusCode !== 200) { debug(`request non-200 status: ${response.statusCode}`) return onError() @@ -191,9 +191,9 @@ function attemptBackport(options, version, isLTS, cb) { gitAttemptBackport(req) } - function gitAttemptBackport(req) { + function gitAttemptBackport (req) { options.logger.debug(`attempting a backport to v${version}...`) - const cp = wrapCP('git', ['am'], { stdio: 'pipe' }, function done() { + const cp = wrapCP('git', ['am'], { stdio: 'pipe' }, function done () { // Success! if (isLTS) { updatePrWithLabels(options, [`lts-watch-v${version}.x`]) @@ -201,7 +201,7 @@ function attemptBackport(options, version, isLTS, cb) { // TODO(Fishrock123): Re-enable this, but do a check first // to make sure the label was set by the bot only. // removeLabelFromPR(options, `dont-land-on-v${version}.x`) - //} + // } setImmediate(() => { options.logger.debug(`backport to v${version} successful`)