Skip to content

Commit

Permalink
Bundle action using esbuild
Browse files Browse the repository at this point in the history
Instead of using a bundled node_modules,
* Run `npm install` before performing various tasks

Change pr-checks to not be particularly picky about the generated
content because it will differ between different versions as everything
is bundled together.
  • Loading branch information
jsoref committed Nov 26, 2024
1 parent db6ee56 commit 34f71d6
Show file tree
Hide file tree
Showing 33 changed files with 3,154 additions and 108 deletions.
3 changes: 2 additions & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
lib/*.js linguist-generated=true
*/*-action.js linguist-generated=true
*/*-action-post.js linguist-generated=true
.github/workflows/__* linguist-generated=true

# Reduce incidence of needless merge conflicts on CHANGELOG.md
Expand Down
6 changes: 6 additions & 0 deletions .github/actions/prepare-test/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ outputs:
runs:
using: composite
steps:
- name: npm install
shell: bash
run: |
if command -v npm >/dev/null 2>/dev/null; then
npm ci
fi
- name: Move codeql-action
shell: bash
run: |
Expand Down
4 changes: 4 additions & 0 deletions .github/actions/update-bundle/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ runs:
shell: bash
run: npm install -g ts-node

- name: Install
shell: bash
run: npm ci

- name: Run update script
working-directory: ${{ github.action_path }}
shell: bash
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/update-bundle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ async function main() {
const previousDefaults: Defaults = JSON.parse(fs.readFileSync('../../../src/defaults.json', 'utf8'));
const newDefaults = await getNewDefaults(previousDefaults);
// Update the source file in the repository. Calling workflows should subsequently rebuild
// the Action to update `lib/defaults.json`.
// the Action.
fs.writeFileSync('../../../src/defaults.json', JSON.stringify(newDefaults, null, 2) + "\n");
}

Expand Down
43 changes: 29 additions & 14 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,23 @@ jobs:
- name: Checkout
uses: actions/checkout@v4

- name: Install
shell: bash
run: npm install

- name: Lint
id: lint
run: npm run-script lint-ci

- name: Upload sarif
uses: github/codeql-action/upload-sarif@v3
# Only upload SARIF for the latest version of Node.js
if: "!cancelled() && matrix.node-types-version == 'current' && !startsWith(github.head_ref, 'dependabot/')"
if: ${{ !cancelled() && matrix.node-types-version == 'current' && !startsWith(github.head_ref, 'dependabot/') }}
with:
sarif_file: eslint.sarif
category: eslint

- name: Update version of @types/node
- name: Override version of @types/node
if: matrix.node-types-version != 'current'
env:
NODE_TYPES_VERSION: ${{ matrix.node-types-version }}
Expand All @@ -52,6 +56,25 @@ jobs:
# `npm install` on Linux.
npm install
# esbuild embeds package.json version details into these files.
# Since the jq step has actively changed package.json, we know that if these files
# are successfully rebuilt (without the changes below), they would be dirty.
#
# In order to allow check-js.sh to verify that it can build them at all, we ignore them,
# delete them, and commit those changes. Thus, when it runs, it will be able to try to
# build them, and as long at they build, it will be happy. If it can't build them, it can
# complain, although that error won't make much sense, because you shouldn't update them
# using the wrong node types version information.
(
echo '*/*-action.js';
echo '*/*-action-post.js'
) >> .gitignore
for action in $(
find * -mindepth 1 -maxdepth 1 -type f -name action.yml
); do
git rm -f "$(dirname "$action")"/*-action*.js
done
if [ ! -z "$(git status --porcelain)" ]; then
git config --global user.email "[email protected]"
git config --global user.name "github-actions[bot]"
Expand All @@ -63,17 +86,6 @@ jobs:
- name: Check generated JS
run: .github/workflows/script/check-js.sh

check-node-modules:
if: github.event_name != 'push' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/releases/v')
name: Check modules up to date
runs-on: macos-latest
timeout-minutes: 45

steps:
- uses: actions/checkout@v4
- name: Check node modules up to date
run: .github/workflows/script/check-node-modules.sh

check-file-contents:
if: github.event_name != 'push' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/releases/v')
name: Check file contents
Expand Down Expand Up @@ -102,7 +114,7 @@ jobs:
npm-test:
if: github.event_name != 'push' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/releases/v')
name: Unit Test
needs: [check-js, check-node-modules]
needs: check-js
strategy:
fail-fast: false
matrix:
Expand All @@ -112,6 +124,9 @@ jobs:

steps:
- uses: actions/checkout@v4
- name: Build
run: |
npm run build
- name: npm test
run: |
# Run any commands referenced in package.json using Bash, otherwise
Expand Down
17 changes: 11 additions & 6 deletions .github/workflows/rebuild.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,19 @@ jobs:
run: |
git fetch origin "$BASE_BRANCH"
# Allow merge conflicts in `lib`, since rebuilding should resolve them.
# Allow merge conflicts in `action(-post|-pre|).js`, since rebuilding should resolve them.
git merge "origin/$BASE_BRANCH" || echo "Merge conflicts detected"
# Check for merge conflicts outside of `lib`. Disable git diff's trailing whitespace check
# since `node_modules/@types/semver/README.md` fails it.
if git -c core.whitespace=-trailing-space diff --check | grep --invert-match '^lib/'; then
echo "Merge conflicts detected outside of lib/ directory. Please resolve them manually."
git -c core.whitespace=-trailing-space diff --check | grep --invert-match '^lib/' || true
git_diff_ignore_generated_actions() {
git diff --check |
grep --invert-match -- '-action-pre\.js$' |
grep --invert-match -- '-action\.js$' |
grep --invert-match -- '-action-post\.js$'
}
if git_diff_ignore_generated_actions | grep -q .; then
echo "Merge conflicts detected outside of generated action js files. Please resolve them manually."
git_diff_ignore_generated_actions || true
exit 1
fi
Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/script/check-js.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ if [ ! -z "$(git status --porcelain)" ]; then
>&2 echo "Failed: Repo should be clean before testing!"
exit 1
fi
# Wipe the lib directory in case there are extra unnecessary files in there
rm -rf lib
# Generate the JavaScript files
npm run-script build
# Check that repo is still clean
if [ ! -z "$(git status --porcelain)" ]; then
# If we get a fail here then the PR needs attention
>&2 echo "Failed: JavaScript files are not up to date. Run 'rm -rf lib && npm run-script build' to update"
>&2 echo "Failed: JavaScript files are not up to date. Run 'npm run-script build' to update"
git status
exit 1
fi
Expand Down
20 changes: 0 additions & 20 deletions .github/workflows/script/check-node-modules.sh

This file was deleted.

18 changes: 18 additions & 0 deletions .github/workflows/script/package.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/sh
bundle_file() {
module=$(dirname "$1")
file=$(perl -ne 'next unless m<'"$2"': .(?:.*/|)(.*\.js)>;print $1' "$1")
if [ -n "$file" ]; then
if [ "$2" = main ]; then
suffix=''
else
suffix="-$2"
fi
./node_modules/.bin/esbuild "lib/$module-action$suffix.js" --bundle --minify --platform=node --outfile="./$module/$file"
perl -pi -e 's/scripts:\{.*?\}/scripts:{}/' "./$module/$file"
fi
};
for a in */action.yml; do
bundle_file $a main;
bundle_file $a post;
done
21 changes: 0 additions & 21 deletions .github/workflows/script/update-node-modules.sh

