Re: [PATCH v3 6/7] ci(vs-build): build with NO_GETTEXT

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 5, 2021, 06:45 UTC
Message-ID: <87o8bh6yhf.fsf@evledraar.gmail.com>
In-Reply-To: <2c4cd9dd1c8d966c8df0349bb820449ae1290793.1625439315.git.gitgitgadget@gmail.com>

On Sun, Jul 04 2021, Dennis Ameling via GitGitGadget wrote:

> From: Dennis Ameling <dennis@dennisameling.com>

Re the v3 cover letter & my v2 comment:

Show 8 quoted lines

> 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.

Show 29 quoted lines

> 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