-
Notifications
You must be signed in to change notification settings - Fork 98
Add a set of GitHub Actions to perform GitGitGadget's job #1982
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
Conversation
lib/ci-helper.ts
Outdated
| process.env.GIT_EXEC_PATH = "/usr/lib/git-core"; | ||
|
|
||
| // get the access tokens via the inputs of the GitHub Action | ||
| this.setAccessToken("gitgitgadget", core.getInput("gitgitgadget-git-access-token")); |
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.
Could the input names be less GitGitGadget specific to make it simpler for other projects? Maybe more purpose oriented.
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 would have loved that! Unfortunately, it seems that the same access token cannot be reused for multiple repositories unless they are in the same org (see here, the underlying limitation is that the access tokens can only be created per installation, which is per-org).
So the main problem here is GitGitGadget's now-problematic historical development, where it first used its own Git fork for all the work (gitgitgadget/git), but was later extended to work on git/git itself (albeit with a different GitHub App, one that lacks write permission to the repository, for security reasons).
There is probably a good way to talk about this kind of setup, something like "primary-repo" and "own-fork" would be descriptive but probably too unintuitive to be useful when extending GitGitGadget to support 3rd-party projects.
Hmm. @webstech @mjcheetham how about pr-repo-token? And git-git-token would become upstream-repo-token? And dscho-git-token would either go away (that would be the easiest) or would become test-repo-token?
Maybe I should throw in a diagram?
graph TD
user["user (contributor)"]
upstream-repo["upstream-repo (authoritative project repo)"]
pr-repo["pr-repo (GitGitGadget-enabled GitHub repo)"]
GitGitGadget["GitGitGadget"]
mailing-list["mailing-list"]
user -->|"opens PR"| pr-repo
user -->|"opens PR (if GitHub App installed)"| upstream-repo
upstream-repo -->|"GitGitGadget syncs branches to"| pr-repo
pr-repo -->|"slash commands"| GitGitGadget
upstream-repo -->|"slash commands (if App installed)"| GitGitGadget
GitGitGadget -->|"sends patch series"| mailing-list
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 like the pr-repo-*, upstream-repo-*, test-repo* abstract naming.
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.
lib/ci-helper.ts
Outdated
| this.setAccessToken("dscho", core.getInput("dscho-git-access-token")); | ||
|
|
||
| // eslint-disable-next-line security/detect-non-literal-fs-filename | ||
| if (!fs.existsSync(this.workDir)) await git(["init", "--bare", "--initial-branch", "main", this.workDir]); |
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.
The branch name should probably be in the config or obtained from the remote (ugh). I mention this without knowing if it will be a factor later. Just remember there was a LOT of discussion about main.
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.
Oh, I should have mentioned that the --initial-branch main parameter is there just to suppress the ugly warning. It serves no other purpose because upstream Git does not even update its own main branch anymore (it has been deleted in the meantime, even, showing just how the Git maintainer cared about said LOT of discussion).
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.
Maybe changing that branch name from main to unused would clear things up?
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.
Done.
lib/ci-helper.ts
Outdated
| public async setupGitHubAction(): Promise<void> { | ||
| // help dugite realize where `git` is... | ||
| process.env.LOCAL_GIT_DIRECTORY = "/usr/"; | ||
| process.env.GIT_EXEC_PATH = "/usr/lib/git-core"; |
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.
This path is Debian specific, since they use lib rather than libexec here. We should at least add a comment here, less we hit issues in the future with moving to different distros or if Debian changes their minds again.
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.
Hmm. I added that mostly because in my VS Code-based testing, GIT_EXEC_PATH was set and referred to /usr/local/lib/git-core/ (because I installed microsoft/git there)...
I guess I'll just remove this statement and see whether things still work on the runners.
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.
Ah no, I was wrong... Dugite forcefully sets the GIT_EXEC_PATH, and it uses an incorrect one for Ubuntu's git: https://github.com/desktop/dugite/blob/v2.7.1/lib/git-environment.ts#L44-L64. So yes, we do need to set the exec path.
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.
lib/ci-helper.ts
Outdated
| process.env.GIT_EXEC_PATH = "/usr/lib/git-core"; | ||
|
|
||
| // get the access tokens via the inputs of the GitHub Action | ||
| this.setAccessToken("gitgitgadget", core.getInput("gitgitgadget-git-access-token")); |
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 like the pr-repo-*, upstream-repo-*, test-repo* abstract naming.
lib/ci-helper.ts
Outdated
| const prCommentUrl = core.getInput("pr-comment-url"); | ||
| const [, owner, repo, prNumber, commentId] = | ||
| prCommentUrl.match(/^https:\/\/github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)#issuecomment-(\d+)$/) || []; | ||
| if (!this.config.repo.owners.includes(owner) || repo !== "git") { |
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.
Should repo be checked against this.config.repo.name? This seems git specific.
| description: 'Handles slash commands such as /submit and /preview' | ||
| author: 'Johannes Schindelin' | ||
| inputs: | ||
| pr-repo-token: |
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.
The name and desc are unclear. This is really the state tracking repo isn't it? The pr can be on multiple (well at least two) forks. Just trying to see the outside user viewpoint. If a model project is going to be provided (that was the direction I wanted), the desc can be clearer there.
| smtp-pass: | ||
| description: 'The SMTP password to use for sending emails' | ||
| required: true | ||
| pr-comment-url: |
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.
Where does this come from? comment.id and other context variables have the data this contains.
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.
The context would contain comment.id and other relevant metadata iff the workflow was triggered by issue_comment in pr-repo (or in upstream-repo or in test-repo). But it doesn't even run in the same GitHub org...
So why not provide the metadata split up, as comment-id, owner, repo? For ease of use: GitHub occasionally misses sending webhook events, or cannot reach the Azure Function, or something else goes awry. In such a case, someone with write permissions to gitgitgadget-workflows can conveniently trigger the run by simply copying the link to the respective PR comment and pasting it when triggering the workflow manually.
| name: 'Handle PR Comment' | ||
| description: 'Handles slash commands such as /submit and /preview' | ||
| author: 'Johannes Schindelin' | ||
| inputs: |
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.
re: support for other projects
Probably need to add an optional config input. Maybe also a static method in CIHelper to get a user specified input var.
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.
Right, but I'd probably make that more automagic by teaching CIHelper's constructor to call core.getInput("config"), and if it is set (and not empty!), parse it as JSON and use it instead of the default config.
Then, I'd add those as optional Action inputs, and then I'd set the config as a repository variable in the gitgitgadget-workflows repository and then modify the workflow definitions to pass that variable through.
Having said all that, I'd want to do that in a separate set of PRs, after making those modifications and testing them out on Cygwin contributions. In other words: later.
| async function run() { | ||
| const { CIHelper } = await import("../dist/index.js") | ||
|
|
||
| const ci = new CIHelper() |
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.
re: support for other projects
Need to call the previously mentioned CIHelper function to get an input var for the config prior to this.
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.
As mentioned above, I'd put that into the constructor of CIHelper, to heed the DRY principle insofar possible.
| test-repo-token: | ||
| description: 'The access token to work on dscho/git' | ||
| required: true | ||
| pr-url: |
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.
Why is this a different name from the previous action? (still leaves the question of why other context information is not used for this)
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.
For PR pushes, we don't have a PR issue, and therefore no PR issue URL, that's why 🤭
And we don't have the PR issue webhook event context to work with here, as GitHub Actions' architecture does not allow for cross-repository triggers: The workflow runs in gitgitgadget-workflows, not in, say, gitgitgadget/git.
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.
Could be the same name that both parsePRCommentURLInput and parsePRURLInput use for getInput. The contents are contextual based on the workflow. Just a different point of view.
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.
They could, but I think it is much clearer if the input keys are as different as the exact regular expressions their values must match, i.e. I want to keep pr-url and pr-comment-url as-are.
lib/ci-helper.ts
Outdated
| await unshallow(this.workDir); | ||
|
|
||
| if (setupOptions?.needsMailingListMirror) { | ||
| this.mailingListMirror = "git-mailing-list-mirror.git"; |
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.
Should this be a config value to support other projects with a similar setup?
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!
This will be used to migrate GitGitGadget a set of Azure Pipelines to a set of GitHub Actions and workflows. Signed-off-by: Johannes Schindelin <[email protected]>
In the upcoming GitHub Actions, we must assume that there is no persistent Git worktree to use, unlike the situation of GitGitGadget's Azure Pipelines, which use a single Git worktree that is retained and reused across Pipeline runs. To make this somewhat efficient, let's try to use a partial (semi-shallow) clone. Let's play nice and reuse an already-existing Git worktree (if any), to make local debugging easier. The strategy chosen here is to initialize a partial, blob-less clone, then initialize the `gitgitgadget` Git notes by shallowly fetching the ref with depth 1 (i.e. the tip commit) including all blobs (using `--filter=blob:limit=1g` instead of `--no-filter` to avoid running into bugs introduced in Git v2.48.0-rc0~58^2 (index-pack: repack local links into promisor packs, 2024-11-01) where it falls over its own feet when promisor and non-promisor packs are mixed), and then unshallows the ref with a tree-less fetch. There are a couple of other things we want to set up in the upcoming GitHub Actions, e.g. handling the installation access tokens specified as GitHub Action inputs; Let's combine all of that setup within a single method of the `CIHelper` class. Signed-off-by: Johannes Schindelin <[email protected]>
The upcoming `handle-pr-comment` Action wants to send emails, and for that reason needs to accept SMTP credentials. These can be provided via the `smtp-host`, `smtp-user` and `smtp-pass` Action inputs. Additional options (that might be necessary to support projects other than Git) can be passed in via the `smtp-opts` input, a JSON-formatted object to augment/oeverride nodemailer's `SMTPTransport.Options` when sending the emails. Since most of GitGitGadget's Actions do not want to send emails, it is silently ignored if these Action inputs are unset or empty when `setupGitHubAction()` is called. Signed-off-by: Johannes Schindelin <[email protected]>
GitGitGadget will obtain this information via `git var` whenever sending mails, and also when tagging the patch series' iterations. 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, ready to be consumed in GitHub workflows.
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/`) defining a GitHub Action
via an `action.yml` file and a `index.js` file 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]>
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 `dist/index.js` file, but no matter what I tried, I could not make this work. For that reason, the `res/` folder will need to be copied into `dist/` before publishing as a GitHub Action. Signed-off-by: Johannes Schindelin <[email protected]>
This will be used in the upcoming `handle-pr-comment` GitHub Action. Note that the reason why this information needs to be passed to the respective GitHub Action is that the corresponding GitHub workflows will not run inside `gitgitgadget/git` and will therefore not automatically receive the comment ID and PR number as part of the `github` context. GitGitGadget's GitHub App will need to pass that information along when receiving the webhook event that a PR comment has been created or modified. 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 these trusty
Azure Pipelines we used for so long:
- GitGitGadget PR Handler:
https://dev.azure.com/gitgitgadget/git/_build?definitionId=3
- GitGitGadget PR Handler (git):
https://dev.azure.com/gitgitgadget/git/_build?definitionId=13
- GitGitGadget PR Handler (dscho):
https://dev.azure.com/gitgitgadget/git/_build?definitionId=12
Note that this Action receives the full `pr-comment-url` as Action
input; Readers might wonder why the Action cannot receive the
information in a more fine-grained manner, seeing as the `github`
context of a GitHub workflow triggered by a new PR comment already has
all the necessary values. But this would require adding a
GitGitGadget-specific workflow to the main `git/git` repository,
something that was considered in
#861 but which would
be _extremely_ difficult to convince the core Git contributors to
accept. Therefore, the GitHub workflow using this Action will be a
`workflow_dispatch`-triggered one, for more details see
gitgitgadget-workflows/gitgitgadget-workflows#1.
Signed-off-by: Johannes Schindelin <[email protected]>
This will be used in the upcoming `handle-pr-push` GitHub Action. Just like the `handle-pr-comment` Action, this Action will be used in a GitHub workflow that runs in a different repository than the one where the PR lives that is to be processed, therefore the information cannot be passed in via the `github` context, and combined with ease of debugging, I settled on the full PR URL as the unit of information that the Action input should accept. Signed-off-by: Johannes Schindelin <[email protected]>
This GitHub Action is designed to implement the functionality that is currently handled when pushing to PRs by these Azure Pipelines: - GitGitGadget PR Handler: https://dev.azure.com/gitgitgadget/git/_build?definitionId=3 - GitGitGadget PR Handler (git): https://dev.azure.com/gitgitgadget/git/_build?definitionId=13 - GitGitGadget PR Handler (dscho): https://dev.azure.com/gitgitgadget/git/_build?definitionId=12 This Action will be used in a GitHub workflow that will replace those trusty Azure Pipelines we used for so long. This workflow lives in a different org, for full details see gitgitgadget-workflows/gitgitgadget-workflows#1. Signed-off-by: Johannes Schindelin <[email protected]>
|
@webstech I force-pushed quite a few updates in response to your feedback (thank you!): range-diff
|
lib/ci-helper.ts
Outdated
| "fetch", | ||
| "mirror", | ||
| "--depth=50", | ||
| `+refs/heads/lore-${epoch}:refs/heads/lore-${epoch}`, |
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.
Should lore be in the config? ie could someone follow a similar pattern for a mailing list but not quite follow the lore.kernel.org pattern?
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.
Oh, you're right! I just called git ls-remote https://inbox.sourceware.org/cygwin-patches/0 and it turns out that its HEAD ref is called master...
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.
Actually, it would appear that I was playing some games here, seeing as there are already two epochs of the Git mailing list repository, where the "0th" epoch had a different tree layout (a wide one, which contained all emails in the tip tree, as opposed to the current layout, where the tip tree contains the most recent email as the HEAD:m file). To be able to fetch both into the same mirror repository, I fetched the "0th epoch" into lore-0 and the "1st epoch" into lore-1.
So it's probably safe to assume that whatever public-inbox-based mailing list mirror is used will have to potentially deal with different epochs, too, and come up with a similar naming scheme as I did.
But yes, it's unlikely to be the naming scheme refs/heads/lore-${epoch} that I chose ;-) Therefore, I should totally move the actual ref name into the config, similar to the mirror URL of the Git repository.
For the record, I do believe that we must work with a mailing list mirror repository from which one can make a partial clone; Trying to clone with --filter from, say, lore.kernel.org will result in this warning: "warning: filtering not recognized by server, ignoring".
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.
Will push an update in a moment to this specific end.
| console.time("get open PR head commits"); | ||
| const openPRCommits = ( | ||
| await Promise.all( | ||
| this.config.repo.owners.map(async (repositoryOwner) => { |
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.
Noting this will pull from dscho repo when it is not needed in normal runs.
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.
It's determining which PRs are open in dscho/git, which is currently 0...
|
I have finished reviewing. |
A couple of GitGitGadget's common operations require a local clone of the Git mailing list mirror. For obvious reasons, the idea here is to initialize a partial, shallow clone, so that missing objects (if any) will be retrieved on demand, and no time is wasted on a full clone every time GitGitGadget wants to look at mails. The not-so-obvious idea is to un-shallow the clone using the same strategy (and in fact, the same function) as for the Git "worktree": When I tried to simply delete the `shallow` file during the development of this patch, assuming that Git's partial clone functionality would gracefully fetch any missing commit e.g. when computing a merge base, reality taught me the harsh lesson that it would fail hard in such scenarios. Instead, the `git fetch --unshallow` command is called with `--filter=tree:0` so that _only_ the missing commits (and all of them) are fetched. This still seems to strike a good compromise between saving time on the clone and saving on network round-trips when fetching missing Git objects. Signed-off-by: Johannes Schindelin <[email protected]>
The idea is to take over the job of the "Mirror Git List to GitGitGadget's PRs" Azure Pipeline over at https://dev.azure.com/gitgitgadget/git/_build?definitionId=5. This will obviously require a GitHub workflow, too, which is added in gitgitgadget-workflows/gitgitgadget-workflows#1 Signed-off-by: Johannes Schindelin <[email protected]>
GitGitGadget's code has auto-magic logic to fetch those, but even in a partial clone that would result in a ~100MB fetch. Maybe that's okay, and I worry too much about bandwidth? Inside hosted Actions runners, download speeds of ~30MB/sec are not unheard of, after all... But then, I do think that too few people are careful to waste less resources than is ethical, so I want to be mindful. Note that we use a "funny" `--filter=blob:limit=1g` option here instead of the more natural `--no-filter`: Git introduced a subtle bug (that I have no time nor patience to pursue any further) in v2.48.0-rc0~58^2 (index-pack: repack local links into promisor packs, 2024-11-01) that would occasionally result in this strange error when fetching in the partial clone after an occasional `--no-filter` fetch: git -C git.git fetch upstream remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Compressing objects: 100% (2/2), done. remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0) Receiving objects: 100% (2/2), 14.66 KiB | 7.33 MiB/s, done. BUG: builtin/pack-objects.c:4320: should_include_obj should only be called on existing objects error: pack-objects died of signal 6 fatal: could not finish pack-objects to repack local links fatal: index-pack failed The most likely reason is that the logic needs to improved to accommodate for situations where commits are fetched into a non-promisor pack as part of fetch that notices that some of the commits reachable from that pack are present locally (but they are in promisor packs, which the naïve `pack-objects` call fails to anticipate). But again, I do not have time to pursue this any further. Keeping out non-promisor packs avoids the bug, and that's that. Signed-off-by: Johannes Schindelin <[email protected]>
The idea is to take over the job of the "Update GitGitGadget's PRs" Azure Pipeline over at https://dev.azure.com/gitgitgadget/git/_build?definitionId=2. This will obviously require a GitHub workflow, which is added in gitgitgadget-workflows/gitgitgadget-workflows#1 Signed-off-by: Johannes Schindelin <[email protected]>
…otes This ports the Azure Pipeline "Update GitGitGadget's commit to mail notes" (https://dev.azure.com/gitgitgadget/git/_build?definitionId=9) logic to a method of the `CIHelper` class that will be used in a GitHub Action so that the Azure Pipeline can be retired. This was the only Azure Pipeline of GitGitGadget which did not, actually, use the `misc-helper.ts` script, but instead was opaquely implemented as a lengthy shell scriptlet inside the Pipeline definition that called two of the shell scripts in the `gitgitgadget/gitgitgadget` repository: `lookup-commit.sh` and `update-mail-to-commit-notes.sh`. Since the scripts that are called by the new method expect a persisted, non-partial clone of the Git mailing list, we need to play a couple of games here to make it work in a GitHub workflow (that runs on ephemeral runners where the repository has to be cloned afresh in every run). Also: These shell scripts, by virtue of needing to be interpreted by a shell interpreter outside of node.js, need to be copied into the `dist/script/` subdirectory before publishing the GitHub Action, so that they can be found and interpreted as expected. Signed-off-by: Johannes Schindelin <[email protected]>
The idea is to take over the job of the "Update GitGitGadget's commit to mail notes" Azure Pipeline over at https://dev.azure.com/gitgitgadget/git/_build?definitionId=9, making use of the just-ported logic in `CIHelper`. This will obviously require a GitHub workflow, too, which is added in gitgitgadget-workflows/gitgitgadget-workflows#1 Signed-off-by: Johannes Schindelin <[email protected]>
range-diff
|
| "fetch", | ||
| "mirror", | ||
| "--depth=50", | ||
| "+REF:REF".replace("REF", this.config.mailrepo.mirrorRef || `refs/heads/lore-${epoch}`), |
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.
Does mirrorRef need to include refs/heads/? I suppose you could put things pretty much anywhere you want in a repo.
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.
It does not have to, but it avoids ambiguity.
Publish the GitHub Actions to a separate branch This PR is part 4 (the final part in this repository) of addressing #609, and it is stacked on top of #1980, #1981, and #1982 (and therefore contains also the commits of those PRs), therefore I will leave this in draft mode until those PRs are merged. After laying the groundwork for, and implementing, the set of GitHub Actions that can perform the same job as GitGitGadget's current Azure Pipelines can perform, this here PR adds automation to 1. transpile the `CIHelper` class (together with its dependencies) from Typescript to JavaScript, 2. bundle it as a single, dependency-less `dist/index.js`, 3. copy the required resources (`WELCOME.md`, some shell scripts) into the location expected by that `dist/index.js`, 4. remove all the rest except for the minimal GitHub Actions (`*/action.yml`, `*/index.js`), 5. commit the result as the new tip commit of the `v1` branch (creating it as needed), 6. tag that tip commit as `v1.<running-number>`, 7. push out the `v1` branch and the tag. The result of this is that GitGitGadget can still be developed conveniently in this here repository, and whenever anything gets merged to the `main` branch, the `v1` branch is automatically updated so that it will be picked up by GitHub workflows containing statements like: ```yaml - uses: gitgitgadget/gitgitgadget/handle-pr-comment@v1 ``` That way, we _finally_ address the fragile nature of the current setup where a set of Azure Pipelines maintain their own clone of `gitgitgadget/gitgitgadget`, and having to run `npm ci && npm run build` as needed. This closes #1759
This PR is part 3 of addressing #609, and it is stacked on top of #1980 and #1981 (and therefore contains also the commits of those PRs), therefore I will leave this in draft mode until those PRs are merged.
The grand idea is to bundle the
CIHelperclass together with all its direct and transitive dependencies into one big, honkingdist/index.js, and then add a set of really minimal GitHub Actions that call intoCIHelper. The Actions are added in sub-directories so that they can be called in GitHub workflows via e.g.- uses: gitgitgadget/gitgitgadget/update-prs@1.The component used for bundling
CIHelperis@vercel/ncc. To support acting as a GitHub Action,@actions/coreis installed.To allow for really minimal GitHub Actions, the
CIHelperclass is augmented accordingly to re-implement more logic that is currently either inmisc-helper.tsor in the (non-public 😞) Azure Pipelines definitions.The naming convention for specifying the necessary tokens as GitHub Actions inputs is:
upstream-repo-token: This is to comment on PRs ingit/gitpr-repo-token: This is to comment on PRs ingitgitgadget/git(as well as to be able to push to that repository)test-repo-token: This is to comment on PRs indscho/git(used exclusively for testing)To clarify, here is a diagram:
graph TD user["user (contributor)"] upstream-repo["upstream-repo (authoritative project repo)"] pr-repo["pr-repo (GitGitGadget-enabled GitHub repo)"] GitGitGadget["GitGitGadget"] mailing-list["mailing-list"] user -->|"opens PR"| pr-repo user -->|"opens PR (if GitHub App installed)"| upstream-repo upstream-repo -->|"GitGitGadget syncs branches to"| pr-repo pr-repo -->|"slash commands"| GitGitGadget upstream-repo -->|"slash commands (if App installed)"| GitGitGadget GitGitGadget -->|"sends patch series"| mailing-list