Skip to content

Commit

Permalink
Merge pull request #2245 from github/henrymercer/ignore-already-speci…
Browse files Browse the repository at this point in the history
…fied-flags

Ensure `--overwrite` flag is only passed once
  • Loading branch information
henrymercer committed Apr 16, 2024
2 parents 9b87e0a + 556b3bc commit c4fb451
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Note that the only difference between `v2` and `v3` of the CodeQL Action is the
## [UNRELEASED]

- We are rolling out a feature in April/May 2024 that improves the reliability and performance of analyzing code when analyzing a compiled language with the `autobuild` [build mode](https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages#codeql-build-modes). [#2235](https://github.com/github/codeql-action/pull/2235)
- Fix a bug where the `init` Action would fail if `--overwrite` was specified in `CODEQL_ACTION_EXTRA_OPTIONS`. [#2245](https://github.com/github/codeql-action/pull/2245)

## 3.25.0 - 15 Apr 2024

Expand Down
15 changes: 11 additions & 4 deletions lib/codeql.js

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

2 changes: 1 addition & 1 deletion lib/codeql.js.map

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions lib/codeql.test.js

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

2 changes: 1 addition & 1 deletion lib/codeql.test.js.map

Large diffs are not rendered by default.

36 changes: 36 additions & 0 deletions src/codeql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,42 @@ test("runTool outputs last line of stderr if fatal error could not be found", as
);
});

test("Avoids duplicating --overwrite flag if specified in CODEQL_ACTION_EXTRA_OPTIONS", async (t) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.12.6"));
// safeWhich throws because of the test CodeQL object.
sinon.stub(safeWhich, "safeWhich").resolves("");

process.env["CODEQL_ACTION_EXTRA_OPTIONS"] =
'{ "database": { "init": ["--overwrite"] } }';

await codeqlObject.databaseInitCluster(
stubConfig,
"sourceRoot",
undefined,
undefined,
createFeatures([]),
getRunnerLogger(false),
);

t.true(runnerConstructorStub.calledOnce);
const args = runnerConstructorStub.firstCall.args[1] as string[];
t.is(
args.filter((option: string) => option === "--overwrite").length,
1,
"--overwrite should only be passed once",
);

// Clean up
const configArg = args.find((arg: string) =>
arg.startsWith("--codescanning-config="),
);
t.truthy(configArg, "Should have injected a codescanning config");
const configFile = configArg!.split("=")[1];
await del(configFile, { force: true });
});

export function stubToolRunnerConstructor(
exitCode: number = 0,
stderr?: string,
Expand Down
20 changes: 16 additions & 4 deletions src/codeql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,9 @@ export async function getCodeQLForCmd(
`--source-root=${sourceRoot}`,
...(await getLanguageAliasingArguments(this)),
...extraArgs,
...getExtraOptionsFromEnv(["database", "init"]),
...getExtraOptionsFromEnv(["database", "init"], {
ignoringOptions: ["--overwrite"],
}),
],
{ stdin: externalRepositoryToken },
);
Expand Down Expand Up @@ -835,7 +837,9 @@ export async function getCodeQLForCmd(
"--expect-discarded-cache",
"--min-disk-free=1024", // Try to leave at least 1GB free
"-v",
...getExtraOptionsFromEnv(["database", "run-queries"]),
...getExtraOptionsFromEnv(["database", "run-queries"], {
ignoringOptions: ["--expect-discarded-cache"],
}),
];
if (
await util.codeQlVersionAbove(
Expand Down Expand Up @@ -1174,10 +1178,18 @@ export async function getCodeQLForCmd(

/**
* Gets the options for `path` of `options` as an array of extra option strings.
*
* @param ignoringOptions Options that should be ignored, for example because they have already
* been passed and it is an error to pass them more than once.
*/
function getExtraOptionsFromEnv(paths: string[]) {
function getExtraOptionsFromEnv(
paths: string[],
{ ignoringOptions }: { ignoringOptions?: string[] } = {},
) {
const options: ExtraOptions = util.getExtraOptionsEnvParam();
return getExtraOptions(options, paths, []);
return getExtraOptions(options, paths, []).filter(
(option) => !ignoringOptions?.includes(option),
);
}

/**
Expand Down

0 comments on commit c4fb451

Please sign in to comment.