This file was deleted.

7 changes: 3 additions & 4 deletions .github/workflows/update-dependencies.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ jobs:
run: |
git fetch origin "$BRANCH" --depth=1
git checkout "origin/$BRANCH"
.github/workflows/script/update-node-modules.sh update
npm run build
if [ ! -z "$(git status --porcelain)" ]; then
git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com"
git config --global user.name "github-actions[bot]"
git add node_modules
git commit -am "Update checked-in dependencies"
git commit -am "Update action bundles"
git push origin "HEAD:$BRANCH"
echo "Pushed a commit to update the checked-in dependencies." \
echo "Pushed a commit to update the checked-in action bundles." \
"Please mark the PR as ready for review to trigger PR checks." |
gh pr comment --body-file - --repo github/codeql-action "${{ github.event.pull_request.number }}"
gh pr ready --undo --repo github/codeql-action "${{ github.event.pull_request.number }}"
Expand Down
8 changes: 5 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Ignore for example failing-tests.json from AVA
node_modules/.cache/
# actions are bundled to make this repository lightweight for consumers
node_modules/
# lib is generated by tsc
lib
# Java build files
.gradle/
*.class
Expand All @@ -8,4 +10,4 @@ node_modules/.cache/
# eslint sarif report
eslint.sarif
# for local incremental compilation
tsconfig.tsbuildinfo
tsconfig.tsbuildinfo
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ No user facing changes.

