| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
cached and latest are failing (expected as they only support 5.7.0 and not 5.7.1). I can conditionally setup 5.7.0 for latest, or we could wait to merge this PR until latest and cached reflect the last release which should support 5.7.1 🤔 (but this would mean that we're not testing nightly-latest until then). |
Sorry, something went wrong.
|
For cached to reflect the latest release, we'd need to wait another 2 weeks or so. I think perhaps while Swift is in beta, we should just test that the Swift extractor is compatible with the Swift version it declares: codeql/experimental/swift/tools/*/extractor --version | awk '/version/ { print $3 }'
This mimics the internal tests, and while it's further away from customer setups, it avoids us having to update these tests every release while Swift is in beta. |
Sorry, something went wrong.
Oh, that's a nice workaround. Updated 👍 (although I guess the internal tests should also be testing prior versions of Swift to catch problems similar to the one we just encountered) |
Sorry, something went wrong.
|
Ah, that approach isn't very easy because we need to setup Swift before running codeql-action/init, but at that point CodeQL hasn't been downloaded yet, so we can't get the Swift version from the CodeQL distribution. In that case, we might have to stop running this test on cached due to the bumpiness of which CodeQL version the cache contains, and manually specify which Swift versions correspond to each bundle. Optionally, we could define a top-level setup-swift boolean in each generated workflow definition, and update sync.py to add the appropriate setup step to the Action. |
Sorry, something went wrong.
|
I've updated all 4 Swift-related PR checks to use 5.7.1, and stopped testing cached where I could but running into a couple issues:
Is it a viable solution to just wait until the newest CLI version that supports 5.7.1 makes it into latest and cached, and then merge this PR? It doesn't solve the longer-term problem of extractors not providing backwards compatibility with prior Swift versions, but hopefully this is not a common occurrence even in beta, and ideally should be tested in the CLI. |
Sorry, something went wrong.
|
Aditya suggested checking the Swift version from the extractor after the init step as a stopgap until the newest CLI makes it to latest. Will give that a try now. |
Sorry, something went wrong.
|
I think the above approach should work, but we are still getting this error — /home/runner/work/_temp/4bebb1c6-be8f-43cf-9dfd-75dc640e9229.sh: line 1: codeql/experimental/swift/tools/*/extractor: No such file or directory Is this perhaps the wrong path, or has the extractor been cleaned up after the init step? |
Sorry, something went wrong.
Hmm, this is a little annoying. I think the problem is that the prepare-test step moves everything in the CodeQL Action repo apart from the multi-language test repository into action: Run mkdir ../action
mkdir ../action
mv * .github ../action/
mv ../action/tests/multi-language-repo/{*,.github} .
mv ../action/.github/workflows .github
I expect that if we replace the call by ../action/.github/setup-swift then that should work. |
Sorry, something went wrong.
Thank you — that did it! That makes sense 😸 |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks sensible, good work! Minor Bash suggestions.
Sorry, something went wrong.
Co-authored-by: Aditya Sharad <6874315+adityasharad@users.noreply.github.com>
|
Hm... the PR checks I changed don't seem to have run anymore. I'll close and reopen to see if this triggers them. EDIT: Could be that the file path should be ./../action/.github/setup-swift rather than ../action/.github/setup-swift? Not sure why that would prevent the checks from running at all, but have updated for consistency. |
Sorry, something went wrong.
|
Ok.. that seems to have done it 😆 re-requesting review (again), sorry! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR changes the setup-swift Action to set up the version of Swift the extractor declares (except on Windows). It also adds the nightly-latest PR check back to a couple of PR checks, which were removed in #1412 to unblock the release.
Note that after this PR merges, we should remember to update the corresponding integration test within the CLI, and include the bumped setup-swift SHA (#1415) as well.
When the newest version of the CLI (that supports 5.7.1) makes it to latest and cached, we should be able to remove this logic and unconditionally setup 5.7.1, as future versions of the extractor should maintain backwards compatibility with prior Swift versions.
Merge / deployment checklist