Skip to content

Commit

Permalink
try new method to merge forks
Browse files Browse the repository at this point in the history
  • Loading branch information
ephys committed Apr 12, 2024
1 parent 257ac5f commit 825917f
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 30 deletions.
35 changes: 35 additions & 0 deletions .github/workflows/test-forks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Test updating PR from forks
on:
workflow_dispatch:
jobs:
autoupdate:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Setup Node.js
id: setup-node
uses: actions/setup-node@v4
with:
node-version: 20.x
- name: Generate Sequelize Bot Token
id: generate-token
uses: actions/create-github-app-token@v1
with:
app-id: '${{ secrets.SEQUELIZE_BOT_APP_ID }}'
private-key: '${{ secrets.SEQUELIZE_BOT_PRIVATE_KEY }}'
- name: Test Local Action
id: test-action
uses: ./
with:
update-pr-branches: true
update-requires-labels: 'test-pr'
update-requires-source: 'fork'
env:
GITHUB_TOKEN: '${{ steps.generate-token.outputs.token }}'
- name: Print Output
id: output
run: |
echo "Updated PRs:"
echo "${{ steps.test-action.outputs.updated-prs }}"
echo "Conflicted PRs:"
echo "${{ steps.test-action.outputs.conflicted-prs }}"
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ inputs:
description: 'Do not update PRs that were opened by the specified authors (comma-separated list of usernames. If the author is an app, it must be in the format "app/<login>")'
required: false
default: ''
update-requires-source:
description: 'Only update PRs that are either forks or branches from the same repository. Accepts "all" (default), "forks", and "branches"'
required: false
default: 'all'
dry-run:
description: 'Do not perform any actions, only log what would be done'
required: false
Expand Down
65 changes: 36 additions & 29 deletions lib/action.mjs

Large diffs are not rendered by default.

116 changes: 115 additions & 1 deletion src/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import core from '@actions/core';
import github from '@actions/github';
import type { PullRequestEvent, PushEvent } from '@octokit/webhooks-types';
import { isString } from '@sequelize/utils';
import childProcess from 'node:child_process';
import fs from 'node:fs/promises';
import * as path from 'node:path';
import { setTimeout } from 'node:timers/promises';

isString.assert(process.env.GITHUB_TOKEN, 'GITHUB_TOKEN env must be provided');
Expand Down Expand Up @@ -48,6 +50,11 @@ const updateRequiresReadyState = getEnumInput('update-requires-ready-state', REA
const updateRequiresLabels = getCommaSeparatedInput('update-requires-labels');
const updateExcludedLabels = getCommaSeparatedInput('update-excluded-labels');
const updateExcludedAuthors = getCommaSeparatedInput('update-excluded-authors');
const updateRequiresSource = getEnumInput('update-requires-source', [
'all',
'fork',
'branches',
] as const);

interface RepositoryId {
owner: string;
Expand All @@ -63,7 +70,13 @@ interface PullRequest {
enabledAt: string;
};
baseRef: { name: string };
baseRepository: {
nameWithOwner: string;
};
headRef: { name: string };
headRepository: {
nameWithOwner: string;
};
isDraft: boolean;
labels: {
nodes: [
Expand All @@ -72,6 +85,7 @@ interface PullRequest {
},
];
};
maintainerCanModify: boolean;
mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';
number: number;
}
Expand All @@ -95,6 +109,13 @@ fragment PR on PullRequest {
}
baseRef { name }
headRef { name }
headRepository {
nameWithOwner
}
baseRepository {
nameWithOwner
}
maintainerCanModify
}
`;

Expand Down Expand Up @@ -239,6 +260,20 @@ async function updatePrBranch(repositoryId: RepositoryId, pullRequest: PullReque
return;
}

if (!prMatchesSource(pullRequest, updateRequiresSource)) {
console.info(`[PR ${pullRequest.number}] Not from the expected source, skipping update.`);

return;
}

if (isForkPr(pullRequest) && !pullRequest.maintainerCanModify) {
console.info(
`[PR ${pullRequest.number}] Fork PR refuses updates from maintainers, skipping update.`,
);

return;
}

const isBehind = await checkPrIsBehindTarget(repositoryId, pullRequest);
if (!isBehind) {
return;
Expand All @@ -248,13 +283,75 @@ async function updatePrBranch(repositoryId: RepositoryId, pullRequest: PullReque

console.info(`[PR ${pullRequest.number}] ✅ Updating branch.`);

if (!dryRun) {
if (dryRun) {
return;
}

// The "update-branch" endpoint does not allow modifying pull requests from repositories we do not own,
// even if the "allow maintainers to modify" setting is enabled on the PR.
if (!isForkPr(pullRequest)) {
// This operation cannot be done with GITHUB_TOKEN, as the GITHUB_TOKEN does not trigger subsequent workflows.
return userBot.rest.pulls.updateBranch({
...repositoryId,
pull_number: pullRequest.number,
});
}

// For fork PRs, we use git directly instead:
// - Clone the repository in a new directory
// - Merge the base branch into the PR branch
// - Push the changes to the PR branch

const targetDirectoryName = `pr-${pullRequest.number}`;
const targetDirectoryPath = path.join(process.cwd(), targetDirectoryName);

console.log('cloning repo in', targetDirectoryPath);

// gh repo clone sequelize/sequelize
childProcess.execFileSync(
'gh',
[
'repo',
'clone',
pullRequest.baseRepository.nameWithOwner,
targetDirectoryName,
'--',
'--branch',
pullRequest.baseRef.name,
],
{
stdio: 'inherit',
env: {
...process.env,
GH_TOKEN: process.env.GITHUB_TOKEN,
},
},
);

console.log('checking out PR branch');

childProcess.execFileSync('gh', ['pr', 'checkout', String(pullRequest.number)], {
cwd: targetDirectoryPath,
stdio: 'inherit',
env: {
...process.env,
GH_TOKEN: process.env.GITHUB_TOKEN,
},
});

console.log('executing git merge');

childProcess.execFileSync('git', ['merge', pullRequest.baseRef.name, '--no-edit'], {
cwd: targetDirectoryPath,
stdio: 'inherit',
});

console.log('executing git push');

childProcess.execFileSync('git', ['push'], {
cwd: targetDirectoryPath,
stdio: 'inherit',
});
}

interface CompareBranchResponse {
Expand Down Expand Up @@ -460,6 +557,19 @@ function prMatchesReadyState(pullRequest: PullRequest, readyState: (typeof READY
}
}

function prMatchesSource(pullRequest: PullRequest, source: typeof updateRequiresSource) {
switch (source) {
case 'all':
return true;

case 'fork':
return isForkPr(pullRequest);

case 'branches':
return !isForkPr(pullRequest);
}
}

function isConflictManagementEnabledForPr(pullRequest: PullRequest) {
if (!prMatchesReadyState(pullRequest, conflictRequiresReadyState)) {
console.info(
Expand Down Expand Up @@ -495,3 +605,7 @@ function isConflictManagementEnabledForPr(pullRequest: PullRequest) {

return true;
}

function isForkPr(pullRequest: PullRequest) {
return pullRequest.baseRepository.nameWithOwner !== pullRequest.headRepository.nameWithOwner;
}

0 comments on commit 825917f

Please sign in to comment.