diff mbox series

GitHub ci(windows): speed up initializing Git for Windows' minimal SDK again

Message ID pull.1841.git.1734447458896.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 55d62306eeada186154fd538cb79efd579f7d9f6
Headers show
Series GitHub ci(windows): speed up initializing Git for Windows' minimal SDK again | expand

Commit Message

Johannes Schindelin Dec. 17, 2024, 2:57 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

It used to be the case that initializing the minimal SDK (i.e. a
radically slimmed-down subset of Git for Windows' development
environment intended to perform the CI builds and little else) took
a bit over one minute, would then be cached, and subsequent jobs would
take at most half a dozen seconds to initialize said minimal SDK.

It is important that this step is fast because we have to run the test
suite in parallel, in a set of matrix jobs, to offset the slowness of
the shell-based test suite, and each and every job has to initialize the
very same minimal SDK.

While it may sound as if parallelizing the jobs might only waste the
generously-provided build minutes but at least the _wallclock_ time
would pass quick, in reality it matters a lot: Frequently Git for
Windows' or GitGitGadget PRs get stuck waiting for quite a while before
CI builds start because other PRs' builds still spend substantial
amounts of time to run, blocking due to the concurrency limit being
reached.

Since 91839a88277 (ci: create script to set up Git for Windows SDK,
2024-10-09), the situation has worsened: every job that requires the
minimal Git for Windows SDK spends roughly two-and-a-half minutes doing
so.

With the switch away from the GitHub Action `setup-git-for-windows-sdk`,
we incurred more downsides:

- It is no longer possible for said Action to fix problems independently
  from the Git repository, e.g. when new rules about GitHub Actions
  require changes in the way the minimal SDK is initialized.

- The minimal SDK was installed specifically outside of the worktree so
  as not to clutter it nor incur an additional cost to verify that the
  worktree is clean.

Therefore, even if it would be nice to have a shared process between
GitHub and GitLab based CI builds, let's switch the GitHub-based CI back
to the tried-and-tested `setup-git-for-windows-sdk` Action.

This commit partially reverts 91839a88277 (ci: create script to set up
Git for Windows SDK, 2024-10-09).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    Speed up the Git for Windows SDK initialization again
    
    While waiting for way too many builds in
    https://github.com/gitgitgadget/git/actions to finish, I noticed that
    the minimal Git for Windows SDK initialization now takes a whopping 2.5
    minutes. That's way too much. It used to take a little over a minute
    when uncached, and 2-5 seconds when cached.
    
    Let's fix this regression by reverting to using the
    setup-git-for-windows-sdk GitHub Action (also because that Action will
    soon see another dramatic speed-up, see
    https://github.com/git-for-windows/setup-git-for-windows-sdk/pull/965).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1841%2Fdscho%2Fci-windows-jobs-speedup-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1841/dscho/ci-windows-jobs-speedup-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1841

 .github/workflows/main.yml | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)


base-commit: 631ddbbcbd912530e1b78e5d782e72879f7f1fb2

Comments

Junio C Hamano Dec. 17, 2024, 8:33 p.m. UTC | #1 
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> This commit partially reverts 91839a88277 (ci: create script to set up
> Git for Windows SDK, 2024-10-09).

Thanks, will queue.

> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 9301a1edd6d..916a64b6736 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -113,15 +113,13 @@ jobs:
>        cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
>      steps:
>      - uses: actions/checkout@v4
> -    - name: setup SDK
> -      shell: powershell
> -      run: ci/install-sdk.ps1
> +    - uses: git-for-windows/setup-git-for-windows-sdk@v1
>      - name: build
> -      shell: powershell
> +      shell: bash
>        env:
>          HOME: ${{runner.workspace}}
>          NO_PERL: 1
> -      run: git-sdk/usr/bin/bash.exe -l -c 'ci/make-test-artifacts.sh artifacts'
> +      run: . /etc/profile && ci/make-test-artifacts.sh artifacts
>      - name: zip up tracked files
>        run: git archive -o artifacts/tracked.tar.gz HEAD
>      - name: upload tracked files and build artifacts
> @@ -149,12 +147,10 @@ jobs:
>      - name: extract tracked files and build artifacts
>        shell: bash
>        run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
> -    - name: setup SDK
> -      shell: powershell
> -      run: ci/install-sdk.ps1
> +    - uses: git-for-windows/setup-git-for-windows-sdk@v1
>      - name: test
> -      shell: powershell
> -      run: git-sdk/usr/bin/bash.exe -l -c 'ci/run-test-slice.sh ${{matrix.nr}} 10'
> +      shell: bash
> +      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10
>      - name: print test failures
>        if: failure() && env.FAILED_TEST_ARTIFACTS != ''
>        shell: bash
>
> base-commit: 631ddbbcbd912530e1b78e5d782e72879f7f1fb2
Patrick Steinhardt Dec. 18, 2024, 5:56 a.m. UTC | #2 
On Tue, Dec 17, 2024 at 12:33:10PM -0800, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > This commit partially reverts 91839a88277 (ci: create script to set up
> > Git for Windows SDK, 2024-10-09).
> 
> Thanks, will queue.

Okay. Too bad that things regressed this badly with the change. I agree
that reverting is the right thing to do for now. I may revisit this
again in the next release cycle to investigate whether we can get it up
to par with the GitHub Actions. It would be great if the build infra was
shared between our CIs, so I think there's some value in it. But if the
answer is "no" then I guess that's ultimately fine, as well.

Thanks!

Patrick
diff mbox series

Patch

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 9301a1edd6d..916a64b6736 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -113,15 +113,13 @@  jobs:
       cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
     steps:
     - uses: actions/checkout@v4
-    - name: setup SDK
-      shell: powershell
-      run: ci/install-sdk.ps1
+    - uses: git-for-windows/setup-git-for-windows-sdk@v1
     - name: build
-      shell: powershell
+      shell: bash
       env:
         HOME: ${{runner.workspace}}
         NO_PERL: 1
-      run: git-sdk/usr/bin/bash.exe -l -c 'ci/make-test-artifacts.sh artifacts'
+      run: . /etc/profile && ci/make-test-artifacts.sh artifacts
     - name: zip up tracked files
       run: git archive -o artifacts/tracked.tar.gz HEAD
     - name: upload tracked files and build artifacts
@@ -149,12 +147,10 @@  jobs:
     - name: extract tracked files and build artifacts
       shell: bash
       run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
-    - name: setup SDK
-      shell: powershell
-      run: ci/install-sdk.ps1
+    - uses: git-for-windows/setup-git-for-windows-sdk@v1
     - name: test
-      shell: powershell
-      run: git-sdk/usr/bin/bash.exe -l -c 'ci/run-test-slice.sh ${{matrix.nr}} 10'
+      shell: bash
+      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       shell: bash