* [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* 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 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* 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
* [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 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
* [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* 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
* [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* 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 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 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
* [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 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