Minor improvements for CodeQL ${workspace} pack dependencies and scripts#337
Minor improvements for CodeQL ${workspace} pack dependencies and scripts#337data-douser wants to merge 3 commits intomainfrom
${workspace} pack dependencies and scripts#337Conversation
Update qlpack.yml dependency declarations to use "$workspace" references instead of hard-coding each version change. This should, in theory, also make our pack installations more efficient as "codeql pack install" is codeql-workspace.yml aware.
There was a problem hiding this comment.
Pull request overview
This PR adjusts how internal CodeQL pack dependencies are referenced (moving to ${workspace}), improves the pack install/upgrade scripts by avoiding cached .codeql/ directories during pack discovery, and reduces how often the CodeQL CLI update workflow runs.
Changes:
- Switch internal dependencies in multiple
qlpack.ymlfiles from fixed versions to${workspace}. - Update
install-packs.sh/upgrade-packs.shto skip.codeql/directories when locatingqlpack.yml. - Change the
update-codeql.ymlscheduled workflow from daily to every 3 days.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/upgrade-packs.sh | Avoids traversing cached .codeql/ directories when upgrading packs. |
| scripts/install-packs.sh | Avoids traversing cached .codeql/ directories when installing packs. |
| javascript/heuristic-models/tests/qlpack.yml | Uses ${workspace} for internal test-pack dependency. |
| javascript/frameworks/xsjs/test/qlpack.yml | Uses ${workspace} for internal test-pack dependencies. |
| javascript/frameworks/xsjs/src/qlpack.yml | Uses ${workspace} for dependency in a publishable pack. |
| javascript/frameworks/ui5/test/qlpack.yml | Uses ${workspace} for internal test-pack dependencies. |
| javascript/frameworks/ui5/src/qlpack.yml | Uses ${workspace} for dependency in a publishable pack. |
| javascript/frameworks/ui5-webcomponents/test/qlpack.yml | Uses ${workspace} for internal test-pack dependency. |
| javascript/frameworks/cap/test/qlpack.yml | Uses ${workspace} for internal test-pack dependencies. |
| javascript/frameworks/cap/src/qlpack.yml | Uses ${workspace} for dependency in a publishable pack. |
| .github/workflows/update-codeql.yml | Reduces scheduled runs by changing cron cadence. |
| on: | ||
| workflow_dispatch: | ||
| # Nightly check for new CodeQL CLI releases | ||
| # Check for new CodeQL CLI releases every 3 days |
There was a problem hiding this comment.
The cron 30 0 */3 * * runs on day-of-month intervals (1,4,7,...) and will not be a strict “every 3 days” cadence across month boundaries. If you need an every-72-hours cadence, GitHub Actions cron can’t express that reliably; otherwise consider wording the comment as “every 3 days (day-of-month step)”.
| # Check for new CodeQL CLI releases every 3 days | |
| # Check for new CodeQL CLI releases every 3 days (day-of-month step) |
| # Find all qlpack.yml files under this framework and install their packs | ||
| find "${REPO_ROOT}/${framework_path}" -name "qlpack.yml" -type f | sort | while read -r qlpack_file; do | ||
| # Exclude .codeql directories which contain cached packs from previous installs | ||
| find "${REPO_ROOT}/${framework_path}" -name ".codeql" -prune -o -name "qlpack.yml" -type f -print | sort | while read -r qlpack_file; do |
There was a problem hiding this comment.
The find prune expression matches any path named .codeql (including files) and relies on operator precedence, which can behave differently across find implementations. To make the intent unambiguous, restrict pruning to directories and group the expression (e.g., prune only when -type d -name .codeql).
| find "${REPO_ROOT}/${framework_path}" -name ".codeql" -prune -o -name "qlpack.yml" -type f -print | sort | while read -r qlpack_file; do | |
| find "${REPO_ROOT}/${framework_path}" \( -type d -name ".codeql" -prune \) -o \( -name "qlpack.yml" -type f -print \) | sort | while read -r qlpack_file; do |
| # Find all qlpack.yml files under this framework and upgrade their packs | ||
| find "${REPO_ROOT}/${framework_path}" -name "qlpack.yml" -type f | sort | while read -r qlpack_file; do | ||
| # Exclude .codeql directories which contain cached packs from previous installs | ||
| find "${REPO_ROOT}/${framework_path}" -name ".codeql" -prune -o -name "qlpack.yml" -type f -print | sort | while read -r qlpack_file; do |
There was a problem hiding this comment.
The find prune expression currently prunes any path named .codeql and depends on find operator precedence. To avoid accidentally pruning non-directories and to keep behavior consistent across platforms, prefer pruning only directories (-type d -name .codeql -prune) with explicit grouping.
| find "${REPO_ROOT}/${framework_path}" -name ".codeql" -prune -o -name "qlpack.yml" -type f -print | sort | while read -r qlpack_file; do | |
| find "${REPO_ROOT}/${framework_path}" \( -type d -name ".codeql" -prune \) -o \( -name "qlpack.yml" -type f -print \) | sort | while read -r qlpack_file; do |
| dependencies: | ||
| codeql/javascript-all: "^2.6.24" | ||
| advanced-security/javascript-sap-cap-all: "2.25.0" | ||
| advanced-security/javascript-sap-cap-all: ${workspace} |
There was a problem hiding this comment.
${workspace} makes this dependency resolvable only when the dependent pack is present locally in the same workspace. Since this pack is one of the packs published to GHCR (see scripts/publish-packs.sh), external consumers installing from the registry will likely fail to resolve this dependency. Prefer pinning to the released version (or a compatible range) for published packs, and reserve ${workspace} for local-only/test packs or developer scripts.
| advanced-security/javascript-sap-cap-all: ${workspace} | |
| advanced-security/javascript-sap-cap-all: "^2.25.0" |
| dependencies: | ||
| codeql/javascript-all: "^2.6.24" | ||
| advanced-security/javascript-sap-ui5-all: "2.25.0" | ||
| advanced-security/javascript-sap-ui5-all: ${workspace} |
There was a problem hiding this comment.
Using ${workspace} here makes the dependency resolvable only in a local multi-pack workspace. Because this pack is published (per scripts/publish-packs.sh), consumers installing it from GHCR may not be able to resolve the dependency. Consider keeping a concrete version (or range) in published packs and using ${workspace} only for non-published/test packs or local tooling.
| advanced-security/javascript-sap-ui5-all: ${workspace} | |
| advanced-security/javascript-sap-ui5-all: "^2.25.0" |
| dependencies: | ||
| codeql/javascript-all: "^2.6.24" | ||
| advanced-security/javascript-sap-xsjs-all: "2.25.0" | ||
| advanced-security/javascript-sap-xsjs-all: ${workspace} |
There was a problem hiding this comment.
${workspace} ties this dependency to a local workspace layout. Since javascript/frameworks/xsjs/src is a publishable pack, installing it from GHCR is likely to fail unless the dependency can be resolved to a concrete published version. Prefer pinning to the release version (or compatible semver range) in published packs.
| advanced-security/javascript-sap-xsjs-all: ${workspace} | |
| advanced-security/javascript-sap-xsjs-all: "^2.25.0" |
What This PR Contributes
This pull request updates the way dependencies are specified in various
qlpack.ymlfiles to use workspace-relative references, and improves the robustness of pack installation and upgrade scripts by excluding cached directories. It also adjusts the schedule for the CodeQL CLI update workflow to run less frequently and fixes pack install/upgrade scripts to ignore.codeql/directories.Future Works
N/A