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

Swift: simplify codeql workflow #19029

Merged
merged 2 commits into from
Mar 18, 2025
Merged

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Mar 14, 2025

  • removed ql test running and upgrade/downgrade scripts checking (now done internally)
  • removed all the bazel caching stuff, that never really worked any way
  • moved misc/codegen generic testing to a separate workflow, as it's not swift specific any more
  • reinstated checking that the extractor can be built locally from the codeql repo on both macOS and Linux.

@github-actions github-actions bot added the Swift label Mar 14, 2025
@redsun82 redsun82 force-pushed the reddsun82/swift-ql-test-to-internal branch 3 times, most recently from 4f59145 to 8691b1c Compare March 14, 2025 15:06
@redsun82 redsun82 added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Mar 14, 2025
* remove ql test running and upgrade/downgrade scripts checking (now
  done internally)
* removed all the bazel caching stuff, that never really worked any way
* moved `misc/codegen` generic testing to a separate workflow, as it's
  not swift specific any more
* reinstanted checking that the extractor can be built locally from
  the `codeql` repo.
@redsun82 redsun82 force-pushed the reddsun82/swift-ql-test-to-internal branch from 8691b1c to 622aa7c Compare March 14, 2025 15:14
@redsun82 redsun82 marked this pull request as ready for review March 17, 2025 09:42
@Copilot Copilot bot review requested due to automatic review settings March 17, 2025 09:42
@redsun82 redsun82 requested review from a team as code owners March 17, 2025 09:42

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the CodeQL workflow for Swift by removing redundant test scripts and caching, refactoring workflow triggers, and isolating generic codegen tasks.

  • Added a new Codegen workflow with formatting checks and bazel tests.
  • Updated the Swift workflow to use a matrix for build-and-test and updated the codegen target command.
  • Removed deprecated push triggers and unused scripts.

Reviewed Changes

Copilot reviewed 3 out of 7 changed files in this pull request and generated no comments.

File Description
.github/workflows/codegen.yml Added a new workflow file to run code generation tests.
.github/workflows/swift.yml Updated workflow triggers, job names, and commands for Swift CI.
Files not reviewed (4)
  • swift/actions/build-and-test/action.yml: Language not supported
  • swift/actions/database-upgrade-scripts/action.yml: Language not supported
  • swift/actions/run-ql-tests/action.yml: Language not supported
  • swift/actions/share-extractor-pack/action.yml: Language not supported
Comments suppressed due to low confidence (1)

.github/workflows/swift.yml:72

  • Removing the fully qualified label for the codegen target may cause incorrect target resolution if the target is not defined in the working directory set by defaults.
bazel run codegen -- --generate=trap,cpp --cpp-output=$PWD/generated-cpp-files

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@redsun82 redsun82 removed the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Mar 17, 2025
Copy link
Collaborator

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

One question.

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: bazelbuild/setup-bazelisk@v2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you no longer need this? Are you getting Bazel from the Actions image then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, see actions/runner-images#221 (comment). To be noted, is that on all images bazel actually points to a bazelisk binary, which means the correct bazel version from .bazelversion will be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(see for example https://github.com/github/codeql/actions/runs/13896248292/job/38877429011?pr=19029#step:4:5 where bazelisk reports downloading the required version)

@redsun82 redsun82 merged commit bed7ab5 into main Mar 18, 2025
19 checks passed
@redsun82 redsun82 deleted the reddsun82/swift-ql-test-to-internal branch March 18, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants