Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle multiple types for advanced scenarios #143

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ jobs:
last-successful-event: ""

# The path where your repository is. This is only required for cases where the repository code is checked out or moved to a specific path.
# The property can also contain several types separated by comma i.e. schedule,workflow_dispatch
#
# Default: .
working-directory: ""
Expand Down
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ inputs:
required: false
default: ""
last-successful-event:
description: "The type of event to check for the last successful commit corresponding to that workflow-id, e.g. push, pull_request, release etc"
description: "The type of event to check for the last successful commit corresponding to that workflow-id, e.g. push or pull_request or release. The property can also contain several types separated by a comma. E.g. schedule,workflow_dispatch."
default: "push"
working-directory:
description: "The directory where your repository is located"
Expand Down
61 changes: 46 additions & 15 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37914,6 +37914,9 @@ let BASE_SHA;
else {
process.stdout.write("\n");
process.stdout.write(`WARNING: Unable to find a successful workflow run on 'origin/${mainBranchName}', or the latest successful workflow was connected to a commit which no longer exists on that branch (e.g. if that branch was rebased)\n`);
process.stdout.write(`We are therefore defaulting to use HEAD~1 on 'origin/${mainBranchName}'\n`);
process.stdout.write("\n");
process.stdout.write(`NOTE: You can instead make this a hard error by setting 'error-on-no-successful-workflow' on the action in your workflow.\n`);
if (fallbackSHA) {
BASE_SHA = fallbackSHA;
process.stdout.write(`Using provided fallback SHA: ${fallbackSHA}\n`);
Expand Down Expand Up @@ -37978,21 +37981,49 @@ function findSuccessfulCommit(workflow_id, run_id, owner, repo, branch, lastSucc
process.stdout.write("\n");
process.stdout.write(`Workflow Id not provided. Using workflow '${workflow_id}'\n`);
}
// fetch all workflow runs on a given repo/branch/workflow with push and success
const shas = yield octokit
.request(`GET /repos/${owner}/${repo}/actions/workflows/${workflow_id}/runs`, {
owner,
repo,
// on some workflow runs we do not have branch property
branch: lastSuccessfulEvent === "push" ||
lastSuccessfulEvent === "workflow_dispatch"
? branch
: undefined,
workflow_id,
event: lastSuccessfulEvent,
status: "success",
})
.then(({ data: { workflow_runs } }) => workflow_runs.map((run) => run.head_sha));
let shas = [];
// if there are several events separated by comma
if (lastSuccessfulEvent.includes(",")) {
// find the greatest workflow id among the types and retrieve the sha
const events = lastSuccessfulEvent.split(",");
const workflowIdMap = new Map();
for (const event of events) {
const workflowRun = yield octokit
.request(`GET /repos/${owner}/${repo}/actions/workflows/${workflow_id}/runs`, {
owner,
repo,
// on non-push workflow runs we do not have branch property
branch: lastSuccessfulEvent === "push" ||
lastSuccessfulEvent === "workflow_dispatch"
? branch
: undefined,
event,
workflow_id,
})
.then(({ data }) => {
return data.workflow_runs[0];
});
workflowIdMap.set(workflowRun.id, workflowRun.head_sha);
}
shas.push(workflowIdMap.get(Math.max(...workflowIdMap.keys())));
}
else {
// fetch all workflow runs on a given repo/branch/workflow with push and success
shas = yield octokit
.request(`GET /repos/${owner}/${repo}/actions/workflows/${workflow_id}/runs`, {
owner,
repo,
// on non-push workflow runs we do not have branch property
branch: lastSuccessfulEvent === "push" ||
lastSuccessfulEvent === "workflow_dispatch"
? branch
: undefined,
workflow_id,
event: lastSuccessfulEvent,
status: "success",
})
.then(({ data: { workflow_runs } }) => workflow_runs.map((run) => run.head_sha));
}
return yield findExistingCommit(octokit, branch, shas);
});
}
Expand Down
112 changes: 75 additions & 37 deletions find-successful-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ let BASE_SHA: string;
} else {
process.stdout.write("\n");
process.stdout.write(
`WARNING: Working directory '${workingDirectory}' doesn't exist.\n`,
`WARNING: Working directory '${workingDirectory}' doesn't exist.\n`
);
}
}
Expand All @@ -50,7 +50,7 @@ let BASE_SHA: string;
const baseResult = spawnSync(
"git",
["merge-base", `origin/${mainBranchName}`, mergeBaseRef],
{ encoding: "utf-8" },
{ encoding: "utf-8" }
);
BASE_SHA = baseResult.stdout;
} catch (e) {
Expand All @@ -65,7 +65,7 @@ let BASE_SHA: string;
owner,
repo,
mainBranchName,
lastSuccessfulEvent,
lastSuccessfulEvent
);
} catch (e) {
core.setFailed(e.message);
Expand All @@ -79,29 +79,36 @@ let BASE_SHA: string;
} else {
process.stdout.write("\n");
process.stdout.write(
`WARNING: Unable to find a successful workflow run on 'origin/${mainBranchName}', or the latest successful workflow was connected to a commit which no longer exists on that branch (e.g. if that branch was rebased)\n`,
`WARNING: Unable to find a successful workflow run on 'origin/${mainBranchName}', or the latest successful workflow was connected to a commit which no longer exists on that branch (e.g. if that branch was rebased)\n`
);
process.stdout.write(
`We are therefore defaulting to use HEAD~1 on 'origin/${mainBranchName}'\n`
);
process.stdout.write("\n");
process.stdout.write(
`NOTE: You can instead make this a hard error by setting 'error-on-no-successful-workflow' on the action in your workflow.\n`
);
if (fallbackSHA) {
BASE_SHA = fallbackSHA;
process.stdout.write(`Using provided fallback SHA: ${fallbackSHA}\n`);
} else {
process.stdout.write(
`We are therefore defaulting to use HEAD~1 on 'origin/${mainBranchName}'\n`,
`We are therefore defaulting to use HEAD~1 on 'origin/${mainBranchName}'\n`
);
process.stdout.write("\n");
process.stdout.write(
`NOTE: You can instead make this a hard error by setting 'error-on-no-successful-workflow' on the action in your workflow.\n`,
`NOTE: You can instead make this a hard error by setting 'error-on-no-successful-workflow' on the action in your workflow.\n`
);
process.stdout.write("\n");

const commitCountOutput = spawnSync(
"git",
["rev-list", "--count", `origin/${mainBranchName}`],
{ encoding: "utf-8" },
{ encoding: "utf-8" }
).stdout;
const commitCount = parseInt(
stripNewLineEndings(commitCountOutput),
10,
10
);

const LAST_COMMIT_CMD = `origin/${mainBranchName}${
Expand All @@ -117,7 +124,7 @@ let BASE_SHA: string;
} else {
process.stdout.write("\n");
process.stdout.write(
`Found the last successful workflow run on 'origin/${mainBranchName}'\n`,
`Found the last successful workflow run on 'origin/${mainBranchName}'\n`
);
process.stdout.write(`Commit: ${BASE_SHA}\n`);
}
Expand Down Expand Up @@ -154,7 +161,7 @@ async function findSuccessfulCommit(
owner: string,
repo: string,
branch: string,
lastSuccessfulEvent: string,
lastSuccessfulEvent: string
): Promise<string | undefined> {
const octokit = new ProxifiedClient();
if (!workflow_id) {
Expand All @@ -168,30 +175,61 @@ async function findSuccessfulCommit(
.then(({ data: { workflow_id } }) => workflow_id);
process.stdout.write("\n");
process.stdout.write(
`Workflow Id not provided. Using workflow '${workflow_id}'\n`,
`Workflow Id not provided. Using workflow '${workflow_id}'\n`
);
}
// fetch all workflow runs on a given repo/branch/workflow with push and success
const shas = await octokit
.request(
`GET /repos/${owner}/${repo}/actions/workflows/${workflow_id}/runs`,
{
owner,
repo,
// on some workflow runs we do not have branch property
branch:
lastSuccessfulEvent === "push" ||
lastSuccessfulEvent === "workflow_dispatch"
? branch
: undefined,
workflow_id,
event: lastSuccessfulEvent,
status: "success",
},
)
.then(({ data: { workflow_runs } }) =>
workflow_runs.map((run: { head_sha: any }) => run.head_sha),
);
let shas = [];
// if there are several events separated by comma
if (lastSuccessfulEvent.includes(",")) {
// find the greatest workflow id among the types and retrieve the sha
const events = lastSuccessfulEvent.split(",");
const workflowIdMap = new Map();
for (const event of events) {
Comment on lines +183 to +187
Copy link

@sparr sparr May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if condition and the else code path below seems unnecessary. A string with no commas, e.g. "foo", will split(",") to ["foo"] and the iteration will work fine over that single element.

const workflowRun = await octokit
.request(
`GET /repos/${owner}/${repo}/actions/workflows/${workflow_id}/runs`,
{
owner,
repo,
// on non-push workflow runs we do not have branch property
branch:
lastSuccessfulEvent === "push" ||
lastSuccessfulEvent === "workflow_dispatch"
? branch
: undefined,
event,
workflow_id,
}
)
.then(({ data }) => {
return data.workflow_runs[0];
});
workflowIdMap.set(workflowRun.id, workflowRun.head_sha);
}
shas.push(workflowIdMap.get(Math.max(...workflowIdMap.keys())));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it will push all the SHAs from one event, then all of them from another. So they won't be in order in the final array. Which means the call to findExistingCommit below, which returns the first commit in the list that exists, might not return the correct commit?

} else {
// fetch all workflow runs on a given repo/branch/workflow with push and success
shas = await octokit
.request(
`GET /repos/${owner}/${repo}/actions/workflows/${workflow_id}/runs`,
{
owner,
repo,
// on non-push workflow runs we do not have branch property
branch:
lastSuccessfulEvent === "push" ||
lastSuccessfulEvent === "workflow_dispatch"
? branch
: undefined,
workflow_id,
event: lastSuccessfulEvent,
status: "success",
}
)
.then(({ data: { workflow_runs } }) =>
workflow_runs.map((run: { head_sha: any }) => run.head_sha)
);
}

return await findExistingCommit(octokit, branch, shas);
}
Expand All @@ -208,7 +246,7 @@ async function findMergeBaseRef(): Promise<string> {
function findMergeQueuePr(): string {
const { head_ref, base_sha } = github.context.payload.merge_group;
const result = new RegExp(
`^refs/heads/gh-readonly-queue/${mainBranchName}/pr-(\\d+)-${base_sha}$`,
`^refs/heads/gh-readonly-queue/${mainBranchName}/pr-(\\d+)-${base_sha}$`
).exec(head_ref);
return result ? result.at(1) : undefined;
}
Expand All @@ -223,7 +261,7 @@ async function findMergeQueueBranch(): Promise<string> {
const octokit = new ProxifiedClient();
const result = await octokit.request(
`GET /repos/${owner}/${repo}/pulls/${pull_number}`,
{ owner, repo, pull_number: +pull_number },
{ owner, repo, pull_number: +pull_number }
);
return result.data.head.ref;
}
Expand All @@ -234,7 +272,7 @@ async function findMergeQueueBranch(): Promise<string> {
async function findExistingCommit(
octokit: Octokit,
branchName: string,
shas: string[],
shas: string[]
): Promise<string | undefined> {
for (const commitSha of shas) {
if (await commitExists(octokit, branchName, commitSha)) {
Expand All @@ -250,7 +288,7 @@ async function findExistingCommit(
async function commitExists(
octokit: Octokit,
branchName: string,
commitSha: string,
commitSha: string
): Promise<boolean> {
try {
spawnSync("git", ["cat-file", "-e", commitSha], {
Expand All @@ -273,7 +311,7 @@ async function commitExists(
});

return commits.data.some(
(commit: { sha: string }) => commit.sha === commitSha,
(commit: { sha: string }) => commit.sha === commitSha
);
} catch {
return false;
Expand Down
Loading