-
Notifications
You must be signed in to change notification settings - Fork 202
Workflow to automatically bump cli version if different from the current version #1871
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
Changes from all commits
3f39af2
5420416
2723816
ec9078d
f7d4891
09d744a
c89d267
f2076df
5a42b8b
91dc39b
7f05af9
c72734e
ef14446
365519a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
name: Create a PR if one doesn't exists | ||
description: > | ||
Creates a commit with the current changes to the repo, and opens a PR for that commit. If | ||
any PR with the same title exists, then this action is marked as succeeded. | ||
inputs: | ||
commit-message: | ||
description: > | ||
The message for the commit to be created. | ||
required: true | ||
|
||
title: | ||
description: > | ||
The title of the PR. If empty, the title and body will be determined from the commit message. | ||
default: '' | ||
required: false | ||
|
||
body: | ||
description: > | ||
The body (description) of the PR. The `title` input must be specified in order for this input to be used. | ||
default: '' | ||
required: false | ||
|
||
head-branch: | ||
description: > | ||
The name of the branch to hold the new commit. If an existing open PR with the same head | ||
branch exists, the new branch will be force-pushed to that PR instead of creating a new PR. | ||
required: true | ||
|
||
base-branch: | ||
description: > | ||
The base branch to target with the new PR. | ||
required: true | ||
|
||
token: | ||
description: | | ||
The GitHub token to use. It must have enough privileges to | ||
make API calls to create and close pull requests. | ||
required: true | ||
|
||
runs: | ||
using: composite | ||
steps: | ||
- name: Update git config | ||
shell: bash | ||
run: | | ||
git config --global user.email "github-actions@github.com" | ||
git config --global user.name "github-actions[bot]" | ||
- name: Commit, Push and Open PR | ||
shell: bash | ||
env: | ||
COMMIT_MESSAGE: ${{ inputs.commit-message }} | ||
HEAD_BRANCH: ${{ inputs.head-branch }} | ||
BASE_BRANCH: ${{ inputs.base-branch }} | ||
GH_TOKEN: ${{ inputs.token }} | ||
TITLE: ${{ inputs.title }} | ||
BODY: ${{ inputs.body }} | ||
run: | | ||
set -exu | ||
if ! [[ $(git diff --stat) != '' ]]; then | ||
exit 0 # exit early | ||
fi | ||
# stage changes in the working tree | ||
git add . | ||
git commit -m "$COMMIT_MESSAGE" | ||
git checkout -b "$HEAD_BRANCH" | ||
# CAUTION: gits history changes with the following | ||
git push --force origin "$HEAD_BRANCH" | ||
PR_JSON=$(gh pr list --state open --json number --head "$HEAD_BRANCH") | ||
if [[ $? -ne 0 ]]; then | ||
echo "Failed to fetch existing PRs." | ||
exit 1 | ||
fi | ||
PR_NUMBERS=$(echo $PR_JSON | jq '. | length') | ||
if [[ $PR_NUMBERS -ne 0 ]]; then | ||
echo "Found existing open PR: $PR_NUMBERS" | ||
exit 0 | ||
fi | ||
gh pr create --head "$HEAD_BRANCH" --base "$BASE_BRANCH" --title "$TITLE" --body "$BODY" --assignee ${{ github.actor }} | ||
if [[ $? -ne 0 ]]; then | ||
echo "Failed to create new PR." | ||
exit 1 | ||
fi |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,39 @@ | ||||||||
name: Bump CLI version | ||||||||
on: | ||||||||
workflow_dispatch: | ||||||||
pull_request: | ||||||||
branches: [main] | ||||||||
paths: | ||||||||
- .github/actions/create-pr/action.yml | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add this file as well.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is the last change that needs to be done before we can merge. |
||||||||
- .github/workflows/bump-cli.yml | ||||||||
schedule: | ||||||||
tjgurwara99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
- cron: 0 0 */14 * * # run every 14 days | ||||||||
|
||||||||
permissions: | ||||||||
contents: write | ||||||||
pull-requests: write | ||||||||
|
||||||||
jobs: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a With this change, I think we can avoid changing the setting to grant read+write permissions to workflows on PRs from forks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok. I did try that but it failed, I thought maybe it still needed that permission - didn't know it was because it was from the fork, it got rejected. That's great, thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It still won't work on your fork, which is ok. The If you're making automation changes in future it may be easier to use a branch on the parent repo (otherwise forks are totally fine). |
||||||||
build: | ||||||||
name: Build | ||||||||
runs-on: ubuntu-latest | ||||||||
steps: | ||||||||
- name: Checkout | ||||||||
uses: actions/checkout@v3 | ||||||||
with: | ||||||||
fetch-depth: 1 | ||||||||
- name: Bump CLI | ||||||||
env: | ||||||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||||||||
run: | | ||||||||
scripts/replace-cli-version.sh | ||||||||
- name: Commit, Push and Open a PR | ||||||||
uses: ./.github/actions/create-pr | ||||||||
with: | ||||||||
token: ${{ secrets.GITHUB_TOKEN }} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the token doesn't have permission to push. I would think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the right permission, but it won't work while this PR runs from a fork (by design). All write permissions get downgraded to read on forks. |
||||||||
base-branch: main | ||||||||
head-branch: github-action/bump-cli | ||||||||
commit-message: Bump CLI version from ${{ env.PREVIOUS_VERSION }} to ${{ env.LATEST_VERSION }} for integration tests | ||||||||
title: Bump CLI Version to ${{ env.LATEST_VERSION }} for integration tests | ||||||||
body: > | ||||||||
Bumps CLI version from ${{ env.PREVIOUS_VERSION }} to ${{ env.LATEST_VERSION }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
#!/bin/bash | ||
|
||
VERSIONS=$(gh api -H "Accept: application/vnd.github+json" /repos/github/codeql-cli-binaries/releases | jq -r '.[].tag_name' | head -2) | ||
|
||
# we are exporting these variables so that we can access these variables in the workflow | ||
LATEST_VERSION=$(echo $VERSIONS | awk '{ print $1 }') | ||
PREVIOUS_VERSION=$(echo $VERSIONS | awk '{ print $2 }') | ||
|
||
Comment on lines
+6
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we do a minor version bump, we need to add the new version instead of replacing it. Doing something like this can check if the minor version has changed: LATEST_MINOR_VERSION="$(echo $LATEST_VERSION | awk 'BEGIN{FS=".";}{print $1$2}')"
PREVIOUS_MINOR_VERSION="$(echo $PREVIOUS_VERSION | awk 'BEGIN{FS=".";}{print $1$2}')"
if [ $LATEST_MINOR_VERSION != $PREVIOUS_MINOR_VERSION ];
then
# handle case where minor version is changed
else
# handle case where patch version is changed
fi There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the list of versions in [
"v2.11.5",
"v2.7.6",
"v2.8.5",
"v2.9.4",
"v2.10.5",
"nightly"
] and a version [
"v2.11.6", // this is the new version that got added
"v2.11.5",
"v2.7.6",
"v2.8.5",
"v2.9.4",
"v2.10.5",
"nightly"
] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The minor version bump would be
(Or even, we could add the new minor version and remove the oldest one (i.e. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we can actually remove 2.7.6. The removal depends on the support GHES versions and GHES 3.3 is still supported. GHES 3.3 recommends CodeQL 2.7.6, so we can't remove that version yet. In general, I don't think we can remove a version automatically because it's not strictly aligned with the CodeQL CLI releases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GHES 3.3 is only supported for another 6 days (2023-01-18), so it kind of aligns with the CodeQL CLI (give or take 1 week 😄). But we don't have to automate the removal (or at least not in this PR) if we want to keep that separate/manual! 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lol, I always get confused with semver 😂 For some reason I thought number after the second dot was minor release 😅 I think the removal of a version should be done through a separate workflow/action - from what I can tell, there doesn't seem to be an automated way of telling the which version should be removed. I also think that given a manual trigger with the some inputs (like for example in the image below) and manually adding the version to remove would be better unless we want to completely automate it which may not be as simple or quick 😄 Let me know though whichever way is preferred and I can think on what I can do. PS: I think this issue was for the patch release being released every two weeks or so, so this might benefit from a separate issue which I can take (this PR has been open for a while now - I want to close it 😂 of course if it meets the acceptable standards 😉) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good points @tjgurwara99. I expect that this workflow will always be triggered manually since the timing of when to run this workflow depends on a bunch of human decisions. I'm personally fine with focusing on patch releases for now and working on a new PR to add minor version bumps. I think we can avoid creating a second workflow for minor version bumps and handle that with a new set of inputs. And removing releases when a GHES version drops out of support is probably something that should be done manually since it also means we can remove certain feature flags that only pertain to the older version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added the issue and assigned it to myself 😄 will get to it whenever I get time |
||
echo "LATEST_VERSION=$LATEST_VERSION" >> $GITHUB_ENV | ||
echo "PREVIOUS_VERSION=$PREVIOUS_VERSION" >> $GITHUB_ENV | ||
|
||
aeisenberg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sed -i "s/$PREVIOUS_VERSION/$LATEST_VERSION/g" supported_cli_versions.json | ||
sed -i "s/$PREVIOUS_VERSION/$LATEST_VERSION/g" extensions/ql-vscode/test/vscode-tests/ensureCli.ts |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
[ | ||
"v2.11.6", | ||
"v2.7.6", | ||
tjgurwara99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"v2.8.5", | ||
"v2.9.4", | ||
"v2.10.5", | ||
"nightly" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this needs to change to
GITHUB_TOKEN
so that the git commands can work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, the checkout step has already taken care of the ambiguity. Both
GH_TOKEN
andGITHUB_TOKEN
work 😄 - I also think there might actually be no need for this since theGH_TOKEN
is not being used in this action at all...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need it for
gh pr create
.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this is after the
checkout
step which sets up the wholegh
cli stuff right? We're calling it as a composite step in another action, and this itself is not a standalone action I believe 😅 Unless I'm mistaken 😓There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. I believe the
checkout
step sets up Git, and uses the token information to set up Git remotes and credentials, but does not dogh auth
for the CLI. I believe the CLI will still be expecting a token inGH/GITHUB_TOKEN
and will attempt interactive auth if that's missing (which you don't want in CI).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I'll keep it as it is then 😄