* [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow
@ 2021-06-23 15:24 Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 1/6] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
` (7 more replies)
0 siblings, 8 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
This patch series upgrades to newer versions of a couple GitHub Actions we
use, and also streamlines the Windows jobs using the relatively new
setup-git-for-windows-sdk Action
[https://github.com/marketplace/actions/setup-git-for-windows-sdk] (Git for
Windows is running with this Action for a while now, getting all the kinks
out).
This patch series should also address the problem where seen was pushed so
rapidly that the windows-test jobs failed because they no longer checked out
the identical revision as the windows-build job.
Dennis Ameling (1):
ci(vs-build): build with NO_GETTEXT
Johannes Schindelin (5):
ci: use the new GitHub Action to download git-sdk-64-minimal
ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
ci: upgrade to using actions/{up,down}load-artifacts v2
ci(windows): transfer also the Git-tracked files to the test jobs
ci: accelerate the checkout
.github/workflows/main.yml | 157 +++++++++++--------------------------
1 file changed, 46 insertions(+), 111 deletions(-)
base-commit: 670b81a890388c60b7032a4f5b879f2ece8c4558
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-878%2Fdscho%2Fuse-setup-git-for-windows-sdk-action-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-878/dscho/use-setup-git-for-windows-sdk-action-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/878
--
gitgitgadget
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 1/6] ci: use the new GitHub Action to download git-sdk-64-minimal
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
@ 2021-06-23 15:24 ` Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 2/6] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In our continuous builds, Windows is the odd cookie that requires a
complete development environment to be downloaded because there is no
suitable one installed by default on Windows.
Side note: technically, there _is_ a development environment present in
GitHub Actions' build agents: MSYS2. But it differs from Git for
Windows' SDK in subtle points, unfortunately enough so to prevent Git's
test suite from running without failures.
Traditionally, we support downloading this environment (which we
nicknamed `git-sdk-64-minimal`) via a PowerShell scriptlet that accesses
the build artifacts of a dedicated Azure Pipeline (which packages a tiny
subset of the full Git for Windows SDK, containing just enough to build
Git and run its test suite).
This PowerShell script is unfortunately not very robust and sometimes
fails due to network issues.
Of course, we could add code to detect that situation, wait a little,
try again, if it fails again wait a little longer, lather, rinse and
repeat.
Instead of doing all of this in Git's own `.github/workflows/`, though,
let's offload this logic to the new GitHub Action at
https://github.com/marketplace/actions/setup-git-for-windows-sdk
This Action not only downloads and extracts git-sdk-64-minimal _outside_
the worktree (making it no longer necessary to meddle with
`.gitignore` or `.git/info/exclude`), it also adds the `bash.exe` to the
`PATH` and sets the environment variable `MSYSTEM` (an implementation
detail that Git's workflow should never have needed to know about).
This allows us to convert all those funny PowerShell tasks that wanted
to call git-sdk-64-minimal's `bash.exe`: they all are now regular `bash`
scriptlets.
This finally lets us get rid of the funny quoting and escaping where we
had to pay attention not only to quote and escape the Bash scriptlets
properly, but also to add a second level of escaping (with backslashes
for double quotes and backticks for dollar signs) to stop PowerShell
from doing unintended things.
Further, this Action uses a fast caching strategy native to GitHub
Actions that should accelerate the download across CI runs:
git-sdk-64-minimal is usually updated once per 24h, and needs to be
cached only once within that period. Caching it (unfortunately only on
a per-branch basis) speeds up the download step, and makes it much more
robust at the same time by virtue of accessing a cache location that is
closer in the network topology.
With this we can drop the home-rolled caching where we try to accelerate
the test phase by uploading git-sdk-64-minimal as a workflow artifact
after using it to build Git, and then download it as workflow artifact
in the test phase.
Even better: the `vs-test` job no longer needs to depend on the
`windows-build` job. The only reason it depended on it was to ensure
that the `git-sdk-64-minimal` workflow artifact was available.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 103 +++++++------------------------------
1 file changed, 19 insertions(+), 84 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 73856bafc9..c62766e7b1 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -82,43 +82,18 @@ jobs:
runs-on: windows-latest
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- shell: bash
- run: |
- ## Get artifact
- urlbase=https://dev.azure.com/git-for-windows/git/_apis/build/builds
- id=$(curl "$urlbase?definitions=22&statusFilter=completed&resultFilter=succeeded&\$top=1" |
- jq -r ".value[] | .id")
- download_url="$(curl "$urlbase/$id/artifacts" |
- jq -r '.value[] | select(.name == "git-sdk-64-minimal").resource.downloadUrl')"
- curl --connect-timeout 10 --retry 5 --retry-delay 0 --retry-max-time 240 \
- -o artifacts.zip "$download_url"
-
- ## Unzip and remove the artifact
- unzip artifacts.zip
- rm artifacts.zip
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
- shell: powershell
+ shell: bash
env:
HOME: ${{runner.workspace}}
- MSYSTEM: MINGW64
NO_PERL: 1
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
-
- ci/make-test-artifacts.sh artifacts
- "@
+ run: ci/make-test-artifacts.sh artifacts
- name: upload build artifacts
uses: actions/upload-artifact@v1
with:
name: windows-artifacts
path: artifacts
- - name: upload git-sdk-64-minimal
- uses: actions/upload-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: git-sdk-64-minimal
windows-test:
runs-on: windows-latest
needs: [windows-build]
@@ -136,25 +111,14 @@ jobs:
- name: extract build artifacts
shell: bash
run: tar xf artifacts.tar.gz
- - name: download git-sdk-64-minimal
- uses: actions/download-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: ${{github.workspace}}/git-sdk-64-minimal/
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: test
- shell: powershell
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- # Let Git ignore the SDK
- printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
-
- ci/run-test-slice.sh ${{matrix.nr}} 10
- "@
+ shell: bash
+ run: 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
+ shell: bash
+ run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v1
@@ -165,27 +129,12 @@ jobs:
needs: ci-config
if: needs.ci-config.outputs.enabled == 'yes'
env:
- MSYSTEM: MINGW64
NO_PERL: 1
GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
runs-on: windows-latest
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- shell: bash
- run: |
- ## Get artifact
- urlbase=https://dev.azure.com/git-for-windows/git/_apis/build/builds
- id=$(curl "$urlbase?definitions=22&statusFilter=completed&resultFilter=succeeded&\$top=1" |
- jq -r ".value[] | .id")
- download_url="$(curl "$urlbase/$id/artifacts" |
- jq -r '.value[] | select(.name == "git-sdk-64-minimal").resource.downloadUrl')"
- curl --connect-timeout 10 --retry 5 --retry-delay 0 --retry-max-time 240 \
- -o artifacts.zip "$download_url"
-
- ## Unzip and remove the artifact
- unzip artifacts.zip
- rm artifacts.zip
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
uses: actions/checkout@v2
with:
@@ -211,19 +160,17 @@ jobs:
shell: bash
run: |
cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
- -DMSGFMT_EXE=`pwd`/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
+ -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
- shell: powershell
+ shell: bash
env:
MSVC: 1
VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
run: |
- & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- mkdir -p artifacts &&
- eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\"
- "@
+ mkdir -p artifacts &&
+ eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- name: upload build artifacts
uses: actions/upload-artifact@v1
with:
@@ -231,18 +178,14 @@ jobs:
path: artifacts
vs-test:
runs-on: windows-latest
- needs: [vs-build, windows-build]
+ needs: vs-build
strategy:
fail-fast: false
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- uses: actions/download-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: ${{github.workspace}}/git-sdk-64-minimal/
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download build artifacts
uses: actions/download-artifact@v1
with:
@@ -252,23 +195,15 @@ jobs:
shell: bash
run: tar xf artifacts.tar.gz
- name: test
- shell: powershell
+ shell: bash
env:
- MSYSTEM: MINGW64
NO_SVN_TESTS: 1
GIT_TEST_SKIP_REBASE_P: 1
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- # Let Git ignore the SDK and the test-cache
- printf '%s\n' /git-sdk-64-minimal/ /test-cache/ >>.git/info/exclude
-
- ci/run-test-slice.sh ${{matrix.nr}} 10
- "@
+ run: 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
+ shell: bash
+ run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 2/6] ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 1/6] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
@ 2021-06-23 15:24 ` Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 3/6] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We use a `.bat` script to copy the DLLs in the `vs-build` job, and those
type of scripts are native to CMD, not to PowerShell.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c62766e7b1..d430c4e0d2 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -152,10 +152,8 @@ jobs:
- name: add msbuild to PATH
uses: microsoft/setup-msbuild@v1
- name: copy dlls to root
- shell: powershell
- run: |
- & compat\vcbuild\vcpkg_copy_dlls.bat release
- if (!$?) { exit(1) }
+ shell: cmd
+ run: compat\vcbuild\vcpkg_copy_dlls.bat release
- name: generate Visual Studio solution
shell: bash
run: |
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 3/6] ci: upgrade to using actions/{up,down}load-artifacts v2
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 1/6] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 2/6] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
@ 2021-06-23 15:24 ` Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 4/6] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The GitHub Actions to upload/download workflow artifacts saw a major
upgrade since Git's GitHub workflow was established. Let's use it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index d430c4e0d2..a399114c0f 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -90,7 +90,7 @@ jobs:
NO_PERL: 1
run: ci/make-test-artifacts.sh artifacts
- name: upload build artifacts
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: windows-artifacts
path: artifacts
@@ -104,7 +104,7 @@ jobs:
steps:
- uses: actions/checkout@v1
- name: download build artifacts
- uses: actions/download-artifact@v1
+ uses: actions/download-artifact@v2
with:
name: windows-artifacts
path: ${{github.workspace}}
@@ -121,7 +121,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-windows
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -170,7 +170,7 @@ jobs:
mkdir -p artifacts &&
eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- name: upload build artifacts
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: vs-artifacts
path: artifacts
@@ -185,7 +185,7 @@ jobs:
- uses: actions/checkout@v1
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download build artifacts
- uses: actions/download-artifact@v1
+ uses: actions/download-artifact@v2
with:
name: vs-artifacts
path: ${{github.workspace}}
@@ -204,7 +204,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-windows
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -242,7 +242,7 @@ jobs:
if: failure()
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -269,7 +269,7 @@ jobs:
if: failure()
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 4/6] ci(windows): transfer also the Git-tracked files to the test jobs
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2021-06-23 15:24 ` [PATCH 3/6] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
@ 2021-06-23 15:24 ` Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 5/6] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
` (3 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Git's test suite is excruciatingly slow on Windows, mainly due to the
fact that it executes a lot of shell script code, and that's simply not
native to Windows.
To help with that, we established the pattern where the artifacts are
first built in one job, and then multiple test jobs run in parallel
using the artifacts built in the first job.
We take pains in transferring only the build outputs, and letting
`actions/checkout` fill in the rest of the files.
One major downside of that strategy is that the test jobs might fail to
check out the intended revision (e.g. because the branch has been
updated while the build was running, as is frequently the case with the
`seen` branch).
Let's transfer also the files tracked by Git, and skip the checkout step
in the test jobs.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a399114c0f..0f7516c9ef 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -89,7 +89,9 @@ jobs:
HOME: ${{runner.workspace}}
NO_PERL: 1
run: ci/make-test-artifacts.sh artifacts
- - name: upload build artifacts
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
uses: actions/upload-artifact@v2
with:
name: windows-artifacts
@@ -102,15 +104,14 @@ jobs:
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- - uses: actions/checkout@v1
- - name: download build artifacts
+ - name: download tracked files and build artifacts
uses: actions/download-artifact@v2
with:
name: windows-artifacts
path: ${{github.workspace}}
- - name: extract build artifacts
+ - name: extract tracked files and build artifacts
shell: bash
- run: tar xf artifacts.tar.gz
+ run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: test
shell: bash
@@ -169,7 +170,9 @@ jobs:
run: |
mkdir -p artifacts &&
eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- - name: upload build artifacts
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
uses: actions/upload-artifact@v2
with:
name: vs-artifacts
@@ -182,16 +185,15 @@ jobs:
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- - uses: actions/checkout@v1
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- - name: download build artifacts
+ - name: download tracked files and build artifacts
uses: actions/download-artifact@v2
with:
name: vs-artifacts
path: ${{github.workspace}}
- - name: extract build artifacts
+ - name: extract tracked files and build artifacts
shell: bash
- run: tar xf artifacts.tar.gz
+ run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- name: test
shell: bash
env:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2021-06-23 15:24 ` [PATCH 4/6] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
@ 2021-06-23 15:24 ` Dennis Ameling via GitGitGadget
2021-07-04 8:27 ` Ævar Arnfjörð Bjarmason
2021-06-23 15:24 ` [PATCH 6/6] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
7 siblings, 1 reply; 41+ messages in thread
From: Dennis Ameling via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Dennis Ameling
From: Dennis Ameling <dennis@dennisameling.com>
We already build Git for Windows with `NO_GETTEXT` when compiling with
GCC. Let's do the same with Visual C, too.
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0f7516c9ef..3b40c677ab 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -159,7 +159,7 @@ jobs:
shell: bash
run: |
cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
- -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
+ -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 6/6] ci: accelerate the checkout
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2021-06-23 15:24 ` [PATCH 5/6] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
@ 2021-06-23 15:24 ` Johannes Schindelin via GitGitGadget
2021-07-01 22:03 ` [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Junio C Hamano
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-06-23 15:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
By upgrading from v1 to v2 of `actions/checkout`, we avoid fetching all
the tags and the complete history: v2 only fetches one revision by
default. This should make things a lot faster.
Note that `actions/checkout@v2` seems to be incompatible with running in
containers: https://github.com/actions/checkout/issues/151. Therefore,
we stick with v1 there.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3b40c677ab..405204c78a 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -81,7 +81,7 @@ jobs:
if: needs.ci-config.outputs.enabled == 'yes'
runs-on: windows-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
shell: bash
@@ -134,7 +134,7 @@ jobs:
GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
runs-on: windows-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
uses: actions/checkout@v2
@@ -237,7 +237,7 @@ jobs:
jobname: ${{matrix.vector.jobname}}
runs-on: ${{matrix.vector.pool}}
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/run-build-and-tests.sh
- run: ci/print-test-failures.sh
@@ -282,7 +282,7 @@ jobs:
jobname: StaticAnalysis
runs-on: ubuntu-18.04
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/run-static-analysis.sh
documentation:
@@ -292,6 +292,6 @@ jobs:
jobname: Documentation
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/test-documentation.sh
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2021-06-23 15:24 ` [PATCH 6/6] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
@ 2021-07-01 22:03 ` Junio C Hamano
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
7 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-07-01 22:03 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> This patch series upgrades to newer versions of a couple GitHub Actions we
> use, and also streamlines the Windows jobs using the relatively new
> setup-git-for-windows-sdk Action
> [https://github.com/marketplace/actions/setup-git-for-windows-sdk] (Git for
> Windows is running with this Action for a while now, getting all the kinks
> out).
>
> This patch series should also address the problem where seen was pushed so
> rapidly that the windows-test jobs failed because they no longer checked out
> the identical revision as the windows-build job.
https://github.com/git/git/actions/runs/991636423 is the run on the
real 'seen' for the day, which fails vs-build. The same 'seen',
with this topic excluded, seems to make everything pass and can be
seen at https://github.com/git/git/actions/runs/991543592
CI/PR(seen) runs in the past few days all include this topic, and all
of them have failed the vs-build job.
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 0/7] ci: speed-up the Windows parts of our GitHub workflow
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2021-07-01 22:03 ` [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Junio C Hamano
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
` (7 more replies)
7 siblings, 8 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
This patch series upgrades to newer versions of a couple GitHub Actions we
use, and also streamlines the Windows jobs using the relatively new
setup-git-for-windows-sdk Action
[https://github.com/marketplace/actions/setup-git-for-windows-sdk] (Git for
Windows is running with this Action for a while now, getting all the kinks
out).
This patch series should also address the problem where seen was pushed so
rapidly that the windows-test jobs failed because they no longer checked out
the identical revision as the windows-build job.
Changes since v1:
* Added a patch to fix make NO_GETTEXT=Yep artifacts-tar (not to include
.mo files), as suggested by Matthias Aßauer in the GitGitGadget PR, which
should fix the CI failure in seen that Junio pointed out. The bug was
unhidden by mr/cmake fixing the CMake build (which ignored NO_GETTEXT
before).
Dennis Ameling (1):
ci(vs-build): build with NO_GETTEXT
Johannes Schindelin (6):
ci: use the new GitHub Action to download git-sdk-64-minimal
ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
ci: upgrade to using actions/{up,down}load-artifacts v2
ci(windows): transfer also the Git-tracked files to the test jobs
artifacts-tar: respect NO_GETTEXT
ci: accelerate the checkout
.github/workflows/main.yml | 157 +++++++++++--------------------------
Makefile | 5 +-
2 files changed, 50 insertions(+), 112 deletions(-)
base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-878%2Fdscho%2Fuse-setup-git-for-windows-sdk-action-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-878/dscho/use-setup-git-for-windows-sdk-action-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/878
Range-diff vs v1:
1: 8da66c425a1 = 1: 2e4db688deb ci: use the new GitHub Action to download git-sdk-64-minimal
2: a913af64f9d = 2: 6b12fe2284c ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
3: 7930465e809 = 3: c256bbf4b1c ci: upgrade to using actions/{up,down}load-artifacts v2
4: 1520a9ffb57 = 4: 59dc44428fb ci(windows): transfer also the Git-tracked files to the test jobs
-: ----------- > 5: c31d2e7f44a artifacts-tar: respect NO_GETTEXT
5: a5084795ab0 = 6: 8bab4c17b8a ci(vs-build): build with NO_GETTEXT
6: e772d8f1d9b = 7: 88a44863cd0 ci: accelerate the checkout
--
gitgitgadget
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In our continuous builds, Windows is the odd cookie that requires a
complete development environment to be downloaded because there is no
suitable one installed by default on Windows.
Side note: technically, there _is_ a development environment present in
GitHub Actions' build agents: MSYS2. But it differs from Git for
Windows' SDK in subtle points, unfortunately enough so to prevent Git's
test suite from running without failures.
Traditionally, we support downloading this environment (which we
nicknamed `git-sdk-64-minimal`) via a PowerShell scriptlet that accesses
the build artifacts of a dedicated Azure Pipeline (which packages a tiny
subset of the full Git for Windows SDK, containing just enough to build
Git and run its test suite).
This PowerShell script is unfortunately not very robust and sometimes
fails due to network issues.
Of course, we could add code to detect that situation, wait a little,
try again, if it fails again wait a little longer, lather, rinse and
repeat.
Instead of doing all of this in Git's own `.github/workflows/`, though,
let's offload this logic to the new GitHub Action at
https://github.com/marketplace/actions/setup-git-for-windows-sdk
This Action not only downloads and extracts git-sdk-64-minimal _outside_
the worktree (making it no longer necessary to meddle with
`.gitignore` or `.git/info/exclude`), it also adds the `bash.exe` to the
`PATH` and sets the environment variable `MSYSTEM` (an implementation
detail that Git's workflow should never have needed to know about).
This allows us to convert all those funny PowerShell tasks that wanted
to call git-sdk-64-minimal's `bash.exe`: they all are now regular `bash`
scriptlets.
This finally lets us get rid of the funny quoting and escaping where we
had to pay attention not only to quote and escape the Bash scriptlets
properly, but also to add a second level of escaping (with backslashes
for double quotes and backticks for dollar signs) to stop PowerShell
from doing unintended things.
Further, this Action uses a fast caching strategy native to GitHub
Actions that should accelerate the download across CI runs:
git-sdk-64-minimal is usually updated once per 24h, and needs to be
cached only once within that period. Caching it (unfortunately only on
a per-branch basis) speeds up the download step, and makes it much more
robust at the same time by virtue of accessing a cache location that is
closer in the network topology.
With this we can drop the home-rolled caching where we try to accelerate
the test phase by uploading git-sdk-64-minimal as a workflow artifact
after using it to build Git, and then download it as workflow artifact
in the test phase.
Even better: the `vs-test` job no longer needs to depend on the
`windows-build` job. The only reason it depended on it was to ensure
that the `git-sdk-64-minimal` workflow artifact was available.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 103 +++++++------------------------------
1 file changed, 19 insertions(+), 84 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 73856bafc9d..c62766e7b1a 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -82,43 +82,18 @@ jobs:
runs-on: windows-latest
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- shell: bash
- run: |
- ## Get artifact
- urlbase=https://dev.azure.com/git-for-windows/git/_apis/build/builds
- id=$(curl "$urlbase?definitions=22&statusFilter=completed&resultFilter=succeeded&\$top=1" |
- jq -r ".value[] | .id")
- download_url="$(curl "$urlbase/$id/artifacts" |
- jq -r '.value[] | select(.name == "git-sdk-64-minimal").resource.downloadUrl')"
- curl --connect-timeout 10 --retry 5 --retry-delay 0 --retry-max-time 240 \
- -o artifacts.zip "$download_url"
-
- ## Unzip and remove the artifact
- unzip artifacts.zip
- rm artifacts.zip
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
- shell: powershell
+ shell: bash
env:
HOME: ${{runner.workspace}}
- MSYSTEM: MINGW64
NO_PERL: 1
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
-
- ci/make-test-artifacts.sh artifacts
- "@
+ run: ci/make-test-artifacts.sh artifacts
- name: upload build artifacts
uses: actions/upload-artifact@v1
with:
name: windows-artifacts
path: artifacts
- - name: upload git-sdk-64-minimal
- uses: actions/upload-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: git-sdk-64-minimal
windows-test:
runs-on: windows-latest
needs: [windows-build]
@@ -136,25 +111,14 @@ jobs:
- name: extract build artifacts
shell: bash
run: tar xf artifacts.tar.gz
- - name: download git-sdk-64-minimal
- uses: actions/download-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: ${{github.workspace}}/git-sdk-64-minimal/
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: test
- shell: powershell
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- # Let Git ignore the SDK
- printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
-
- ci/run-test-slice.sh ${{matrix.nr}} 10
- "@
+ shell: bash
+ run: 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
+ shell: bash
+ run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v1
@@ -165,27 +129,12 @@ jobs:
needs: ci-config
if: needs.ci-config.outputs.enabled == 'yes'
env:
- MSYSTEM: MINGW64
NO_PERL: 1
GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
runs-on: windows-latest
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- shell: bash
- run: |
- ## Get artifact
- urlbase=https://dev.azure.com/git-for-windows/git/_apis/build/builds
- id=$(curl "$urlbase?definitions=22&statusFilter=completed&resultFilter=succeeded&\$top=1" |
- jq -r ".value[] | .id")
- download_url="$(curl "$urlbase/$id/artifacts" |
- jq -r '.value[] | select(.name == "git-sdk-64-minimal").resource.downloadUrl')"
- curl --connect-timeout 10 --retry 5 --retry-delay 0 --retry-max-time 240 \
- -o artifacts.zip "$download_url"
-
- ## Unzip and remove the artifact
- unzip artifacts.zip
- rm artifacts.zip
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
uses: actions/checkout@v2
with:
@@ -211,19 +160,17 @@ jobs:
shell: bash
run: |
cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
- -DMSGFMT_EXE=`pwd`/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
+ -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
- shell: powershell
+ shell: bash
env:
MSVC: 1
VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
run: |
- & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- mkdir -p artifacts &&
- eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\"
- "@
+ mkdir -p artifacts &&
+ eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- name: upload build artifacts
uses: actions/upload-artifact@v1
with:
@@ -231,18 +178,14 @@ jobs:
path: artifacts
vs-test:
runs-on: windows-latest
- needs: [vs-build, windows-build]
+ needs: vs-build
strategy:
fail-fast: false
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- uses: actions/download-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: ${{github.workspace}}/git-sdk-64-minimal/
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download build artifacts
uses: actions/download-artifact@v1
with:
@@ -252,23 +195,15 @@ jobs:
shell: bash
run: tar xf artifacts.tar.gz
- name: test
- shell: powershell
+ shell: bash
env:
- MSYSTEM: MINGW64
NO_SVN_TESTS: 1
GIT_TEST_SKIP_REBASE_P: 1
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- # Let Git ignore the SDK and the test-cache
- printf '%s\n' /git-sdk-64-minimal/ /test-cache/ >>.git/info/exclude
-
- ci/run-test-slice.sh ${{matrix.nr}} 10
- "@
+ run: 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
+ shell: bash
+ run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We use a `.bat` script to copy the DLLs in the `vs-build` job, and those
type of scripts are native to CMD, not to PowerShell.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c62766e7b1a..d430c4e0d20 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -152,10 +152,8 @@ jobs:
- name: add msbuild to PATH
uses: microsoft/setup-msbuild@v1
- name: copy dlls to root
- shell: powershell
- run: |
- & compat\vcbuild\vcpkg_copy_dlls.bat release
- if (!$?) { exit(1) }
+ shell: cmd
+ run: compat\vcbuild\vcpkg_copy_dlls.bat release
- name: generate Visual Studio solution
shell: bash
run: |
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 4/7] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The GitHub Actions to upload/download workflow artifacts saw a major
upgrade since Git's GitHub workflow was established. Let's use it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index d430c4e0d20..a399114c0f8 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -90,7 +90,7 @@ jobs:
NO_PERL: 1
run: ci/make-test-artifacts.sh artifacts
- name: upload build artifacts
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: windows-artifacts
path: artifacts
@@ -104,7 +104,7 @@ jobs:
steps:
- uses: actions/checkout@v1
- name: download build artifacts
- uses: actions/download-artifact@v1
+ uses: actions/download-artifact@v2
with:
name: windows-artifacts
path: ${{github.workspace}}
@@ -121,7 +121,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-windows
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -170,7 +170,7 @@ jobs:
mkdir -p artifacts &&
eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- name: upload build artifacts
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: vs-artifacts
path: artifacts
@@ -185,7 +185,7 @@ jobs:
- uses: actions/checkout@v1
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download build artifacts
- uses: actions/download-artifact@v1
+ uses: actions/download-artifact@v2
with:
name: vs-artifacts
path: ${{github.workspace}}
@@ -204,7 +204,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-windows
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -242,7 +242,7 @@ jobs:
if: failure()
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -269,7 +269,7 @@ jobs:
if: failure()
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 4/7] ci(windows): transfer also the Git-tracked files to the test jobs
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2021-07-03 21:26 ` [PATCH v2 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Git's test suite is excruciatingly slow on Windows, mainly due to the
fact that it executes a lot of shell script code, and that's simply not
native to Windows.
To help with that, we established the pattern where the artifacts are
first built in one job, and then multiple test jobs run in parallel
using the artifacts built in the first job.
We take pains in transferring only the build outputs, and letting
`actions/checkout` fill in the rest of the files.
One major downside of that strategy is that the test jobs might fail to
check out the intended revision (e.g. because the branch has been
updated while the build was running, as is frequently the case with the
`seen` branch).
Let's transfer also the files tracked by Git, and skip the checkout step
in the test jobs.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a399114c0f8..0f7516c9ef3 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -89,7 +89,9 @@ jobs:
HOME: ${{runner.workspace}}
NO_PERL: 1
run: ci/make-test-artifacts.sh artifacts
- - name: upload build artifacts
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
uses: actions/upload-artifact@v2
with:
name: windows-artifacts
@@ -102,15 +104,14 @@ jobs:
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- - uses: actions/checkout@v1
- - name: download build artifacts
+ - name: download tracked files and build artifacts
uses: actions/download-artifact@v2
with:
name: windows-artifacts
path: ${{github.workspace}}
- - name: extract build artifacts
+ - name: extract tracked files and build artifacts
shell: bash
- run: tar xf artifacts.tar.gz
+ run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: test
shell: bash
@@ -169,7 +170,9 @@ jobs:
run: |
mkdir -p artifacts &&
eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- - name: upload build artifacts
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
uses: actions/upload-artifact@v2
with:
name: vs-artifacts
@@ -182,16 +185,15 @@ jobs:
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- - uses: actions/checkout@v1
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- - name: download build artifacts
+ - name: download tracked files and build artifacts
uses: actions/download-artifact@v2
with:
name: vs-artifacts
path: ${{github.workspace}}
- - name: extract build artifacts
+ - name: extract tracked files and build artifacts
shell: bash
- run: tar xf artifacts.tar.gz
+ run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- name: test
shell: bash
env:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2021-07-03 21:26 ` [PATCH v2 4/7] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-04 8:30 ` Ævar Arnfjörð Bjarmason
2021-07-03 21:26 ` [PATCH v2 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
` (2 subsequent siblings)
7 siblings, 1 reply; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We obviously do not want to bundle `.mo` files during `make
artifacts-tar NO_GETTEXT`, but that was the case.
To fix that, go a step beyond just fixing the symptom, and simply
define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
set.
Helped-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index c3565fc0f8f..04e852be015 100644
--- a/Makefile
+++ b/Makefile
@@ -2675,10 +2675,13 @@ po/git.pot: $(GENERATED_H) FORCE
.PHONY: pot
pot: po/git.pot
+ifdef NO_GETTEXT
+POFILES :=
+MOFILES :=
+else
POFILES := $(wildcard po/*.po)
MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
-ifndef NO_GETTEXT
all:: $(MOFILES)
endif
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 6/7] ci(vs-build): build with NO_GETTEXT
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2021-07-03 21:26 ` [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
@ 2021-07-03 21:26 ` Dennis Ameling via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
7 siblings, 0 replies; 41+ messages in thread
From: Dennis Ameling via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Dennis Ameling
From: Dennis Ameling <dennis@dennisameling.com>
We already build Git for Windows with `NO_GETTEXT` when compiling with
GCC. Let's do the same with Visual C, too.
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0f7516c9ef3..3b40c677ab5 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -159,7 +159,7 @@ jobs:
shell: bash
run: |
cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
- -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
+ -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 7/7] ci: accelerate the checkout
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2021-07-03 21:26 ` [PATCH v2 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
@ 2021-07-03 21:26 ` Johannes Schindelin via GitGitGadget
2021-07-04 8:54 ` Ævar Arnfjörð Bjarmason
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
7 siblings, 1 reply; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-03 21:26 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
By upgrading from v1 to v2 of `actions/checkout`, we avoid fetching all
the tags and the complete history: v2 only fetches one revision by
default. This should make things a lot faster.
Note that `actions/checkout@v2` seems to be incompatible with running in
containers: https://github.com/actions/checkout/issues/151. Therefore,
we stick with v1 there.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3b40c677ab5..405204c78a7 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -81,7 +81,7 @@ jobs:
if: needs.ci-config.outputs.enabled == 'yes'
runs-on: windows-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
shell: bash
@@ -134,7 +134,7 @@ jobs:
GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
runs-on: windows-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
uses: actions/checkout@v2
@@ -237,7 +237,7 @@ jobs:
jobname: ${{matrix.vector.jobname}}
runs-on: ${{matrix.vector.pool}}
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/run-build-and-tests.sh
- run: ci/print-test-failures.sh
@@ -282,7 +282,7 @@ jobs:
jobname: StaticAnalysis
runs-on: ubuntu-18.04
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/run-static-analysis.sh
documentation:
@@ -292,6 +292,6 @@ jobs:
jobname: Documentation
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/test-documentation.sh
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-06-23 15:24 ` [PATCH 5/6] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
@ 2021-07-04 8:27 ` Ævar Arnfjörð Bjarmason
2021-07-04 8:52 ` Ævar Arnfjörð Bjarmason
2021-07-13 12:19 ` Johannes Schindelin
0 siblings, 2 replies; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-04 8:27 UTC (permalink / raw)
To: Dennis Ameling via GitGitGadget; +Cc: git, Johannes Schindelin, Dennis Ameling
On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
> From: Dennis Ameling <dennis@dennisameling.com>
>
> We already build Git for Windows with `NO_GETTEXT` when compiling with
> GCC. Let's do the same with Visual C, too.
>
> Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> .github/workflows/main.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 0f7516c9ef..3b40c677ab 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -159,7 +159,7 @@ jobs:
> shell: bash
> run: |
> cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
> - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> - name: MSBuild
> run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> - name: bundle artifact tar
To a first approximation isn't the point of CI to test a diversity of
setups? So "we already do X in job A, let's do it X in job B" seems like
the opposite of a self-justifying rationale, surely our default approach
would be to do things differently?
Perhaps this change makes sense, just commenting on the explanation in
the commit message. I think it should cover:
* Why: Is it being done for speed, hard to install libintl or whatever?
* Coverage: Before we tested on this platform with this knob, do we
have another CI job that covers what we'll lose in coverage here, if
not is it OK because it's tested in some other way etc?
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT
2021-07-03 21:26 ` [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
@ 2021-07-04 8:30 ` Ævar Arnfjörð Bjarmason
2021-07-04 22:52 ` Johannes Schindelin
0 siblings, 1 reply; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-04 8:30 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> We obviously do not want to bundle `.mo` files during `make
> artifacts-tar NO_GETTEXT`, but that was the case.
Should be:
make artifacts-tar NO_GETTEXT=YesPlease
(Without the =<something> we try to find a "NO_GETTEXT" target)
> To fix that, go a step beyond just fixing the symptom, and simply
> define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
> set.
How about fixing the cause instead of the symptom...
> Helped-by: Matthias Aßhauer <mha1993@live.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index c3565fc0f8f..04e852be015 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2675,10 +2675,13 @@ po/git.pot: $(GENERATED_H) FORCE
> .PHONY: pot
> pot: po/git.pot
>
> +ifdef NO_GETTEXT
> +POFILES :=
> +MOFILES :=
> +else
> POFILES := $(wildcard po/*.po)
> MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
>
> -ifndef NO_GETTEXT
> all:: $(MOFILES)
> endif
...i.e. this patch just seems like odd (ab)use of Makefile logic.
Later on in the artifacts-tar rule we rely on our immediate dependency
list in $^ to feed to "tar czf", and here we're going to set $(MOFILES)
to an empty list, just to later interpolate that empty list into that
list of dependencies.
Wouldn't the mores straightforward thing to do be the diff I've got at
the end here, perhaps with a preceding commit just for the split-up of
the dependency list?
This matches how we do things elsewhere, i.e. we don't ifdef e.g. this away:
LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
rather we keep the list as-is, and ifdef the actual addition of the
dependency, e.g.:
ifndef NO_PERL
all:: $(LIB_PERL_GEN)
[...]
endif
One reason we do it like this is because we *don't* want to forget what
the MOFILES were, because you want e.g. "make clean" to clean them up
(not that it matters in this case, we rm -rf po/build).
Doesn't matter much here, but following this pattern leads to subtle
"bugs", e.g. an outstanding issue in your 179227d6e21 (Optionally skip
linking/copying the built-ins, 2020-09-21) (which I noted on-list in
passing before, IIRC) where during a build we end up with stale
built-ins from a previous build in the build directory, because we
pruned the list during definition time, as opposed to adding an inverse
"I should remove this then" rule.
("bug" because it doesn't have any actual effect I know of other than
bothering me that I have e.g. a git-add in my build-dir still :)
diff --git a/Makefile b/Makefile
index c3565fc0f8f..7fb1d4b6caa 100644
--- a/Makefile
+++ b/Makefile
@@ -3146,9 +3146,18 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
endif
-artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
- GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
- $(MOFILES)
+ARTIFACTS_TAR =
+ARTIFACTS_TAR += GIT-BUILD-OPTIONS
+ARTIFACTS_TAR += $(ALL_COMMANDS_TO_INSTALL)
+ARTIFACTS_TAR += $(SCRIPT_LIB)
+ARTIFACTS_TAR += $(OTHER_PROGRAMS)
+ARTIFACTS_TAR += $(TEST_PROGRAMS)
+ARTIFACTS_TAR += $(test_bindir_programs)
+ifndef NO_GETTEXT
+ARTIFACTS_TAR += $(MOFILES)
+endif
+
+artifacts-tar:: $(ARTIFACTS_TAR)
$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
test -n "$(ARTIFACTS_DIRECTORY)"
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-07-04 8:27 ` Ævar Arnfjörð Bjarmason
@ 2021-07-04 8:52 ` Ævar Arnfjörð Bjarmason
2021-07-13 12:19 ` Johannes Schindelin
1 sibling, 0 replies; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-04 8:52 UTC (permalink / raw)
To: Dennis Ameling via GitGitGadget; +Cc: git, Johannes Schindelin, Dennis Ameling
On Sun, Jul 04 2021, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
>
>> From: Dennis Ameling <dennis@dennisameling.com>
>>
>> We already build Git for Windows with `NO_GETTEXT` when compiling with
>> GCC. Let's do the same with Visual C, too.
>>
>> Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>> .github/workflows/main.yml | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index 0f7516c9ef..3b40c677ab 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -159,7 +159,7 @@ jobs:
>> shell: bash
>> run: |
>> cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
>> - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
>> + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
>> - name: MSBuild
>> run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
>> - name: bundle artifact tar
>
> To a first approximation isn't the point of CI to test a diversity of
> setups? So "we already do X in job A, let's do it X in job B" seems like
> the opposite of a self-justifying rationale, surely our default approach
> would be to do things differently?
>
> Perhaps this change makes sense, just commenting on the explanation in
> the commit message. I think it should cover:
>
> * Why: Is it being done for speed, hard to install libintl or whatever?
>
> * Coverage: Before we tested on this platform with this knob, do we
> have another CI job that covers what we'll lose in coverage here, if
> not is it OK because it's tested in some other way etc?
Aside: I should have replied to the v2[1] version of this, but the same
comment applies to it. Sorry about the confusion.
1. http://lore.kernel.org/git/8bab4c17b8a6d7a3ed66e5d9e3eb8f598b35b874.1625347592.git.gitgitgadget@gmail.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 7/7] ci: accelerate the checkout
2021-07-03 21:26 ` [PATCH v2 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
@ 2021-07-04 8:54 ` Ævar Arnfjörð Bjarmason
2021-07-04 22:37 ` Johannes Schindelin
0 siblings, 1 reply; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-04 8:54 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> By upgrading from v1 to v2 of `actions/checkout`, we avoid fetching all
> the tags and the complete history: v2 only fetches one revision by
> default. This should make things a lot faster.
>
> Note that `actions/checkout@v2` seems to be incompatible with running in
> containers: https://github.com/actions/checkout/issues/151. Therefore,
> we stick with v1 there.
I'd suggest that we shouldn't link to a "closed" issue here and instead
to what seems to be the successor issue:
https://github.com/actions/checkout/issues/334
But looking at #151 most of the issue is a bazillion commit references
to this commit being rebased again and again, seems like github isn't
especially well set up for the "spam" our perpetual rebasing of the same
commits causes :)
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 7/7] ci: accelerate the checkout
2021-07-04 8:54 ` Ævar Arnfjörð Bjarmason
@ 2021-07-04 22:37 ` Johannes Schindelin
0 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin @ 2021-07-04 22:37 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Johannes Schindelin via GitGitGadget, git
[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]
Hi Ævar,
On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
>
> On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > By upgrading from v1 to v2 of `actions/checkout`, we avoid fetching all
> > the tags and the complete history: v2 only fetches one revision by
> > default. This should make things a lot faster.
> >
> > Note that `actions/checkout@v2` seems to be incompatible with running in
> > containers: https://github.com/actions/checkout/issues/151. Therefore,
> > we stick with v1 there.
>
> I'd suggest that we shouldn't link to a "closed" issue here and instead
> to what seems to be the successor issue:
> https://github.com/actions/checkout/issues/334
I'd suggest that we can still link to this issue, even if it was closed
without the bug actually having been fixed. The ticket describes the
problem well.
> But looking at #151 most of the issue is a bazillion commit references
> to this commit being rebased again and again, seems like github isn't
> especially well set up for the "spam" our perpetual rebasing of the same
> commits causes :)
Sure, blame me.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT
2021-07-04 8:30 ` Ævar Arnfjörð Bjarmason
@ 2021-07-04 22:52 ` Johannes Schindelin
2021-07-05 6:33 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 41+ messages in thread
From: Johannes Schindelin @ 2021-07-04 22:52 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Johannes Schindelin via GitGitGadget, git
[-- Attachment #1: Type: text/plain, Size: 4694 bytes --]
Hi Ævar,
On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
> On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > We obviously do not want to bundle `.mo` files during `make
> > artifacts-tar NO_GETTEXT`, but that was the case.
>
> Should be:
>
> make artifacts-tar NO_GETTEXT=YesPlease
>
> (Without the =<something> we try to find a "NO_GETTEXT" target)
Correct. Will fix.
> > To fix that, go a step beyond just fixing the symptom, and simply
> > define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
> > set.
>
> How about fixing the cause instead of the symptom...
Yes, from my point of view, I did that.
> > Helped-by: Matthias Aßhauer <mha1993@live.de>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > Makefile | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index c3565fc0f8f..04e852be015 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2675,10 +2675,13 @@ po/git.pot: $(GENERATED_H) FORCE
> > .PHONY: pot
> > pot: po/git.pot
> >
> > +ifdef NO_GETTEXT
> > +POFILES :=
> > +MOFILES :=
> > +else
> > POFILES := $(wildcard po/*.po)
> > MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
> >
> > -ifndef NO_GETTEXT
> > all:: $(MOFILES)
> > endif
>
> ...i.e. this patch just seems like odd (ab)use of Makefile logic.
>
> Later on in the artifacts-tar rule we rely on our immediate dependency
> list in $^ to feed to "tar czf", and here we're going to set $(MOFILES)
> to an empty list, just to later interpolate that empty list into that
> list of dependencies.
>
> Wouldn't the mores straightforward thing to do be the diff I've got at
> the end here, perhaps with a preceding commit just for the split-up of
> the dependency list?
>
> This matches how we do things elsewhere, i.e. we don't ifdef e.g. this away:
>
> LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
> LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
>
> rather we keep the list as-is, and ifdef the actual addition of the
> dependency, e.g.:
>
> ifndef NO_PERL
> all:: $(LIB_PERL_GEN)
> [...]
> endif
>
> One reason we do it like this is because we *don't* want to forget what
> the MOFILES were, because you want e.g. "make clean" to clean them up
> (not that it matters in this case, we rm -rf po/build).
We don't need to be careful about cleaning files we did not generate in
the first place.
Your suggestion amounts to unnecessary work. If we asked for NO_GETTEXT,
why bother generating the list of `.po` files at all? (Rhetorical
question; The answer is "we don't need to".)
> Doesn't matter much here, but following this pattern leads to subtle
> "bugs", e.g. an outstanding issue in your 179227d6e21 (Optionally skip
> linking/copying the built-ins, 2020-09-21) (which I noted on-list in
> passing before, IIRC) where during a build we end up with stale
> built-ins from a previous build in the build directory, because we
> pruned the list during definition time, as opposed to adding an inverse
> "I should remove this then" rule.
>
> ("bug" because it doesn't have any actual effect I know of other than
> bothering me that I have e.g. a git-add in my build-dir still :)
>
> diff --git a/Makefile b/Makefile
> index c3565fc0f8f..7fb1d4b6caa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3146,9 +3146,18 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
> OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
> endif
>
> -artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
> - GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
> - $(MOFILES)
> +ARTIFACTS_TAR =
> +ARTIFACTS_TAR += GIT-BUILD-OPTIONS
> +ARTIFACTS_TAR += $(ALL_COMMANDS_TO_INSTALL)
> +ARTIFACTS_TAR += $(SCRIPT_LIB)
> +ARTIFACTS_TAR += $(OTHER_PROGRAMS)
> +ARTIFACTS_TAR += $(TEST_PROGRAMS)
> +ARTIFACTS_TAR += $(test_bindir_programs)
> +ifndef NO_GETTEXT
> +ARTIFACTS_TAR += $(MOFILES)
> +endif
> +
> +artifacts-tar:: $(ARTIFACTS_TAR)
Apart from going out of its way to retain the construction of the `.po`
file list (which is totally pointless when operating under `NO_GETTEXT`),
this is also a sore to my eyes. So I won't do that.
Thank you for trying to assist in improving this patch series,
Dscho
> $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
> SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
> test -n "$(ARTIFACTS_DIRECTORY)"
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2021-07-03 21:26 ` [PATCH v2 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
` (7 more replies)
7 siblings, 8 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git; +Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin
This patch series upgrades to newer versions of a couple GitHub Actions we
use, and also streamlines the Windows jobs using the relatively new
setup-git-for-windows-sdk Action
[https://github.com/marketplace/actions/setup-git-for-windows-sdk] (Git for
Windows is running with this Action for a while now, getting all the kinks
out).
This patch series should also address the problem where seen was pushed so
rapidly that the windows-test jobs failed because they no longer checked out
the identical revision as the windows-build job.
Changes since v2:
* Made the handwaving make [...] NO_GETTEXT comment in the commit message
of the patch "artifacts-tar: respect NO_GETTEXT" more explicit, by
setting NO_GETTEXT to a bogus value as required by make.
* Added an explicit NO_GETTEXT=YesPlease to the make artifacts-tar
invocation in the vs-build job, as well as an explanation in the
corresponding commit message why this explicit mention is technically not
required.
Changes since v1:
* Added a patch to fix make NO_GETTEXT=Yep artifacts-tar (not to include
.mo files), as suggested by Matthias Aßauer in the GitGitGadget PR, which
should fix the CI failure in seen that Junio pointed out. The bug was
unhidden by mr/cmake fixing the CMake build (which ignored NO_GETTEXT
before).
Dennis Ameling (1):
ci(vs-build): build with NO_GETTEXT
Johannes Schindelin (6):
ci: use the new GitHub Action to download git-sdk-64-minimal
ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
ci: upgrade to using actions/{up,down}load-artifacts v2
ci(windows): transfer also the Git-tracked files to the test jobs
artifacts-tar: respect NO_GETTEXT
ci: accelerate the checkout
.github/workflows/main.yml | 157 +++++++++++--------------------------
Makefile | 5 +-
2 files changed, 50 insertions(+), 112 deletions(-)
base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-878%2Fdscho%2Fuse-setup-git-for-windows-sdk-action-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-878/dscho/use-setup-git-for-windows-sdk-action-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/878
Range-diff vs v2:
1: 2e4db688deb = 1: 2e4db688deb ci: use the new GitHub Action to download git-sdk-64-minimal
2: 6b12fe2284c = 2: 6b12fe2284c ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
3: c256bbf4b1c = 3: c256bbf4b1c ci: upgrade to using actions/{up,down}load-artifacts v2
4: 59dc44428fb = 4: 59dc44428fb ci(windows): transfer also the Git-tracked files to the test jobs
5: c31d2e7f44a ! 5: 64f7b1f4e23 artifacts-tar: respect NO_GETTEXT
@@ Commit message
artifacts-tar: respect NO_GETTEXT
We obviously do not want to bundle `.mo` files during `make
- artifacts-tar NO_GETTEXT`, but that was the case.
+ artifacts-tar NO_GETTEXT=Yep`, but that was the case.
To fix that, go a step beyond just fixing the symptom, and simply
define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
6: 8bab4c17b8a ! 6: 2c4cd9dd1c8 ci(vs-build): build with NO_GETTEXT
@@ Commit message
We already build Git for Windows with `NO_GETTEXT` when compiling with
GCC. Let's do the same with Visual C, too.
+ Note that we do not technically _need_ to pass `NO_GETTEXT` explicitly
+ in that `make artifacts-tar` invocation because we do this while `MSVC`
+ is set (which will set `uname_S := Windows`, which in turn will set
+ `NO_GETTEXT = YesPlease`). But it is definitely nicer to be explicit
+ here.
+
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
+ Helped-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## .github/workflows/main.yml ##
@@ .github/workflows/main.yml: jobs:
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
+@@ .github/workflows/main.yml: jobs:
+ VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
+ run: |
+ mkdir -p artifacts &&
+- eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
++ eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)"
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
7: 88a44863cd0 = 7: db54bf9a779 ci: accelerate the checkout
--
gitgitgadget
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-06 19:16 ` Junio C Hamano
2021-07-04 22:55 ` [PATCH v3 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
7 siblings, 1 reply; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In our continuous builds, Windows is the odd cookie that requires a
complete development environment to be downloaded because there is no
suitable one installed by default on Windows.
Side note: technically, there _is_ a development environment present in
GitHub Actions' build agents: MSYS2. But it differs from Git for
Windows' SDK in subtle points, unfortunately enough so to prevent Git's
test suite from running without failures.
Traditionally, we support downloading this environment (which we
nicknamed `git-sdk-64-minimal`) via a PowerShell scriptlet that accesses
the build artifacts of a dedicated Azure Pipeline (which packages a tiny
subset of the full Git for Windows SDK, containing just enough to build
Git and run its test suite).
This PowerShell script is unfortunately not very robust and sometimes
fails due to network issues.
Of course, we could add code to detect that situation, wait a little,
try again, if it fails again wait a little longer, lather, rinse and
repeat.
Instead of doing all of this in Git's own `.github/workflows/`, though,
let's offload this logic to the new GitHub Action at
https://github.com/marketplace/actions/setup-git-for-windows-sdk
This Action not only downloads and extracts git-sdk-64-minimal _outside_
the worktree (making it no longer necessary to meddle with
`.gitignore` or `.git/info/exclude`), it also adds the `bash.exe` to the
`PATH` and sets the environment variable `MSYSTEM` (an implementation
detail that Git's workflow should never have needed to know about).
This allows us to convert all those funny PowerShell tasks that wanted
to call git-sdk-64-minimal's `bash.exe`: they all are now regular `bash`
scriptlets.
This finally lets us get rid of the funny quoting and escaping where we
had to pay attention not only to quote and escape the Bash scriptlets
properly, but also to add a second level of escaping (with backslashes
for double quotes and backticks for dollar signs) to stop PowerShell
from doing unintended things.
Further, this Action uses a fast caching strategy native to GitHub
Actions that should accelerate the download across CI runs:
git-sdk-64-minimal is usually updated once per 24h, and needs to be
cached only once within that period. Caching it (unfortunately only on
a per-branch basis) speeds up the download step, and makes it much more
robust at the same time by virtue of accessing a cache location that is
closer in the network topology.
With this we can drop the home-rolled caching where we try to accelerate
the test phase by uploading git-sdk-64-minimal as a workflow artifact
after using it to build Git, and then download it as workflow artifact
in the test phase.
Even better: the `vs-test` job no longer needs to depend on the
`windows-build` job. The only reason it depended on it was to ensure
that the `git-sdk-64-minimal` workflow artifact was available.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 103 +++++++------------------------------
1 file changed, 19 insertions(+), 84 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 73856bafc9d..c62766e7b1a 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -82,43 +82,18 @@ jobs:
runs-on: windows-latest
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- shell: bash
- run: |
- ## Get artifact
- urlbase=https://dev.azure.com/git-for-windows/git/_apis/build/builds
- id=$(curl "$urlbase?definitions=22&statusFilter=completed&resultFilter=succeeded&\$top=1" |
- jq -r ".value[] | .id")
- download_url="$(curl "$urlbase/$id/artifacts" |
- jq -r '.value[] | select(.name == "git-sdk-64-minimal").resource.downloadUrl')"
- curl --connect-timeout 10 --retry 5 --retry-delay 0 --retry-max-time 240 \
- -o artifacts.zip "$download_url"
-
- ## Unzip and remove the artifact
- unzip artifacts.zip
- rm artifacts.zip
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
- shell: powershell
+ shell: bash
env:
HOME: ${{runner.workspace}}
- MSYSTEM: MINGW64
NO_PERL: 1
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
-
- ci/make-test-artifacts.sh artifacts
- "@
+ run: ci/make-test-artifacts.sh artifacts
- name: upload build artifacts
uses: actions/upload-artifact@v1
with:
name: windows-artifacts
path: artifacts
- - name: upload git-sdk-64-minimal
- uses: actions/upload-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: git-sdk-64-minimal
windows-test:
runs-on: windows-latest
needs: [windows-build]
@@ -136,25 +111,14 @@ jobs:
- name: extract build artifacts
shell: bash
run: tar xf artifacts.tar.gz
- - name: download git-sdk-64-minimal
- uses: actions/download-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: ${{github.workspace}}/git-sdk-64-minimal/
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: test
- shell: powershell
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- # Let Git ignore the SDK
- printf '%s\n' /git-sdk-64-minimal/ >>.git/info/exclude
-
- ci/run-test-slice.sh ${{matrix.nr}} 10
- "@
+ shell: bash
+ run: 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
+ shell: bash
+ run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v1
@@ -165,27 +129,12 @@ jobs:
needs: ci-config
if: needs.ci-config.outputs.enabled == 'yes'
env:
- MSYSTEM: MINGW64
NO_PERL: 1
GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
runs-on: windows-latest
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- shell: bash
- run: |
- ## Get artifact
- urlbase=https://dev.azure.com/git-for-windows/git/_apis/build/builds
- id=$(curl "$urlbase?definitions=22&statusFilter=completed&resultFilter=succeeded&\$top=1" |
- jq -r ".value[] | .id")
- download_url="$(curl "$urlbase/$id/artifacts" |
- jq -r '.value[] | select(.name == "git-sdk-64-minimal").resource.downloadUrl')"
- curl --connect-timeout 10 --retry 5 --retry-delay 0 --retry-max-time 240 \
- -o artifacts.zip "$download_url"
-
- ## Unzip and remove the artifact
- unzip artifacts.zip
- rm artifacts.zip
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
uses: actions/checkout@v2
with:
@@ -211,19 +160,17 @@ jobs:
shell: bash
run: |
cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
- -DMSGFMT_EXE=`pwd`/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
+ -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
- shell: powershell
+ shell: bash
env:
MSVC: 1
VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
run: |
- & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- mkdir -p artifacts &&
- eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\"
- "@
+ mkdir -p artifacts &&
+ eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- name: upload build artifacts
uses: actions/upload-artifact@v1
with:
@@ -231,18 +178,14 @@ jobs:
path: artifacts
vs-test:
runs-on: windows-latest
- needs: [vs-build, windows-build]
+ needs: vs-build
strategy:
fail-fast: false
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- uses: actions/checkout@v1
- - name: download git-sdk-64-minimal
- uses: actions/download-artifact@v1
- with:
- name: git-sdk-64-minimal
- path: ${{github.workspace}}/git-sdk-64-minimal/
+ - uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download build artifacts
uses: actions/download-artifact@v1
with:
@@ -252,23 +195,15 @@ jobs:
shell: bash
run: tar xf artifacts.tar.gz
- name: test
- shell: powershell
+ shell: bash
env:
- MSYSTEM: MINGW64
NO_SVN_TESTS: 1
GIT_TEST_SKIP_REBASE_P: 1
- run: |
- & .\git-sdk-64-minimal\usr\bin\bash.exe -lc @"
- # Let Git ignore the SDK and the test-cache
- printf '%s\n' /git-sdk-64-minimal/ /test-cache/ >>.git/info/exclude
-
- ci/run-test-slice.sh ${{matrix.nr}} 10
- "@
+ run: 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
+ shell: bash
+ run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
uses: actions/upload-artifact@v1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell`
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We use a `.bat` script to copy the DLLs in the `vs-build` job, and those
type of scripts are native to CMD, not to PowerShell.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c62766e7b1a..d430c4e0d20 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -152,10 +152,8 @@ jobs:
- name: add msbuild to PATH
uses: microsoft/setup-msbuild@v1
- name: copy dlls to root
- shell: powershell
- run: |
- & compat\vcbuild\vcpkg_copy_dlls.bat release
- if (!$?) { exit(1) }
+ shell: cmd
+ run: compat\vcbuild\vcpkg_copy_dlls.bat release
- name: generate Visual Studio solution
shell: bash
run: |
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 4/7] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The GitHub Actions to upload/download workflow artifacts saw a major
upgrade since Git's GitHub workflow was established. Let's use it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index d430c4e0d20..a399114c0f8 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -90,7 +90,7 @@ jobs:
NO_PERL: 1
run: ci/make-test-artifacts.sh artifacts
- name: upload build artifacts
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: windows-artifacts
path: artifacts
@@ -104,7 +104,7 @@ jobs:
steps:
- uses: actions/checkout@v1
- name: download build artifacts
- uses: actions/download-artifact@v1
+ uses: actions/download-artifact@v2
with:
name: windows-artifacts
path: ${{github.workspace}}
@@ -121,7 +121,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-windows
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -170,7 +170,7 @@ jobs:
mkdir -p artifacts &&
eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- name: upload build artifacts
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: vs-artifacts
path: artifacts
@@ -185,7 +185,7 @@ jobs:
- uses: actions/checkout@v1
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download build artifacts
- uses: actions/download-artifact@v1
+ uses: actions/download-artifact@v2
with:
name: vs-artifacts
path: ${{github.workspace}}
@@ -204,7 +204,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-windows
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -242,7 +242,7 @@ jobs:
if: failure()
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -269,7 +269,7 @@ jobs:
if: failure()
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v1
+ uses: actions/upload-artifact@v2
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 4/7] ci(windows): transfer also the Git-tracked files to the test jobs
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2021-07-04 22:55 ` [PATCH v3 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Git's test suite is excruciatingly slow on Windows, mainly due to the
fact that it executes a lot of shell script code, and that's simply not
native to Windows.
To help with that, we established the pattern where the artifacts are
first built in one job, and then multiple test jobs run in parallel
using the artifacts built in the first job.
We take pains in transferring only the build outputs, and letting
`actions/checkout` fill in the rest of the files.
One major downside of that strategy is that the test jobs might fail to
check out the intended revision (e.g. because the branch has been
updated while the build was running, as is frequently the case with the
`seen` branch).
Let's transfer also the files tracked by Git, and skip the checkout step
in the test jobs.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a399114c0f8..0f7516c9ef3 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -89,7 +89,9 @@ jobs:
HOME: ${{runner.workspace}}
NO_PERL: 1
run: ci/make-test-artifacts.sh artifacts
- - name: upload build artifacts
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
uses: actions/upload-artifact@v2
with:
name: windows-artifacts
@@ -102,15 +104,14 @@ jobs:
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- - uses: actions/checkout@v1
- - name: download build artifacts
+ - name: download tracked files and build artifacts
uses: actions/download-artifact@v2
with:
name: windows-artifacts
path: ${{github.workspace}}
- - name: extract build artifacts
+ - name: extract tracked files and build artifacts
shell: bash
- run: tar xf artifacts.tar.gz
+ run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: test
shell: bash
@@ -169,7 +170,9 @@ jobs:
run: |
mkdir -p artifacts &&
eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
- - name: upload build artifacts
+ - name: zip up tracked files
+ run: git archive -o artifacts/tracked.tar.gz HEAD
+ - name: upload tracked files and build artifacts
uses: actions/upload-artifact@v2
with:
name: vs-artifacts
@@ -182,16 +185,15 @@ jobs:
matrix:
nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
steps:
- - uses: actions/checkout@v1
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- - name: download build artifacts
+ - name: download tracked files and build artifacts
uses: actions/download-artifact@v2
with:
name: vs-artifacts
path: ${{github.workspace}}
- - name: extract build artifacts
+ - name: extract tracked files and build artifacts
shell: bash
- run: tar xf artifacts.tar.gz
+ run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- name: test
shell: bash
env:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 5/7] artifacts-tar: respect NO_GETTEXT
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2021-07-04 22:55 ` [PATCH v3 4/7] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
` (2 subsequent siblings)
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We obviously do not want to bundle `.mo` files during `make
artifacts-tar NO_GETTEXT=Yep`, but that was the case.
To fix that, go a step beyond just fixing the symptom, and simply
define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
set.
Helped-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index c3565fc0f8f..04e852be015 100644
--- a/Makefile
+++ b/Makefile
@@ -2675,10 +2675,13 @@ po/git.pot: $(GENERATED_H) FORCE
.PHONY: pot
pot: po/git.pot
+ifdef NO_GETTEXT
+POFILES :=
+MOFILES :=
+else
POFILES := $(wildcard po/*.po)
MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
-ifndef NO_GETTEXT
all:: $(MOFILES)
endif
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2021-07-04 22:55 ` [PATCH v3 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
@ 2021-07-04 22:55 ` Dennis Ameling via GitGitGadget
2021-07-05 6:45 ` Ævar Arnfjörð Bjarmason
2021-07-06 19:19 ` Junio C Hamano
2021-07-04 22:55 ` [PATCH v3 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
2021-07-06 23:20 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Junio C Hamano
7 siblings, 2 replies; 41+ messages in thread
From: Dennis Ameling via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Dennis Ameling
From: Dennis Ameling <dennis@dennisameling.com>
We already build Git for Windows with `NO_GETTEXT` when compiling with
GCC. Let's do the same with Visual C, too.
Note that we do not technically _need_ to pass `NO_GETTEXT` explicitly
in that `make artifacts-tar` invocation because we do this while `MSVC`
is set (which will set `uname_S := Windows`, which in turn will set
`NO_GETTEXT = YesPlease`). But it is definitely nicer to be explicit
here.
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Helped-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0f7516c9ef3..c99628681ef 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -159,7 +159,7 @@ jobs:
shell: bash
run: |
cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
- -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
+ -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
- name: MSBuild
run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
- name: bundle artifact tar
@@ -169,7 +169,7 @@ jobs:
VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
run: |
mkdir -p artifacts &&
- eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
+ eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)"
- name: zip up tracked files
run: git archive -o artifacts/tracked.tar.gz HEAD
- name: upload tracked files and build artifacts
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 7/7] ci: accelerate the checkout
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2021-07-04 22:55 ` [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
@ 2021-07-04 22:55 ` Johannes Schindelin via GitGitGadget
2021-07-06 23:20 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Junio C Hamano
7 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2021-07-04 22:55 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
By upgrading from v1 to v2 of `actions/checkout`, we avoid fetching all
the tags and the complete history: v2 only fetches one revision by
default. This should make things a lot faster.
Note that `actions/checkout@v2` seems to be incompatible with running in
containers: https://github.com/actions/checkout/issues/151. Therefore,
we stick with v1 there.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index c99628681ef..e6f99e29a3d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -81,7 +81,7 @@ jobs:
if: needs.ci-config.outputs.enabled == 'yes'
runs-on: windows-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
shell: bash
@@ -134,7 +134,7 @@ jobs:
GIT_CONFIG_PARAMETERS: "'user.name=CI' 'user.email=ci@git'"
runs-on: windows-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
uses: actions/checkout@v2
@@ -237,7 +237,7 @@ jobs:
jobname: ${{matrix.vector.jobname}}
runs-on: ${{matrix.vector.pool}}
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/run-build-and-tests.sh
- run: ci/print-test-failures.sh
@@ -282,7 +282,7 @@ jobs:
jobname: StaticAnalysis
runs-on: ubuntu-18.04
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/run-static-analysis.sh
documentation:
@@ -292,6 +292,6 @@ jobs:
jobname: Documentation
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v1
+ - uses: actions/checkout@v2
- run: ci/install-dependencies.sh
- run: ci/test-documentation.sh
--
gitgitgadget
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT
2021-07-04 22:52 ` Johannes Schindelin
@ 2021-07-05 6:33 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-05 6:33 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git
On Mon, Jul 05 2021, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> >
>> > We obviously do not want to bundle `.mo` files during `make
>> > artifacts-tar NO_GETTEXT`, but that was the case.
>>
>> Should be:
>>
>> make artifacts-tar NO_GETTEXT=YesPlease
>>
>> (Without the =<something> we try to find a "NO_GETTEXT" target)
>
> Correct. Will fix.
>
>> > To fix that, go a step beyond just fixing the symptom, and simply
>> > define the lists of `.po` and `.mo` files as empty if `NO_GETTEXT` is
>> > set.
>>
>> How about fixing the cause instead of the symptom...
>
> Yes, from my point of view, I did that.
>
>> > Helped-by: Matthias Aßhauer <mha1993@live.de>
>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > ---
>> > Makefile | 5 ++++-
>> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index c3565fc0f8f..04e852be015 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -2675,10 +2675,13 @@ po/git.pot: $(GENERATED_H) FORCE
>> > .PHONY: pot
>> > pot: po/git.pot
>> >
>> > +ifdef NO_GETTEXT
>> > +POFILES :=
>> > +MOFILES :=
>> > +else
>> > POFILES := $(wildcard po/*.po)
>> > MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
>> >
>> > -ifndef NO_GETTEXT
>> > all:: $(MOFILES)
>> > endif
>>
>> ...i.e. this patch just seems like odd (ab)use of Makefile logic.
>>
>> Later on in the artifacts-tar rule we rely on our immediate dependency
>> list in $^ to feed to "tar czf", and here we're going to set $(MOFILES)
>> to an empty list, just to later interpolate that empty list into that
>> list of dependencies.
>>
>> Wouldn't the mores straightforward thing to do be the diff I've got at
>> the end here, perhaps with a preceding commit just for the split-up of
>> the dependency list?
>>
>> This matches how we do things elsewhere, i.e. we don't ifdef e.g. this away:
>>
>> LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
>> LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
>>
>> rather we keep the list as-is, and ifdef the actual addition of the
>> dependency, e.g.:
>>
>> ifndef NO_PERL
>> all:: $(LIB_PERL_GEN)
>> [...]
>> endif
>>
>> One reason we do it like this is because we *don't* want to forget what
>> the MOFILES were, because you want e.g. "make clean" to clean them up
>> (not that it matters in this case, we rm -rf po/build).
>
> We don't need to be careful about cleaning files we did not generate in
> the first place.
>
> Your suggestion amounts to unnecessary work. If we asked for NO_GETTEXT,
> why bother generating the list of `.po` files at all? (Rhetorical
> question; The answer is "we don't need to".)
I'm not saying that you in the Windows CI job generated them, but that
in general we want to support doing these in sequence:
make NO_GETTEXT=Y <target>
make NO_GETTEXT= <target>
...
>> Doesn't matter much here, but following this pattern leads to subtle
>> "bugs", e.g. an outstanding issue in your 179227d6e21 (Optionally skip
>> linking/copying the built-ins, 2020-09-21) (which I noted on-list in
>> passing before, IIRC) where during a build we end up with stale
>> built-ins from a previous build in the build directory, because we
>> pruned the list during definition time, as opposed to adding an inverse
>> "I should remove this then" rule.
>>
>> ("bug" because it doesn't have any actual effect I know of other than
>> bothering me that I have e.g. a git-add in my build-dir still :)
>>
>> diff --git a/Makefile b/Makefile
>> index c3565fc0f8f..7fb1d4b6caa 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -3146,9 +3146,18 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
>> OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)
>> endif
>>
>> -artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
>> - GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
>> - $(MOFILES)
>> +ARTIFACTS_TAR =
>> +ARTIFACTS_TAR += GIT-BUILD-OPTIONS
>> +ARTIFACTS_TAR += $(ALL_COMMANDS_TO_INSTALL)
>> +ARTIFACTS_TAR += $(SCRIPT_LIB)
>> +ARTIFACTS_TAR += $(OTHER_PROGRAMS)
>> +ARTIFACTS_TAR += $(TEST_PROGRAMS)
>> +ARTIFACTS_TAR += $(test_bindir_programs)
>> +ifndef NO_GETTEXT
>> +ARTIFACTS_TAR += $(MOFILES)
>> +endif
>> +
>> +artifacts-tar:: $(ARTIFACTS_TAR)
>
> Apart from going out of its way to retain the construction of the `.po`
> file list (which is totally pointless when operating under `NO_GETTEXT`),
..and that yes, generally speaking there *is* a point in doing
that. E.g. we have another discussion now on-list about incorrectly
spelled/copied config variables in po/*.po files.
It would be a natural thing to want to have some "lint" or "check"
target for that which used $(POFILES) as a source, and you'd not want
that:
make check-pofiles
To do nothing under NO_GETTEXT=Y, but still use other Makefile
dependencies, e.g. use config-list.h as a source of truth.
To be clear I don't think anything's breaking now with your patch, I
just find the pattern of conflating the declaration of files in the
Makefile with the current logic of the rules that happen to need them
right now to be an anti-pattern.
> this is also a sore to my eyes. So I won't do that.
I agree that converting it is an eyesore, that's quite verbose, but it
makes any later patch much easier to read. You'll just need to add
line(s), not modify that big dependency list in-place.
> Thank you for trying to assist in improving this patch series,
> Dscho
>> $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
>> SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
>> test -n "$(ARTIFACTS_DIRECTORY)"
>>
>>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT
2021-07-04 22:55 ` [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
@ 2021-07-05 6:45 ` Ævar Arnfjörð Bjarmason
2021-07-05 12:44 ` Johannes Schindelin
2021-07-06 19:19 ` Junio C Hamano
1 sibling, 1 reply; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-05 6:45 UTC (permalink / raw)
To: Dennis Ameling via GitGitGadget; +Cc: git, Johannes Schindelin, Dennis Ameling
On Sun, Jul 04 2021, Dennis Ameling via GitGitGadget wrote:
> From: Dennis Ameling <dennis@dennisameling.com>
Re the v3 cover letter & my v2 comment:
> We already build Git for Windows with `NO_GETTEXT` when compiling with
> GCC. Let's do the same with Visual C, too.
>
> Note that we do not technically _need_ to pass `NO_GETTEXT` explicitly
> in that `make artifacts-tar` invocation because we do this while `MSVC`
> is set (which will set `uname_S := Windows`, which in turn will set
> `NO_GETTEXT = YesPlease`). But it is definitely nicer to be explicit
> here.
Okey, so we never used it in the first place. That makes the subject and
first paragraph of the commit message seem really out of place. So we
really mean something like this instead?:
ci(vs-build): be explicit about NO_GETTEXT
We already supply `NO_GETTEXT` implicitly due to config.mak.uname,
but let's do so explicitly to ???
But if we're being explicit we also have SKIP_DASHED_BUILT_INS=YesPlease
since ef60e9f74b2 (ci: stop linking built-ins to the dashed versions,
2020-09-21) for CI, which has an even bigger effect on what's included
in the tarball, so it seems odd to single out NO_GETTEXT=YesPlease,
unless there's some other reason to do this that I'm missing.
Hrm, isn't the real reason here that before 5/7 this would error out,
because while NO_GETTEXT=Y was implicit and we picked it up from the
config.mak.uname, we just had the $(MOFILES) in the archive-tar list
unconditionally.
So after 5/7 that's not the case, so we don't need this change anymore,
but we're making this change anyway? Seems like the result of this being
the first try at a fix, and then re-sequencing the two & keeping the
now-redundant hotfix.
> Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
> Helped-by: Matthias Aßhauer <mha1993@live.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> .github/workflows/main.yml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 0f7516c9ef3..c99628681ef 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -159,7 +159,7 @@ jobs:
> shell: bash
> run: |
> cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
> - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> - name: MSBuild
> run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> - name: bundle artifact tar
> @@ -169,7 +169,7 @@ jobs:
> VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
> run: |
> mkdir -p artifacts &&
> - eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
> + eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)"
> - name: zip up tracked files
> run: git archive -o artifacts/tracked.tar.gz HEAD
> - name: upload tracked files and build artifacts
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT
2021-07-05 6:45 ` Ævar Arnfjörð Bjarmason
@ 2021-07-05 12:44 ` Johannes Schindelin
0 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin @ 2021-07-05 12:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Dennis Ameling via GitGitGadget, git, Dennis Ameling
[-- Attachment #1: Type: text/plain, Size: 4603 bytes --]
Hi Ævar,
On Mon, 5 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
> On Sun, Jul 04 2021, Dennis Ameling via GitGitGadget wrote:
>
> > From: Dennis Ameling <dennis@dennisameling.com>
>
> Re the v3 cover letter & my v2 comment:
>
> > We already build Git for Windows with `NO_GETTEXT` when compiling with
> > GCC. Let's do the same with Visual C, too.
> >
> > Note that we do not technically _need_ to pass `NO_GETTEXT` explicitly
> > in that `make artifacts-tar` invocation because we do this while `MSVC`
> > is set (which will set `uname_S := Windows`, which in turn will set
> > `NO_GETTEXT = YesPlease`). But it is definitely nicer to be explicit
> > here.
>
> Okey, so we never used it in the first place.
No, you misunderstood.
While it _is_ true that we set `NO_GETTEXT` implicitly (via `MSVC`) when
running `artifacts-tar`, that flag was ignored before this here patch
series.
> That makes the subject and first paragraph of the commit message seem
> really out of place. So we really mean something like this instead?:
>
> ci(vs-build): be explicit about NO_GETTEXT
>
> We already supply `NO_GETTEXT` implicitly due to config.mak.uname,
> but let's do so explicitly to ???
>
> But if we're being explicit we also have SKIP_DASHED_BUILT_INS=YesPlease
> since ef60e9f74b2 (ci: stop linking built-ins to the dashed versions,
> 2020-09-21) for CI, which has an even bigger effect on what's included
> in the tarball, so it seems odd to single out NO_GETTEXT=YesPlease,
> unless there's some other reason to do this that I'm missing.
Yes, you are missing the fact that the `SKIP_DASHED_BUILT_INS` flag is set
explicitly.
The `NO_GETTEXT` flag was _not_ set explicitly. It is set by the section
of `config.mak.uname` that is in effect if `uname_S` is set to `Windows`,
which it is if we set the `MSVC` flag, which we still set in `vs-build`,
for tradition, even if we no longer build with `make MSVC=OhYeah` but
using MSBuild.
I hope this removes any confusion.
> Hrm, isn't the real reason here that before 5/7 this would error out,
> because while NO_GETTEXT=Y was implicit and we picked it up from the
> config.mak.uname, we just had the $(MOFILES) in the archive-tar list
> unconditionally.
>
> So after 5/7 that's not the case, so we don't need this change anymore,
> but we're making this change anyway? Seems like the result of this being
> the first try at a fix, and then re-sequencing the two & keeping the
> now-redundant hotfix.
Excuse me?
This here patch sets `NO_GETTEXT` when building Git in the `vs-build` job,
and consequently sets `NO_GETTEXT` when bundling up the artifacts tar.
It does so to accelerate the build which is legitimate because we already
test the gettext stuff in the `windows-build`/`windows-test` jobs.
There is nothing "hotfix" about this.
Ciao,
Johannes
> > Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
> > Helped-by: Matthias Aßhauer <mha1993@live.de>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > .github/workflows/main.yml | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > index 0f7516c9ef3..c99628681ef 100644
> > --- a/.github/workflows/main.yml
> > +++ b/.github/workflows/main.yml
> > @@ -159,7 +159,7 @@ jobs:
> > shell: bash
> > run: |
> > cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
> > - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> > + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> > - name: MSBuild
> > run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> > - name: bundle artifact tar
> > @@ -169,7 +169,7 @@ jobs:
> > VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
> > run: |
> > mkdir -p artifacts &&
> > - eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)"
> > + eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)"
> > - name: zip up tracked files
> > run: git archive -o artifacts/tracked.tar.gz HEAD
> > - name: upload tracked files and build artifacts
>
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal
2021-07-04 22:55 ` [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
@ 2021-07-06 19:16 ` Junio C Hamano
0 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-07-06 19:16 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> This Action not only downloads and extracts git-sdk-64-minimal _outside_
> the worktree (making it no longer necessary to meddle with
> `.gitignore` or `.git/info/exclude`), it also adds the `bash.exe` to the
> `PATH` and sets the environment variable `MSYSTEM` (an implementation
> detail that Git's workflow should never have needed to know about).
>
> This allows us to convert all those funny PowerShell tasks that wanted
> to call git-sdk-64-minimal's `bash.exe`: they all are now regular `bash`
> scriptlets.
>
> This finally lets us get rid of the funny quoting and escaping where we
> had to pay attention not only to quote and escape the Bash scriptlets
> properly, but also to add a second level of escaping (with backslashes
> for double quotes and backticks for dollar signs) to stop PowerShell
> from doing unintended things.
>
> Further, this Action uses a fast caching strategy native to GitHub
> Actions that should accelerate the download across CI runs:
> git-sdk-64-minimal is usually updated once per 24h, and needs to be
> cached only once within that period. Caching it (unfortunately only on
> a per-branch basis) speeds up the download step, and makes it much more
> robust at the same time by virtue of accessing a cache location that is
> closer in the network topology.
So nice.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT
2021-07-04 22:55 ` [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
2021-07-05 6:45 ` Ævar Arnfjörð Bjarmason
@ 2021-07-06 19:19 ` Junio C Hamano
2021-07-14 8:47 ` Johannes Schindelin
1 sibling, 1 reply; 41+ messages in thread
From: Junio C Hamano @ 2021-07-06 19:19 UTC (permalink / raw)
To: Dennis Ameling via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, Johannes Schindelin,
Dennis Ameling
"Dennis Ameling via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Dennis Ameling <dennis@dennisameling.com>
>
> We already build Git for Windows with `NO_GETTEXT` when compiling with
> GCC. Let's do the same with Visual C, too.
>
> Note that we do not technically _need_ to pass `NO_GETTEXT` explicitly
> in that `make artifacts-tar` invocation because we do this while `MSVC`
> is set (which will set `uname_S := Windows`, which in turn will set
> `NO_GETTEXT = YesPlease`). But it is definitely nicer to be explicit
> here.
In other words, is this a no-op but makes the recipe more readable?
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2021-07-04 22:55 ` [PATCH v3 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
@ 2021-07-06 23:20 ` Junio C Hamano
7 siblings, 0 replies; 41+ messages in thread
From: Junio C Hamano @ 2021-07-06 23:20 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Ævar Arnfjörð Bjarmason, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> This patch series upgrades to newer versions of a couple GitHub Actions we
> use, and also streamlines the Windows jobs using the relatively new
> setup-git-for-windows-sdk Action
> [https://github.com/marketplace/actions/setup-git-for-windows-sdk] (Git for
> Windows is running with this Action for a while now, getting all the kinks
> out).
>
> This patch series should also address the problem where seen was pushed so
> rapidly that the windows-test jobs failed because they no longer checked out
> the identical revision as the windows-build job.
>
> Changes since v2:
>
> * Made the handwaving make [...] NO_GETTEXT comment in the commit message
> of the patch "artifacts-tar: respect NO_GETTEXT" more explicit, by
> setting NO_GETTEXT to a bogus value as required by make.
> * Added an explicit NO_GETTEXT=YesPlease to the make artifacts-tar
> invocation in the vs-build job, as well as an explanation in the
> corresponding commit message why this explicit mention is technically not
> required.
The above indeed helps the CI at GitHub. The last run with 'seen'
can be seen at https://github.com/git/git/actions/runs/1005977569
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-07-04 8:27 ` Ævar Arnfjörð Bjarmason
2021-07-04 8:52 ` Ævar Arnfjörð Bjarmason
@ 2021-07-13 12:19 ` Johannes Schindelin
2021-07-13 15:11 ` Philip Oakley
2021-07-14 7:54 ` Ævar Arnfjörð Bjarmason
1 sibling, 2 replies; 41+ messages in thread
From: Johannes Schindelin @ 2021-07-13 12:19 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Dennis Ameling via GitGitGadget, git, Dennis Ameling
[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]
Hi Ævar,
On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
>
> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > index 0f7516c9ef..3b40c677ab 100644
> > --- a/.github/workflows/main.yml
> > +++ b/.github/workflows/main.yml
> > @@ -159,7 +159,7 @@ jobs:
> > shell: bash
> > run: |
> > cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
> > - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> > + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> > - name: MSBuild
> > run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> > - name: bundle artifact tar
>
> To a first approximation isn't the point of CI to test a diversity of
> setups?
No, the point of CI is to find breakages.
And in this instance, a careful analysis will reveal that the artifacts
generated via `msgfmt` are not even used, let alone tested.
Even more careful analysis will reveal that Git for Windows does not ship
with translations. Therefore, it would be pointless to test the
translations here. Doubly so because Git for Windows is not even built
using MSVC, and this `vs-build` job is really about verifying that we can
build with that compiler, and _iff_ we used `msgfmt` here, it would not be
Visual Studio's (because it doesn't have any `msgfmt`) but the regular one
from Git for Windows' SDK. Triply so because the regular one from Git for
Windows' SDK _is actually used_ in the `windows-build` job, therefore we
_really_ do not need to repeat the same exercise with the same gettext
version and the same git/git revision in the same GitHub workflow run.
In short: there is no diversity to be gained here. (Actually, that is not
quite true: by using gentler language on a first-time contributor instead
of immediately going into full critic mode without any fanfare, we could
have invited more diversity onto this list. Maybe you could pay more
attention to that in the future?)
Back to the issue you pointed out: could this be described in the commit
message? Sure. But I think there is no much worth in it, the surrounding
code and parent commit makes it clear that the `.mo` files aren't used,
and that it would therefore be a waste of electrons to generate them in
each and every `vs-build` run.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-07-13 12:19 ` Johannes Schindelin
@ 2021-07-13 15:11 ` Philip Oakley
2021-07-14 8:51 ` Johannes Schindelin
2021-07-14 7:54 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 41+ messages in thread
From: Philip Oakley @ 2021-07-13 15:11 UTC (permalink / raw)
To: Johannes Schindelin, Ævar Arnfjörð Bjarmason
Cc: Dennis Ameling via GitGitGadget, git, Dennis Ameling
On 13/07/2021 13:19, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
>>
>>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>>> index 0f7516c9ef..3b40c677ab 100644
>>> --- a/.github/workflows/main.yml
>>> +++ b/.github/workflows/main.yml
>>> @@ -159,7 +159,7 @@ jobs:
>>> shell: bash
>>> run: |
>>> cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
>>> - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
>>> + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
>>> - name: MSBuild
>>> run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
>>> - name: bundle artifact tar
>> To a first approximation isn't the point of CI to test a diversity of
>> setups?
> No, the point of CI is to find breakages.
>
> And in this instance, a careful analysis will reveal that the artifacts
> generated via `msgfmt` are not even used, let alone tested.
>
> Even more careful analysis will reveal that Git for Windows does not ship
> with translations. Therefore, it would be pointless to test the
> translations here. Doubly so because Git for Windows is not even built
> using MSVC, and this `vs-build` job is really about verifying that we can
> build with that compiler, and _iff_ we used `msgfmt` here, it would not be
> Visual Studio's (because it doesn't have any `msgfmt`) but the regular one
> from Git for Windows' SDK. Triply so because the regular one from Git for
> Windows' SDK _is actually used_ in the `windows-build` job, therefore we
> _really_ do not need to repeat the same exercise with the same gettext
> version and the same git/git revision in the same GitHub workflow run.
>
> In short: there is no diversity to be gained here. (Actually, that is not
> quite true: by using gentler language on a first-time contributor instead
> of immediately going into full critic mode without any fanfare, we could
> have invited more diversity onto this list. Maybe you could pay more
> attention to that in the future?)
>
> Back to the issue you pointed out: could this be described in the commit
> message? Sure. But I think there is no much worth in it, the surrounding
> code and parent commit makes it clear that the `.mo` files aren't used,
> and that it would therefore be a waste of electrons to generate them in
> each and every `vs-build` run.
>
> Ciao,
> Johannes
Folks,
This problem (vcpkg/VS/CI build breakage for the tar artifact) has also
just come up for me on
https://github.com/git-for-windows/git/pull/3306#issuecomment-879093798
"[RFC] making Git for Windows start buildins in modern Visual Studio
#3306" which is a similar build situation.
Hopefully we can chose a suitable solution regarding what, if any, extra
checks this part of the CI build ought to do with the (unused in this
build) `.mo` files.
--
Philip
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-07-13 12:19 ` Johannes Schindelin
2021-07-13 15:11 ` Philip Oakley
@ 2021-07-14 7:54 ` Ævar Arnfjörð Bjarmason
1 sibling, 0 replies; 41+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-07-14 7:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Dennis Ameling via GitGitGadget, git, Dennis Ameling
On Tue, Jul 13 2021, Johannes Schindelin wrote:
> Hi Ævar,
>
> On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
>>
>> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> > index 0f7516c9ef..3b40c677ab 100644
>> > --- a/.github/workflows/main.yml
>> > +++ b/.github/workflows/main.yml
>> > @@ -159,7 +159,7 @@ jobs:
>> > shell: bash
>> > run: |
>> > cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
>> > - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
>> > + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
>> > - name: MSBuild
>> > run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
>> > - name: bundle artifact tar
>>
>> To a first approximation isn't the point of CI to test a diversity of
>> setups?
>
> No, the point of CI is to find breakages.
>
> And in this instance, a careful analysis will reveal that the artifacts
> generated via `msgfmt` are not even used, let alone tested.
>
> Even more careful analysis will reveal that Git for Windows does not ship
> with translations. Therefore, it would be pointless to test the
> translations here. Doubly so because Git for Windows is not even built
> using MSVC, and this `vs-build` job is really about verifying that we can
> build with that compiler, and _iff_ we used `msgfmt` here, it would not be
> Visual Studio's (because it doesn't have any `msgfmt`) but the regular one
> from Git for Windows' SDK. [...]
Sure, to clarify my comment to Dennis Ameling were to the effect of
"hrm, this commit message doesn't seem to justify this change", not that
the change wasn't justified.
I.e. it *read* a bit similar to "this CI job already uses clang, let's
change this job that uses gcc to use it too", and seemed to take it as a
given that making them consistent was an improvement.
Which prompted my review to the effect "hang on a minute, by default we
want things different in CI, surely?".
From your explanation here the reason seems closer to (but I'm not sure
I got all the nuances involved) that the lack of -DNO_GETTEXT=YesPlease
was always an omission here, and this change really changes nothing.
But then isn't this a change that belongs in config.mak.uname since we
never use this on this platform? Or do you mean by "Git for Windows" the
configuration you build & ship that release in, not "A Git for Windows"
in the sense of what we're building here (which can/does support
gettext, or not?).
(I see reading on that some of that, i.e. the config.mak.uname
suggestion is wrong)....
> [...] Triply so because the regular one from Git for
> Windows' SDK _is actually used_ in the `windows-build` job, therefore we
> _really_ do not need to repeat the same exercise with the same gettext
> version and the same git/git revision in the same GitHub workflow run.
..., okey, So in any case we do test this elsewhere to bring this around
the entirety of this thread I think could be addressed by just changing
We already build Git for Windows with `NO_GETTEXT` when compiling with
GCC. Let's do the same with Visual C, too.
To something like:
In the XYZ job for Windows we don't need to build with GETTEXT
because any coverage gained there is covered by the ABC job. The IJK
job on the same platform already builds with -DNO_GETTEXT=YesPlease,
let's do the same here.
> In short: there is no diversity to be gained here. (Actually, that is not
> quite true: by using gentler language on a first-time contributor instead
> of immediately going into full critic mode without any fanfare, we could
> have invited more diversity onto this list. Maybe you could pay more
> attention to that in the future?)
I'll let others judge for themselves but I do think I'm using gentle
language here, particularly since the quoted "To a first approximation
isn't the point[...]" of mine is immediately followed by (which you
elided) a:
"Perhaps this change makes sense, just commenting on the explanation
in the commit message."
I.e. I think it's quite clear that I'm commenting on my own confusion,
not asserting something about the change itself.
> Back to the issue you pointed out: could this be described in the commit
> message? Sure. But I think there is no much worth in it, the surrounding
> code and parent commit makes it clear that the `.mo` files aren't used,
> and that it would therefore be a waste of electrons to generate them in
> each and every `vs-build` run.
Yes. If we have NO_GETTEXT=Y we shouldn't make the *.mo files and bundle
them up in the archive. It's clear from me from this patch & the
following[1] that we unambiguously end up in a state of NO_GETTEXT=Y.
I was commenting on the "why" of that change in the context of assuming
that the change made sense, but that the commit message wasn't per-es
justifying it to someone unfamiliar with the area.
I think your citing of reasons for the change that aren't at all
mentioned in the commit message supports that.
1. https://lore.kernel.org/git/c31d2e7f44a8b27210dbde9bc6938ce16a9e0c17.1625347592.git.gitgitgadget@gmail.com/
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT
2021-07-06 19:19 ` Junio C Hamano
@ 2021-07-14 8:47 ` Johannes Schindelin
0 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin @ 2021-07-14 8:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dennis Ameling via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, Dennis Ameling
Hi Junio,
On Tue, 6 Jul 2021, Junio C Hamano wrote:
> "Dennis Ameling via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Dennis Ameling <dennis@dennisameling.com>
> >
> > We already build Git for Windows with `NO_GETTEXT` when compiling with
> > GCC. Let's do the same with Visual C, too.
> >
> > Note that we do not technically _need_ to pass `NO_GETTEXT` explicitly
> > in that `make artifacts-tar` invocation because we do this while `MSVC`
> > is set (which will set `uname_S := Windows`, which in turn will set
> > `NO_GETTEXT = YesPlease`). But it is definitely nicer to be explicit
> > here.
>
> In other words, is this a no-op but makes the recipe more readable?
Yes. And it also removes some puzzlement from the thorough reviewer
(Matthias stumbled over it and was wondering why this even works without
`NO_GETTEXT`).
Thanks,
Dscho
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 5/6] ci(vs-build): build with NO_GETTEXT
2021-07-13 15:11 ` Philip Oakley
@ 2021-07-14 8:51 ` Johannes Schindelin
0 siblings, 0 replies; 41+ messages in thread
From: Johannes Schindelin @ 2021-07-14 8:51 UTC (permalink / raw)
To: Philip Oakley
Cc: Ævar Arnfjörð Bjarmason,
Dennis Ameling via GitGitGadget, git, Dennis Ameling
[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]
Hi Philip,
On Tue, 13 Jul 2021, Philip Oakley wrote:
> On 13/07/2021 13:19, Johannes Schindelin wrote:
> >
> > On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
> >
> >> On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
> >>
> >>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> >>> index 0f7516c9ef..3b40c677ab 100644
> >>> --- a/.github/workflows/main.yml
> >>> +++ b/.github/workflows/main.yml
> >>> @@ -159,7 +159,7 @@ jobs:
> >>> shell: bash
> >>> run: |
> >>> cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
> >>> - -DMSGFMT_EXE=C:/git-sdk-64-minimal/mingw64/bin/msgfmt.exe -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> >>> + -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> >>> - name: MSBuild
> >>> run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> >>> - name: bundle artifact tar
> >> To a first approximation isn't the point of CI to test a diversity of
> >> setups?
> > No, the point of CI is to find breakages.
>
> This problem (vcpkg/VS/CI build breakage for the tar artifact) has also
> just come up for me on
> https://github.com/git-for-windows/git/pull/3306#issuecomment-879093798
> "[RFC] making Git for Windows start buildins in modern Visual Studio
> #3306" which is a similar build situation.
>
> Hopefully we can chose a suitable solution regarding what, if any, extra
> checks this part of the CI build ought to do with the (unused in this
> build) `.mo` files.
You will be happy to hear that the fix was in v2 of this patch series (you
responded to v1).
Ciao,
Dscho
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2021-07-14 8:51 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23 15:24 [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 1/6] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 2/6] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 3/6] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 4/6] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
2021-06-23 15:24 ` [PATCH 5/6] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
2021-07-04 8:27 ` Ævar Arnfjörð Bjarmason
2021-07-04 8:52 ` Ævar Arnfjörð Bjarmason
2021-07-13 12:19 ` Johannes Schindelin
2021-07-13 15:11 ` Philip Oakley
2021-07-14 8:51 ` Johannes Schindelin
2021-07-14 7:54 ` Ævar Arnfjörð Bjarmason
2021-06-23 15:24 ` [PATCH 6/6] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
2021-07-01 22:03 ` [PATCH 0/6] ci: speed-up the Windows parts of our GitHub workflow Junio C Hamano
2021-07-03 21:26 ` [PATCH v2 0/7] " Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 4/7] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
2021-07-04 8:30 ` Ævar Arnfjörð Bjarmason
2021-07-04 22:52 ` Johannes Schindelin
2021-07-05 6:33 ` Ævar Arnfjörð Bjarmason
2021-07-03 21:26 ` [PATCH v2 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
2021-07-03 21:26 ` [PATCH v2 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
2021-07-04 8:54 ` Ævar Arnfjörð Bjarmason
2021-07-04 22:37 ` Johannes Schindelin
2021-07-04 22:55 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 1/7] ci: use the new GitHub Action to download git-sdk-64-minimal Johannes Schindelin via GitGitGadget
2021-07-06 19:16 ` Junio C Hamano
2021-07-04 22:55 ` [PATCH v3 2/7] ci (vs-build): use `cmd` to copy the DLLs, not `powershell` Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 3/7] ci: upgrade to using actions/{up,down}load-artifacts v2 Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 4/7] ci(windows): transfer also the Git-tracked files to the test jobs Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 5/7] artifacts-tar: respect NO_GETTEXT Johannes Schindelin via GitGitGadget
2021-07-04 22:55 ` [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT Dennis Ameling via GitGitGadget
2021-07-05 6:45 ` Ævar Arnfjörð Bjarmason
2021-07-05 12:44 ` Johannes Schindelin
2021-07-06 19:19 ` Junio C Hamano
2021-07-14 8:47 ` Johannes Schindelin
2021-07-04 22:55 ` [PATCH v3 7/7] ci: accelerate the checkout Johannes Schindelin via GitGitGadget
2021-07-06 23:20 ` [PATCH v3 0/7] ci: speed-up the Windows parts of our GitHub workflow Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).