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

Fix computation of fallback version number #1510

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

henrymercer
Copy link
Contributor

@henrymercer henrymercer commented Jan 26, 2023

This PR addresses a bug where we would compute a fallback version number like 0.0.0-codeql-bundle-20230105 rather than one like 0.0.0-20230105. This happened because we used the tag name 0.0.0-codeql-bundle-20230105 rather than the bundle version 20230105 to compute the fallback version number.

This fallback version number is crucial at the moment, because the controlled switchover changes haven't yet rolled out to the GitHub-hosted runner images, and therefore at the moment, the GitHub-hosted runner images version the CodeQL tools as 0.0.0-20230105 rather than 2.0.0-20230105 (or 2.0.0). So if we get the fallback version number wrong, we end up having to download the bundle from the GitHub Release in most workflows.

The fix is pretty simple — I've split out the tag -> bundle version logic from getBundleVersionFromUrl into a new function getBundleVersionFromTagName and called this in getCodeQLSource when we compute the fallback version. Another approach would have been to call getBundleVersionFromUrl(`/${tagName}/`) but that's a little hacky.

I think part of the reason why this bug happened is that some of our unit tests do too much stuff. So I've rewritten the ones that test lookup to mock the toolcache, rather than using our own logic to install things into the toolcache. This allows us to test fallback versions like 0.0.0-20230105 more easily and readably by separating out concerns. I've removed one of the cases where the bundle is not cached because this is already tested in the "downloads and caches explicitly requested bundles that aren't in the toolcache" test.

  • Example run before — note the following logs:
    ##[debug]Computed a fallback toolcache version number of 0.0.0-codeql-bundle-20230105 for CodeQL tools version 2.12.0.
    ##[debug]isExplicit: 0.0.0-codeql-bundle-20230105
    ##[debug]explicit? true
    ##[debug]checking cache: /opt/hostedtoolcache/CodeQL/0.0.0-codeql-bundle-20230105/x64
    ##[debug]not found
    ##[debug]Did not find CodeQL tools version 2.12.0 in the toolcache.
    
  • Example run after — note that the bundle is now found in the toolcache:
    ##[debug]Computed a fallback toolcache version number of 0.0.0-20230105 for CodeQL tools version 2.12.0.
    ##[debug]isExplicit: 0.0.0-20230105
    ##[debug]explicit? true
    ##[debug]checking cache: /opt/hostedtoolcache/CodeQL/0.0.0-20230105/x64
    ##[debug]Found tool in cache CodeQL 0.0.0-20230105 x64
    ##[debug]CodeQL found in cache /opt/hostedtoolcache/CodeQL/0.0.0-20230105/x64
    

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.

Copy link
Contributor

@nickfyson nickfyson left a comment

Choose a reason for hiding this comment

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

LGTM

Bug fix itself makes sense, though I have lower confidence in my assessment of the test improvements (all rather unfamiliar) 😅

@henrymercer henrymercer merged commit 43f1a6c into main Jan 26, 2023
@henrymercer henrymercer deleted the henrymercer/fix-fallback-version-number branch January 26, 2023 14:17
@github-actions github-actions bot mentioned this pull request Jan 26, 2023
6 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