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

Add Ruby and Swift language autodetect test #1369

Merged
merged 11 commits into from
Nov 22, 2022

Conversation

angelapwen
Copy link
Contributor

@angelapwen angelapwen commented Nov 15, 2022

This PR check adds Swift and Ruby to the existing multi-language autodetect check. Once Swift is in GA, we will be able to remove the CODEQL_ENABLE_EXPERIMENTAL_FEATURES_SWIFT: "true" set in that check.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@angelapwen angelapwen marked this pull request as ready for review November 15, 2022 21:18
@angelapwen angelapwen requested a review from a team as a code owner November 15, 2022 21:18
@henrymercer
Copy link
Contributor

🤔 Hmm, I think Swift can only be auto-detected in the CLI as of nightly-latest at the moment. I think possibly there's still some part of the Action that's doing language autodetection, even when we're parsing the config in the CLI, but I haven't dug into it.

@angelapwen
Copy link
Contributor Author

angelapwen commented Nov 15, 2022

🤔 Hmm, I think Swift can only be auto-detected in the CLI as of nightly-latest at the moment. I think possibly there's still some part of the Action that's doing language autodetection, even when we're parsing the config in the CLI, but I haven't dug into it.

I thought so too, but was surprised that it passed in a previous run (before I marked ready for review and merged main in): https://github.com/github/codeql-action/actions/runs/3474223934/jobs/5807144328

I can make this repo draft until 2.11.4 is released (so it would be caught under latest and cached) if you think that's safer?

pr-checks/checks/swift-autodetect.yml Outdated Show resolved Hide resolved
pr-checks/checks/swift-autodetect.yml Outdated Show resolved Hide resolved
Comment on lines 1 to 2
# This check should be combined into `multi-language-autodetect.yml` once Swift is GA'ed
# and the `CODEQL_ENABLE_EXPERIMENTAL_FEATURES_SWIFT` environment variable is not needed.
Copy link
Contributor

@henrymercer henrymercer Nov 17, 2022

Choose a reason for hiding this comment

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

I think it would be fine to set CODEQL_ENABLE_EXPERIMENTAL_FEATURES_SWIFT=true in multi-language-autodetect.yml to help cut down on the number of jobs. We could add some checks on the tools version to make sure that language autodetection does the right thing for old CLIs too.

Comment on lines +113 to +114
if: (matrix.version == 'cached' || matrix.version == 'latest' || matrix.version
== 'nightly-latest')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is what you'd meant by using the tools version, but I saw there was precedent for using the matrix.version in the ml-powered-queries PR check.

Currently, just checking Ruby and Swift in the most recent 3 CLIs and the other languages in the hardcoded 3 prior CLIs. This can be made more robust when we work out our testing plan for all supported releases.

@angelapwen angelapwen enabled auto-merge (squash) November 18, 2022 00:23
@angelapwen angelapwen changed the title Add Swift language autodetect test Add Ruby and Swift language autodetect test Nov 18, 2022
@angelapwen angelapwen enabled auto-merge (squash) November 18, 2022 00:25
henrymercer
henrymercer previously approved these changes Nov 18, 2022
@angelapwen
Copy link
Contributor Author

angelapwen commented Nov 18, 2022

Ah, I am having some errors come up in nightly-latest runs due to a bug in the CLI that has been fixed:

  ERROR: Referenced pack 'codeql/regex' not found. (/opt/hostedtoolcache/CodeQL/0.0.0-20221118/x64/codeql/qlpacks/codeql/python-all/0.6.4-dev/qlpack.yml:1,1-1)
  A fatal error occurred: Could not resolve library path for /home/runner/work/codeql-action/codeql-action/codeql-qlpacks/complex-python-qlpack

will have to re-run these checks when the version in nightly-latest is superseded by the version with the fix.

@angelapwen angelapwen merged commit bab5a14 into github:main Nov 22, 2022
@angelapwen angelapwen deleted the swift-autodetect branch November 22, 2022 17:49
@hongbo-miao hongbo-miao mentioned this pull request Nov 29, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants