Skip to content
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
5 changes: 5 additions & 0 deletions init/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ inputs:
description: >-
Explicitly enable or disable caching of project build dependencies.
required: false
check-run-id:
description: >-
[Internal] The ID of the check run, as provided by the Actions runtime environment. Do not set this value manually.
default: ${{ job.check_run_id }}
required: false
outputs:
codeql-path:
description: The path of the CodeQL binary used for analysis
Expand Down
25 changes: 21 additions & 4 deletions lib/init-action-post.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion src/init-action-post-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ test("not uploading failed SARIF when `code-scanning` is not an enabled analysis
test("saves overlay status when overlay-base analysis did not complete successfully", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
process.env["GITHUB_REPOSITORY"] = "github/codeql-action-fake-repository";
process.env["GITHUB_RUN_ID"] = "12345";
process.env["GITHUB_RUN_ATTEMPT"] = "1";
process.env["GITHUB_JOB"] = "analyze";
process.env["RUNNER_TEMP"] = tmpDir;
Comment on lines 318 to 322
Copy link
Member

Choose a reason for hiding this comment

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

Minor: It might be nice to extend setupActionsVars with all environment variables that we'd typically expect in an Actions environment and modify it to accept an argument with a (partial) mapping of specific environment variables we want to set. Not necessarily something for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I'll take a look at this in a separate PR.

// Ensure analyze did not complete successfully.
delete process.env[EnvVar.ANALYZE_DID_COMPLETE_SUCCESSFULLY];
Expand Down Expand Up @@ -370,8 +373,13 @@ test("saves overlay status when overlay-base analysis did not complete successfu
{
attemptedToBuildOverlayBaseDatabase: true,
builtOverlayBaseDatabase: false,
job: {
workflowRunId: 12345,
workflowRunAttempt: 1,
name: "analyze",
},
},
"fourth arg should be the overlay status recording an unsuccessful build attempt",
"fourth arg should be the overlay status recording an unsuccessful build attempt with job details",
);
});
});
Expand Down
21 changes: 16 additions & 5 deletions src/init-action-post-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import { EnvVar } from "./environment";
import { Feature, FeatureEnablement } from "./feature-flags";
import { Logger } from "./logging";
import { OverlayDatabaseMode } from "./overlay";
import { OverlayStatus, saveOverlayStatus } from "./overlay/status";
import {
createOverlayStatus,
OverlayStatus,
saveOverlayStatus,
} from "./overlay/status";
import { RepositoryNwo, getRepositoryNwo } from "./repository";
import { JobStatus } from "./status-report";
import * as uploadLib from "./upload-lib";
Expand Down Expand Up @@ -270,10 +274,17 @@ async function recordOverlayStatus(
return;
}

const overlayStatus: OverlayStatus = {
attemptedToBuildOverlayBaseDatabase: true,
builtOverlayBaseDatabase: false,
};
const checkRunIdInput = actionsUtil.getOptionalInput("check-run-id");
const checkRunId =
checkRunIdInput !== undefined ? parseInt(checkRunIdInput, 10) : undefined;

const overlayStatus: OverlayStatus = createOverlayStatus(
{
attemptedToBuildOverlayBaseDatabase: true,
builtOverlayBaseDatabase: false,
},
Number.isNaN(checkRunId) ? undefined : checkRunId,
Copy link
Member

Choose a reason for hiding this comment

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

Should we also check that this is >= 0 (or similar)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure 👍

);

const diskUsage = await checkDiskUsage(logger);
if (diskUsage === undefined) {
Expand Down
38 changes: 37 additions & 1 deletion src/overlay/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,17 @@ import * as path from "path";

import * as actionsCache from "@actions/cache";

import { getTemporaryDirectory } from "../actions-util";
import {
getTemporaryDirectory,
getWorkflowRunAttempt,
getWorkflowRunID,
} from "../actions-util";
import { type CodeQL } from "../codeql";
import { Logger } from "../logging";
import {
DiskUsage,
getErrorMessage,
getRequiredEnvParam,
waitForResultWithTimeLimit,
} from "../util";

Expand All @@ -38,12 +43,43 @@ function getStatusFilePath(languages: string[]): string {
);
}

/** Details of the job that recorded an overlay status. */
interface JobInfo {
/** The check run ID. This is optional since it is not always available. */
checkRunId?: number;
/** The workflow run ID. */
workflowRunId: number;
/** The workflow run attempt number. */
workflowRunAttempt: number;
/** The name of the job (from GITHUB_JOB). */
name: string;
}

/** Status of an overlay analysis for a group of languages. */
export interface OverlayStatus {
/** Whether the job attempted to build an overlay base database. */
attemptedToBuildOverlayBaseDatabase: boolean;
/** Whether the job successfully built an overlay base database. */
builtOverlayBaseDatabase: boolean;
/** Details of the job that recorded this status. */
job?: JobInfo;
}

/** Creates an `OverlayStatus` populated with the details of the current job. */
export function createOverlayStatus(
attributes: Omit<OverlayStatus, "job">,
checkRunId?: number,
): OverlayStatus {
const job: JobInfo = {
workflowRunId: getWorkflowRunID(),
workflowRunAttempt: getWorkflowRunAttempt(),
name: getRequiredEnvParam("GITHUB_JOB"),
...(checkRunId !== undefined && { checkRunId }),
Copy link
Member

Choose a reason for hiding this comment

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

I think we had a similar point on another PR recently: this is mostly equivalent to just checkRunId, except that with this the JobInfo object doesn't even contain the key? Do you think it's beneficial to explicitly not have the key, rather than it being set to undefined here? If so, it would be good to document why. Otherwise, change it to just checkRunId.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we can't serialize undefined values to JSON, there's no difference in practice. So we might as well use just checkRunId

};
return {
...attributes,
job,
};
}

/**
Expand Down
Loading