From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-06-23 15:24:22
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
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-06-23 15:24:23
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-06-23 15:24:27
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 103 +++++++------------------------------
1 file changed, 19 insertions(+), 84 deletions(-)
From: Dennis Ameling via GitGitGadget <hidden> Date: 2021-06-23 15:24:28
From: Dennis Ameling <redacted>
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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-06-23 15:24:29
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-06-23 15:24:30
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-06-23 15:24:31
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:38
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
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:39
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:48
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:48
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 103 +++++++------------------------------
1 file changed, 19 insertions(+), 84 deletions(-)
From: Dennis Ameling via GitGitGadget <hidden> Date: 2021-07-03 21:26:48
From: Dennis Ameling <redacted>
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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:48
From: Johannes Schindelin <redacted>
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 [off-list ref]
Signed-off-by: Johannes Schindelin <redacted>
---
Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:48
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-03 21:26:48
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
quoted hunk
From: Dennis Ameling <redacted>
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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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?
...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 :)
On Sun, Jul 04 2021, Ævar Arnfjörð Bjarmason wrote:
On Wed, Jun 23 2021, Dennis Ameling via GitGitGadget wrote:
quoted
From: Dennis Ameling <redacted>
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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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?
On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <redacted>
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 :)
From: Johannes Schindelin <hidden> Date: 2021-07-04 22:37:45
Hi Ævar,
On Sun, 4 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
On Sat, Jul 03 2021, Johannes Schindelin via GitGitGadget wrote:
quoted
From: Johannes Schindelin <redacted>
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 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 :)
...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".)
quoted hunk
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 :)
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)"
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:18
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 [off-list ref]
+ Helped-by: Matthias Aßhauer [off-list ref]
Signed-off-by: Johannes Schindelin [off-list ref]
## .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
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:20
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 103 +++++++------------------------------
1 file changed, 19 insertions(+), 84 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:23
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:23
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:23
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:29
From: Johannes Schindelin <redacted>
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 <redacted>
---
.github/workflows/main.yml | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: Johannes Schindelin via GitGitGadget <hidden> Date: 2021-07-04 22:55:29
From: Johannes Schindelin <redacted>
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 [off-list ref]
Signed-off-by: Johannes Schindelin <redacted>
---
Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
From: Dennis Ameling via GitGitGadget <hidden> Date: 2021-07-04 22:55:29
From: Dennis Ameling <redacted>
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 <redacted>
Helped-by: Matthias Aßhauer [off-list ref]
Signed-off-by: Johannes Schindelin <redacted>
---
.github/workflows/main.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
...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>
...
quoted
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 :)
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
quoted
$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
test -n "$(ARTIFACTS_DIRECTORY)"
On Sun, Jul 04 2021, Dennis Ameling via GitGitGadget wrote:
From: Dennis Ameling <redacted>
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.
From: Johannes Schindelin <hidden> Date: 2021-07-05 12:44:23
Hi Ævar,
On Mon, 5 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
On Sun, Jul 04 2021, Dennis Ameling via GitGitGadget wrote:
quoted
From: Dennis Ameling <redacted>
Re the v3 cover letter & my v2 comment:
quoted
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
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
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
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/
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