Repository navigation
Address ubuntu-26.04-related CI failures
#4267
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
Changes from all commits
b63386c
b913bf6
ce7d3f7
dd68f13
eac0005
ab743b6
171607b
34b4917
63b184d
8fb8b47
0b8f456
bcc1fbe
2908c78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,7 +137,15 @@ interface LanguageSetup { | |
| /** Describes partial mappings from built-in languages to their specific setup information. */ | ||
| type LanguageSetups = Partial<Record<BuiltInLanguage, LanguageSetup>>; | ||
|
|
||
| // The default set of CodeQL Bundle versions to use for the PR checks. | ||
| /** A type representing version identifiers. */ | ||
| type Version = | ||
| | "default" | ||
| | "linked" | ||
| | "nightly-latest" | ||
| // This is an approximation, but sufficient validation for our needs here. | ||
| | `stable-v${number}.${number}.${number}`; | ||
|
|
||
| /** The default set of CodeQL Bundle versions to use for the PR checks. */ | ||
| const defaultTestVersions = [ | ||
| // The oldest supported CodeQL version. If bumping, update `CODEQL_MINIMUM_VERSION` in `codeql.ts` | ||
| "stable-v2.20.7", | ||
|
|
@@ -159,7 +167,10 @@ const defaultTestVersions = [ | |
| "linked", | ||
| // A nightly build directly from the our private repo, built in the last 24 hours. | ||
| "nightly-latest", | ||
| ]; | ||
| ] as const satisfies Version[]; | ||
|
|
||
| /** A type representing the default test versions. */ | ||
| type DefaultTestVersion = (typeof defaultTestVersions)[number]; | ||
|
|
||
| /** The default versions we use for languages / frameworks, if not specified as a workflow input. */ | ||
| const defaultLanguageVersions = { | ||
|
|
@@ -313,9 +324,11 @@ const languageSetups: LanguageSetups = { | |
| }, | ||
| }; | ||
|
|
||
| // This is essentially an arbitrary version of `yq`, which happened to be the one that | ||
| // `choco` fetched when we moved away from using that here. | ||
| // See https://github.com/github/codeql-action/pull/3423 | ||
| /** | ||
| * This is essentially an arbitrary version of `yq`, which happened to be the one that | ||
| * `choco` fetched when we moved away from using that here. | ||
| * See https://github.com/github/codeql-action/pull/3423 | ||
| */ | ||
| const YQ_VERSION = "v4.50.1"; | ||
|
|
||
| const CHECKS_DIR = path.join(PR_CHECKS_DIR, "checks"); | ||
|
|
@@ -442,10 +455,23 @@ function generateJobMatrix( | |
| : [operatingSystemConfig["runner-image"]]; | ||
|
|
||
| for (const runnerImage of runnerImagesForOs) { | ||
| matrix.push({ | ||
| os: runnerImage, | ||
| version, | ||
| }); | ||
| // stable-v2.20.7 requires an older version of Java than the default on `ubuntu-latest`. | ||
| // We could install that older version on the newer runners, or use the older runner. | ||
| // We do the latter here. This can be removed once we stop supporting `v2.20.7`. | ||
| // Since this is typed as `DefaultTestVersion`, it will become an error once `stable-v2.20.7` | ||
| // is removed from `defaultTestVersions`. | ||
|
Comment on lines
+461
to
+462
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Though I believe we don't build the PR checks project as part of CI, so we'd only see this locally)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeing it locally is the intended scenario, because you'd locally remove it from
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, it might be worth running |
||
| const requiresOlderRunnerImage: DefaultTestVersion = "stable-v2.20.7"; | ||
| if ( | ||
| runnerImage === "ubuntu-latest" && | ||
| version === requiresOlderRunnerImage | ||
| ) { | ||
| matrix.push({ os: "ubuntu-24.04", version }); | ||
| } else { | ||
|
Comment on lines
+458
to
+469
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another valid approach here would be overriding the runner image for (I think just) the multi-language check, since I believe this issue only affects checks that analyze Java.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, but I think we probably should just merge this for now to unblock other work. |
||
| matrix.push({ | ||
| os: runnerImage, | ||
| version, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presume this probably happened automatically, but we should consider adding a YAML formatter to CI to avoid this sort of noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it happened automatically. Agreed on adding a formatter check, but not as part of this PR.