Remove node_modules and compile typescript into minified scripts
- Bundle action using esbuild -- the main change - fixes #2542
- deleted
libandnode_modules - changed
*/action.ymlto use local minified bundled files - minified+bundled:
*/*action.js+*/*action-post.js - added
package.shwhich is responsible for regenerating the minified files and fixing up the action files.
- deleted
Merge / deployment checklist
Before github/codeql-action@v3 662472033e021d55d94146f66f6058822b0b39fd (6s)
Fri, 01 Nov 2024 13:42:39 GMT Prepare all required actions
Fri, 01 Nov 2024 13:42:39 GMT Getting action download info
Fri, 01 Nov 2024 13:42:39 GMT Download action repository 'github/codeql-action@v3' (SHA:662472033e021d55d94146f66f6058822b0b39fd)
Fri, 01 Nov 2024 13:42:44 GMT Run ./.git/check-spelling-actions/upload-sarif
Fri, 01 Nov 2024 13:42:44 GMT Run github/codeql-action/upload-sarif@v3
Fri, 01 Nov 2024 13:42:44 GMT Uploading results
Fri, 01 Nov 2024 13:42:44 GMT Processing sarif files: ["/tmp/tmp.yvZ4B9jZAF"]
Fri, 01 Nov 2024 13:42:44 GMT Validating /tmp/tmp.yvZ4B9jZAF
Fri, 01 Nov 2024 13:42:44 GMT Combining SARIF files using the CodeQL CLI
Fri, 01 Nov 2024 13:42:44 GMT Adding fingerprints to SARIF file. See https://docs.github.com/en/enterprise-cloud@latest/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#providing-data-to-track-code-scanning-alerts-across-runs for more information.
Fri, 01 Nov 2024 13:42:44 GMT Uploading results
Fri, 01 Nov 2024 13:42:45 GMT Successfully uploaded results
After check-spelling-sandbox/github-codeql-action@thin 3dc118c13c28b2804932fce08b479e77e5007079 (2s)
Tue, 05 Nov 2024 13:22:18 GMT Prepare all required actions
Tue, 05 Nov 2024 13:22:18 GMT Getting action download info
Tue, 05 Nov 2024 13:22:18 GMT Download action repository 'check-spelling-sandbox/github-codeql-action@thin' (SHA:3dc118c13c28b2804932fce08b479e77e5007079)
Tue, 05 Nov 2024 13:22:19 GMT Run ./.git/check-spelling-actions/upload-sarif
Tue, 05 Nov 2024 13:22:19 GMT Run check-spelling-sandbox/github-codeql-action/upload-sarif@thin
Tue, 05 Nov 2024 13:22:20 GMT Uploading results
Tue, 05 Nov 2024 13:22:20 GMT Processing sarif files: ["/tmp/tmp.CoFUHRgCYx"]
Tue, 05 Nov 2024 13:22:20 GMT Validating /tmp/tmp.CoFUHRgCYx
Tue, 05 Nov 2024 13:22:20 GMT Combining SARIF files using the CodeQL CLI
Tue, 05 Nov 2024 13:22:20 GMT Adding fingerprints to SARIF file. See https://docs.github.com/en/enterprise-cloud@latest/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#providing-data-to-track-code-scanning-alerts-across-runs for more information.
Tue, 05 Nov 2024 13:22:20 GMT Uploading results
Tue, 05 Nov 2024 13:22:20 GMT Successfully uploaded results
Thanks for your contribution. I appreciate the work you've put into this change. Before I can have a deeper look, there are some things I'd like to see:
- Please separate the spelling and other minor changes into a different PR.
- Can you explain why you need to add the upload/download artifact shims? We are not running these tests on GHES. We probably do need to update the remaining references to
upload-artifact@v3, but I see no reason we need both. - How does this change the development process? Do we need to run
npm i && npm buildbefore developing locally?CONTRIBUTING.mdwill need to be updated (but this can happen after we do a deeper review).
- Please separate the spelling and other minor changes into a different PR.
- Sure, I've spun up a check pass for this, but moved to #2580 (note that it will have conflicts in one compiled action as
MacOSis present in it)
- Sure, I've spun up a check pass for this, but moved to #2580 (note that it will have conflicts in one compiled action as
- Can you explain why you need to add the upload/download artifact shims? We are not running these tests on GHES. We probably do need to update the remaining references to upload-artifact@v3, but I see no reason we need both.
- The warnings/having v3 stuff come in were annoying and it looked like it mostly shouldn't be there. The idea that one can't properly upgrade a package so that everyone uses a single version of an action is a problem. I release an action that uses this action, and it's pretty crazy for me to have to think about actions that I use and potentially pull in two different versions just so that my action won't break if it's used in GHES (I don't have access to GHES, so I really don't know if it will break or not). -- Spirit moved to https://github.com/github/codeql-action/pull/2583
- How does this change the development process? Do we need to run npm i && npm build before developing locally?
CONTRIBUTING.md will need to be updated (but this can happen after we do a deeper review).
- There's a paragraph about not running
npm install. The text aboutnode_moduleswill need to be removed. I'm not actually sure about the general macOS bits. I built this on macOS and there's no sign offsevents, so it's possible that the macOS only thing will no longer apply. - Yes, the text about not running
npm installwill need to change. I think it should generally suggestnpm ci && npm run buildinstead ofnpm run build - Initial changes
- There's a paragraph about not running
That second item is mostly to deal with annoying things like: https://github.com/check-spelling-sandbox/github-codeql-action/actions/runs/11717020444
Export file baseline information (windows-latest, nightly-latest) The following actions use a deprecated Node.js version and will be forced to run on node20: actions/upload-artifact@v3. For more info: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/ Export file baseline information (macos-latest, nightly-latest) The following actions use a deprecated Node.js version and will be forced to run on node20: actions/upload-artifact@v3. For more info: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/ Export file baseline information (ubuntu-latest, nightly-latest) An error occurred when trying to refresh keys from hkp://keyserver.ubuntu.com: Error: The process '/usr/bin/gpg' failed with exit code 2 Export file baseline information (ubuntu-latest, nightly-latest) An error occurred when trying to refresh keys from hkp://keyserver.ubuntu.com: Error: The process '/usr/bin/gpg' failed with exit code 2 Deprecation notice: v1, v2, and v3 of the artifact actions The following artifacts were uploaded using a version of actions/upload-artifact that is scheduled for deprecation: "with-baseline-information-macos-latest-nightly-latest.sarif.json", "with-baseline-information-windows-latest-nightly-latest.sarif.json". Please update your workflow to use v4 of the artifact actions. Learn more: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/
Of which there were a lot. Anyway, the commit is currently nowhere. I can make a PR for it, or a simpler one that just upgrades. But the current state is https://github.com/github/codeql-action/pull/2046#issuecomment-1861579420. I think someone (me?) may have merged and then had the upgrade reverted when someone realized it broke GHES, but it's too late in my day to find that item...
(In developing this PR, my goal was 0 errors and 0 warnings before opening it, which took a bit of poking.)
After you address the merge conflicts then I can run the tests again.
Also, in a separate PR, you can upgrade all of the references to *-artifact@v3 to @v4 in our workflows. These will need to change before @v3 is removed.
I hadn't thought through .github/workflows/update-dependencies.yml
It's referenced by: https://github.com/github/codeql-action/blob/3ef4c0845750690942ece9abe29a853edce0f43c/.github/workflows/script/check-node-modules.sh#L16
https://github.com/github/codeql-action/blob/3ef4c0845750690942ece9abe29a853edce0f43c/.github/update-release-branch.py#L100-L101
https://github.com/github/codeql-action/blob/3ef4c0845750690942ece9abe29a853edce0f43c/.github/dependabot.yml#L9-L10
https://github.com/github/codeql-action/blob/3ef4c0845750690942ece9abe29a853edce0f43c/.github/workflows/post-release-mergeback.yml#L136-L137
https://github.com/github/codeql-action/blob/3ef4c0845750690942ece9abe29a853edce0f43c/.github/workflows/post-release-mergeback.yml#L156-L163
Thinking about what they're doing also gave me a headache. In the old world, updating dependencies involved committing all of the files in node_modules (and maybe committing lib??). In the new world, if you update package.json/package-lock.json then you need to update all of the actions (as managed by package.sh) which means someone somewhere still needs to do that.
For dependabot, that appeared as https://github.com/github/codeql-action/pull/2575#issuecomment-2455312166
I think that the same stuff still needs to exist, in that we would want a commit (for at least dependabot) that updates the bundled files.
I think what we want to do is to keep the workflow but change its work from committing the node_modules to committing the bundles see https://github.com/github/codeql-action/pull/2578/commits/f73e1b66a2dae6031252e5daa459a533ea2ea654.
I've updated .gitattributes.
Thanks for your work on this. We've now fixed this issue in a different way. See https://github.com/github/codeql-action/pull/3054. Closing this PR now.