-
Notifications
You must be signed in to change notification settings - Fork 97
Start turning GitGitGadget into a set of GitHub Actions and workflows #1392
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
base: main
Are you sure you want to change the base?
Conversation
There were a few linting issues I had missed, so I added another reaction (via https://github.com/gitgitgadget/gitgitgadget/actions/runs/5916258877). |
fyi, I am reviewing this. Just taking some time. |
@webstech no worries! |
This PR will need to be updated using #1473 once that one is merged, to be used in a post-Action of the |
2f31cb4
to
fac3d88
Compare
fbb2ca1
to
992be41
Compare
Network calls in tests not only make everything a lot slower than necessary, but also add an undesirable source of test failures. So let's avoid them. Signed-off-by: Johannes Schindelin <[email protected]>
When testing whether a given `/allow` command yields the expected result, let's mock out the actual GitHub REST API call. Signed-off-by: Johannes Schindelin <[email protected]>
It really needs to be reusable instead of being stuck in the script. In particular since we're about to modify GitGitGadget such that it can be run as a GitHub Action instead. Signed-off-by: Johannes Schindelin <[email protected]>
ec83f79
to
636881b
Compare
It was a bit hard for me to follow the code, getting confused by two separate `getConfig()` functions that do different things (the one from `gitgitgadget-config` initializes the default config, the one from `project-config` gets a singleton that might still be uninitialized). Let's unconfuse things by making the config a parameter of the constructor. It can still be omitted, in which case the default config is used. Note: This commit is only concerned with the CIHelper class, because we will use it from light-weight GitHub Actions in the future. The rest of the code really needs to use the singleton up until the time when we'll get around to The Big Refactor. Signed-off-by: Johannes Schindelin <[email protected]>
b2b129a
to
43af519
Compare
When no `workDir` is specified, default to using the `git/` directory in the current directory. This will be initialized as a bare repository in the upcoming GitHub Actions. Signed-off-by: Johannes Schindelin <[email protected]>
In #1974, I changed the way the push token is handed down from CIHelper to GitGitGadget instead of relying on the Git config (or environment) to specify it implicitly. Let's do the same for the URL of the repository to which to push the Git notes and patch series iterations' tags. Signed-off-by: Johannes Schindelin <[email protected]>
…Actions Some of them need a local (partial) clone of the Git mailing list, most don't, though. So let's make it easy for the Actions to specify what they need. Signed-off-by: Johannes Schindelin <[email protected]>
This will be used in the upcoming `handle-pr-comment` PR. Signed-off-by: Johannes Schindelin <[email protected]>
This will be used to convert GitGitGadget to a GitHub Action. Signed-off-by: Johannes Schindelin <[email protected]>
The plan is to turn GitGitGadget into a set of GitHub Actions that can be run conveniently from GitHub workflows. To that end, we will add subdirectories (e.g. `init/`, `handle-prs/`, etc) that reflect the individual functionality that is currently called in Azure Pipelines via subcommands of the `misc-helper` script. Those subdirectories will contain the bare minimum required to implement a Javascript Action that calls into the CIHelper class: an `action.yml` file to declare the inputs/outputs, and a minimal `index.js` that consumes the inputs and passes them to the CIHelper. The CIHelper class will be used by the Action via as a single, dependency-free `dist/index.js`, and it will be the only dependency of the Action's `index.js`. In particular, we must not let the Action's minimal `index.js` depend on `@actions/core` because it simply won't be there. So let's add it as a dependency of the CIHelper class and provide a function to allow calling the imported `@actions/core` functions from the Actions' `index.js`. Signed-off-by: Johannes Schindelin <[email protected]>
Preparing for turning GitGitGadget into a bunch of GitHub Actions, we install `@vercel/ncc`, a 'simple CLI for compiling a Node.js module into a single file, together with all its dependencies, gcc-style.'. This will allow us to bundle a minimal set of Javascript files and publish the result via a tag. The idea is to run `ncc build -s -m lib/ci-helper.ts -o dist/` to obtain a minimized `dist/index.js` that has no additional dependencies, and then have subdirectories (e.g. `update-prs/`) with the individual `action.yml` and `index.js` that starts with the following line const { CIHelper } = require('../dist/index.js') This will allow us to publish a single tag that backs the various functionalities via different GitHub Actions, e.g: - uses: gitgitgadget/gitgitgadget/update-prs@v1 with: config: ${{ vars.GITGITGADGET_CONFIG }} Signed-off-by: Johannes Schindelin <[email protected]>
This adds a convenience `script` entry to the `package.json` file that allows bundling everything into `dist/`, dependency-free. Since `ncc` generates a lot of `.d.ts` files (which are not required to run the GitHub Action), we go ahead and tell Git to ignore these. Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
According to vercel/ncc#829, `ncc` has some magic that includes the resource in the bundled package if the path is specified using `path.join(__dirname, ...)`. However, we're already using ESModule, therefore we need to use the `import.meta.url` syntax (`import.meta.dirname` unfortunately does not seem to work in my hands). Having said that, the promise was that `ncc` would include this asset in the output, but this is not true for me. Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
I am about to add some JavaScript code, where semicolons are frequently unneeded. Let's skip them there. Signed-off-by: Johannes Schindelin <[email protected]>
The idea is to let `ncc` turn the CIHelper code into a dependency-free `dist/index.js` and then remove all files other than `*/action.yml` and `*/index.js`, commit and tag that. That's the GitHub Action, ready to be used in GitHub workflows. Since `ncc` produces an ECMAScript module, we cannot use the old-style `require()` call but need to use `await import()` instead. Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
This adds a reaction to a PR comment in my fork, to verify that GitGitGadget can do such things when called via the new GitHub Action. Signed-off-by: Johannes Schindelin <[email protected]>
This is the culmination of all the preparation: The first GitHub Action that will be used in a GitHub workflow that will replace the trusty Azure Pipelines we used for so long. Signed-off-by: Johannes Schindelin <[email protected]>
When debugging locally, say, a `/submit` command, it is incredibly not okay if emails are sent or if comments are added to the respective PR on GitHub. Setting the `GITGITGADGET_DEBUG` variable will now prevent all such things from happening. Signed-off-by: Johannes Schindelin <[email protected]>
These files were generated via: npm ci && npm run build && npm run build-dist Note that recent `ncc` also generates tons of `.d.ts` files and also a `package.json` file. We don't need the `.d.ts` files (and `.gitignore` already helpfully matches those), but we do need a minimal `package.json` file that declares this an ES module, and we do need the `WELCOME.md` resource, therefore this was staged using: echo '{"type":"module"}' >dist/package.json cp -R res dist/ git add -A dist/ Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
For easier debugging via `CIHelper.git(...)`. Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
@webstech 🎉 this workflow run just successfully handled this It's slowly all coming together! |
@webstech while talking about this PR to @mjcheetham, he brought up the question whether this needs to be a set of Actions or whether it would be better to have a single Action covering all the functionality. I can see pros and cons to both approaches. Just to illustrate the difference: The approach currently chosen in this PR+ - uses: gitgitgadget/gitgitgadget/handle-pr-comment@v1
+ with:
+ gitgitgadget-git-access-token: ${{ steps.gitgitgadget-git-token.outputs.token }}
+ git-git-access-token: ${{ steps.git-git-token.outputs.token }}
+ dscho-git-access-token: ${{ steps.dscho-git-token.outputs.token }}
+ smtp-options: '{"user": "[email protected]", "host": "smtp.gmail.com", "pass": ${{ toJSON(secrets.GITGITGADGET_SMTP_PASS) }}}'
+ pr-comment-url: ${{ inputs.pr-comment-url }} The biggest pro I see here is that it allows for defining exactly which inputs are required for any given functionality ( With a single Action+ - uses: gitgitgadget/gitgitgadget@v1
+ with:
+ gitgitgadget-git-access-token: ${{ steps.gitgitgadget-git-token.outputs.token }}
+ git-git-access-token: ${{ steps.git-git-token.outputs.token }}
+ dscho-git-access-token: ${{ steps.dscho-git-token.outputs.token }}
+ smtp-options: '{"user": "[email protected]", "host": "smtp.gmail.com", "pass": ${{ toJSON(secrets.GITGITGADGET_SMTP_PASS) }}}'
+ command: handle-pr-comment ${{ inputs.pr-comment-url }} The biggest pro I see here that we will need only one @webstech what is your opinion? |
This is a continuation of the work done in gitgitgadget/gitgitgadget#1392. Signed-off-by: Johannes Schindelin <[email protected]>
This is based on the discussion at #609.
Migration Plan: Azure Pipelines to GitHub Actions
This document outlines the strategy for migrating GitGitGadget's automation from Azure Pipelines to GitHub Actions.
Background & Motivation
For years, we have relied on a set of Azure Pipelines backed by a database maintained in Git notes. This global state required careful handling of concurrency, which we solved by running all jobs inside a dedicated Azure Pipeline runner on a single Azure VM. This setup guaranteed exclusive access, allowing the pipeline to read, update, and push Git notes without concurrent interference.
However, this approach is fragile and problematic:
Update GitGitGadget's PRs
, no other Azure Pipeline can run, which often results in delays of, say,/submit
ing patch series.Project Goal
My overarching goal is to make GitGitGadget more robust, maintainable, and contributor-friendly by migrating to GitHub Actions and workflows.
Migration Approach
CIHelper
class serves as the main entry point.action.yml
andindex.js
.GitHub Actions & Workflows
gitgitgadget/gitgitgadget
updates a dedicatedv1
branch. This branch contains the full commit history of the main branch (via merging it) but only the transpiled and bundled JavaScript at its tip commit's tree, ready for use as a GitHub Action.v1
branch and maintains an incrementalv1.<number>
tag.gitgitgadget-workflows
org.Concurrency Considerations
PR Checks Integration
gitgitgadget-workflows
to the respective PRs. This approach has been successfully used in the Git for Windows project, and those learnings will be invaluable.Detailed overview of the new GitHub Actions
git-mailing-list-mirror
updatesseen
updatesupdateOpenPrs()
,updateCommitMappings()
andhandleOpenPRs()
seen
updatesgitster/seen
toupstream/seen
Oh, and best of all: the way the Azure Pipelines run GitGitGadget via
misc-helper
will continue working all the way through the migration.