Re: [PATCH 8/8] ci: modification of main.yml to use cmake for vs-build job

[Sibi Siddharthan <sibisiddharthan.github@gmail.com>]

On Fri, Apr 24, 2020 at 11:15 PM Danh Doan <congdanhqx@gmail.com> wrote:
>
> On 2020-04-24 04:01:37+0000, Sibi Siddharthan via GitGitGadget <gitgitgadget@gmail.com> wrote:
> > From: Sibi Siddharthan <sibisiddharthan.github@gmail.com>
> >
> > This patch modifies .github/workflows/main.yml to use CMake for
> > Visual Studio builds.
> >
> > Modified the vs-test step to match windows-test step. This speeds
> > up the vs-test. Calling git-cmd from powershell and then calling git-bash
> > to perform the tests slows things down(factor of about 6). So git-bash
> > is directly called from powershell to perform the tests using prove.
> >
> > NOTE: Since GitHub keeps the same directory for each job
> > (with respect to path) absolute paths are used in the bin-wrapper
> > scripts.
> >
> > GitHub has switched to CMake 3.17.1 which changed the behaviour of
> > FindCURL module. An extra definition (-DCURL_NO_CURL_CMAKE=ON) has been
> > added to revert to the old behaviour.
> >
> > Signed-off-by: Sibi Siddharthan <sibisiddharthan.github@gmail.com>
> > ---
> >  .github/workflows/main.yml | 43 ++++++++++++++++++++++----------------
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> >
> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > index fd4df939b50..94f9a385225 100644
> > --- a/.github/workflows/main.yml
> > +++ b/.github/workflows/main.yml
> > @@ -80,13 +80,6 @@ jobs:
> >      - name: download git-sdk-64-minimal
> >        shell: bash
> >        run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf -
> > -    - name: generate Visual Studio solution
> > -      shell: powershell
> > -      run: |
> > -        & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
> > -          make NDEBUG=1 DEVELOPER=1 vcxproj
> > -        "@
> > -        if (!$?) { exit(1) }
> >      - name: download vcpkg artifacts
> >        shell: powershell
> >        run: |
> > @@ -98,6 +91,13 @@ jobs:
> >          Remove-Item compat.zip
> >      - name: add msbuild to PATH
> >        uses: microsoft/setup-msbuild@v1.0.0
> > +    - name: generate Visual Studio solution
> > +      shell: powershell
> > +      run: |
> > +        & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
> > +          cmake . -DCMAKE_PREFIX_PATH=./compat/vcbuild/vcpkg/installed/x64-windows -DMSGFMT_EXE=./git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> > +        "@
> > +        if (!$?) { exit(1) }
>
> If you intended to modified some steps to provide a better script for
> that step, I believe you should change in that step.
>
> If the order of those steps need to be changed, please clarify your
> reasoning!
>

Previously, the Visual Studio solution was generated, then the vcpkg
artifacts were downloaded.
With CMake, as it searched for the libraries during configure the
vcpkg artifacts are now downloaded before the configure step.

> >      - name: MSBuild
> >        run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> >      - name: bundle artifact tar
> > @@ -125,9 +125,9 @@ jobs:
> >          nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
> >      steps:
> >      - uses: actions/checkout@v1
> > -    - name: download git-64-portable
> > +    - name: download git-sdk-64-minimal
> >        shell: bash
> > -      run: a=git-64-portable && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf -
> > +      run: a=git-sdk-64-minimal && mkdir -p $a && curl -# https://wingit.blob.core.windows.net/ci-artifacts/$a.tar.xz | tar -C $a -xJf -
> >      - name: download build artifacts
> >        uses: actions/download-artifact@v1
> >        with:
> > @@ -136,23 +136,30 @@ jobs:
> >      - name: extract build artifacts
> >        shell: bash
> >        run: tar xf artifacts.tar.gz
> > -    - name: test (parallel)
> > +    - name: test
>
> So the test couldn't be run in parallel anymore?
> It's a regression!
> And we don't need the matrix anymore!!!!!
>
> I wonder if Dscho's effort is wasted, he tried very hard to make
> those tests run in parallel.
> He even tried to re-order the matrix in GfW project to let longest
> test run first!
>
>

Okay, When I removed "(parallel)" does not mean that the tests are not
run in parallel.
Now they are run in the same way how windows-test(step) is run.
The tests(vs-test) actually completes before than the windows-test in the CI

> >        shell: powershell
> >        env:
> >          MSYSTEM: MINGW64
> >          NO_SVN_TESTS: 1
> >          GIT_TEST_SKIP_REBASE_P: 1
> >        run: |
> > -        & git-64-portable\git-cmd.exe --command=usr\bin\bash.exe -lc @"
> > -          # Let Git ignore the SDK and the test-cache
> > -          printf '%s\n' /git-64-portable/ /test-cache/ >>.git/info/exclude
> > +        & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
> > +          # Let Git ignore the SDK
> > +          printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
> >
> > -          cd t &&
> > -          PATH=\"`$PWD/helper:`$PATH\" &&
> > -          test-tool.exe run-command testsuite --jobs=10 -V -x --write-junit-xml \
> > -                  `$(test-tool.exe path-utils slice-tests \
> > -                          ${{matrix.nr}} 10 t[0-9]*.sh)
> > +          ci/run-test-slice.sh ${{matrix.nr}} 10
> >          "@
> > +    - name: ci/print-test-failures.sh
> > +      if: failure()
> > +      shell: powershell
> > +      run: |
> > +        & .\git-sdk-64-minimal\usr\bin\bash.exe -lc ci/print-test-failures.sh
>
> What is changed?
>
> --
> Danh

Thank You,
Sibi Siddharthan