- The CodeQL Action now downloads bundles compressed using Zstandard on GitHub Enterprise Server when using Linux or macOS runners. This speeds up the installation of the CodeQL tools. This feature is already available to GitHub.com users. [#2573](https://github.com/github/codeql-action/pull/2573)
- Update default CodeQL bundle version to 2.19.3. [#2576](https://github.com/github/codeql-action/pull/2576)
- The CodeQL Action is now faster to download by several seconds since `node_modules` are no longer included in this repository. [#2578](https://github.com/github/codeql-action/pull/2578)

## 3.27.0 - 22 Oct 2024

Expand Down
12 changes: 3 additions & 9 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,14 @@ Before you start, ensure that you have a recent version of node (16 or higher) i

### Common tasks

* Transpile the TypeScript to JavaScript: `npm run build`. Note that the JavaScript files are committed to git.
* Run tests: `npm run test`. You’ll need to ensure that the JavaScript files are up-to-date first by running the command above.
* Run the linter: `npm run lint`.
* Transpile the TypeScript to JavaScript: `npm run build`. Note that the bundled action files are committed to git.
* Run tests: `npm run test`. You’ll need to ensure that the `node_modules` are available and JavaScript files are up-to-date first by running the commands above.
* Run the linter: `npm run lint` (requires the first command).

This project also includes configuration to run tests from VSCode (with support for breakpoints) - open the test file you wish to run and choose "Debug AVA test file" from the Run menu in the Run panel.

You may want to run `tsc --watch` from the command line or inside of vscode in order to ensure build artifacts are up to date as you are working.

### Checking in compiled artifacts and `node_modules`

Because CodeQL Action users consume the code directly from this repository, and there can be no build step during an GitHub Actions run, this repository contains all compiled artifacts and node modules. There is a PR check that will fail if any of the compiled artifacts are not up to date. Compiled artifacts are stored in the `lib/` directory. For all day-to-day development purposes, this folder can be ignored.

Only run `npm install` if you are explicitly changing the set of dependencies in `package.json`. The `node_modules` directory should be up to date when you check out, but if for some reason, there is an inconsistency use `npm ci && npm run removeNPMAbsolutePaths` to ensure the directory is in a state consistent with the `package-lock.json`. Note that due to a macOS-specific dependency, this command should be run on a macOS machine. There is a PR check to ensure the consistency of the `node_modules` directory.

### Running the action

To see the effect of your changes and to test them, push your changes in a branch and then look at the [Actions output](https://github.com/github/codeql-action/actions) for that branch. You can also exercise the code locally by running the automated tests.
Expand Down
4 changes: 2 additions & 2 deletions analyze/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,5 @@ outputs:
description: The ID of the uploaded SARIF file.
runs:
using: node20
main: "../lib/analyze-action.js"
post: "../lib/analyze-action-post.js"
main: "analyze-action.js"
post: "analyze-action-post.js"
Loading

0 comments on commit 34f71d6

Please sign in to comment.