ci(vs-build): stop passing the iconv library location explicitly

PATCH In review Johannes Schindelin via GitGitGadget 3 participants 3 messages Dec 4, 2020

lore

Share & tools

lore

Johannes Schindelin via GitGitGadget

author

2020-12-04 14:24 UTC #0 lore raw

PATCH 1/1 ci(vs-build): stop passing the iconv library location explicitly

From: Dennis Ameling <[email protected]>

Something changed in vcpkg (which we use in our Visual C++ build to provide the dependencies such as libcurl) and our vs-build job started failing in CI. The reason is that we had a work-around in place to help CMake find iconv, and this work-around is neither needed nor does it work anymore.

For the full discussion with the vcpkg project, see this comment:
https://github.com/microsoft/vcpkg/issues/14780#issuecomment-735368280

Signed-off-by Dennis Ameling <[email protected]>

Signed-off-by Johannes Schindelin <[email protected]>

---
ci: fix the vs-build job so that CI builds pass again

This was noticed first in a Git for Windows PR.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-805%2Fdscho%2Fvs-build-and-libiconv-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-805/dscho/vs-build-and-libiconv-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/805

 .github/workflows/main.yml      | 1 -
 compat/vcbuild/scripts/clink.pl | 2 +-
 contrib/buildsystems/engine.pl  | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

.github/workflows/main.yml +0 -1

index a940997f1b..f1f0d1fe35 100644
@@ -201,7 +201,6 @@ jobs:
       shell: bash
       run: |
         cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
-        -DIconv_LIBRARY=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows/lib/libiconv.lib -DIconv_INCLUDE_DIR=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows/include \
         -DMSGFMT_EXE=`pwd`/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

compat/vcbuild/scripts/clink.pl +1 -1

index df167d1e1a..3bd824154b 100755
@@ -45,7 +45,7 @@
 		push(@args, "zlib.lib");
 	    }
 	} elsif ("$arg" eq "-liconv") {
-		push(@args, "libiconv.lib");
+		push(@args, "iconv.lib");
 	} elsif ("$arg" eq "-lcrypto") {
 		push(@args, "libcrypto.lib");
 	} elsif ("$arg" eq "-lssl") {

contrib/buildsystems/engine.pl +1 -1

index 2ff9620459..ed6c45988a 100755
@@ -351,7 +351,7 @@ sub handleLinkLine
         } elsif ("$part" eq "-lexpat") {
             push(@libs, "libexpat.lib");
         } elsif ("$part" eq "-liconv") {
-            push(@libs, "libiconv.lib");
+            push(@libs, "iconv.lib");
         } elsif ($part =~ /^[-\/]/) {
             push(@lflags, $part);
         } elsif ($part =~ /\.(a|lib)$/) {

base-commit: 898f80736c75878acc02dc55672317fcc0e0a5a6

Signature · 1 line

gitgitgadget

Junio C Hamano

2020-12-04 20:03 UTC #1 lore raw

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 11 lines · writes:

From: Dennis Ameling <[email protected]>
Something changed in vcpkg (which we use in our Visual C++ build to provide the dependencies such as libcurl) and our vs-build job started failing in CI. The reason is that we had a work-around in place to help CMake find iconv, and this work-around is neither needed nor does it work anymore.
For the full discussion with the vcpkg project, see this comment:
https://github.com/microsoft/vcpkg/issues/14780#issuecomment-735368280

Quoted 6 lines

Signed-off-by Dennis Ameling <[email protected]>
Signed-off-by Johannes Schindelin <[email protected]>
---
ci: fix the vs-build job so that CI builds pass again
This was noticed first in a Git for Windows PR.

This is probably the same breakage I've been seeing, e.g.
https://github.com/git/git/runs/1494253517

Thanks for taking care of it.

I guess we should just apply directly to 'master' (or 'maint' and merge up), but I can queue it just like all other topics and have it traverse through 'seen'->'next'->'master'.

Johannes Schindelin

2020-12-04 22:15 UTC #2 lore raw

Hi Junio,

Quoted 19 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 16 lines · writes:
From: Dennis Ameling <[email protected]>
Something changed in vcpkg (which we use in our Visual C++ build to provide the dependencies such as libcurl) and our vs-build job started failing in CI. The reason is that we had a work-around in place to help CMake find iconv, and this work-around is neither needed nor does it work anymore.
For the full discussion with the vcpkg project, see this comment:
https://github.com/microsoft/vcpkg/issues/14780#issuecomment-735368280
Signed-off-by Dennis Ameling <[email protected]>
Signed-off-by Johannes Schindelin <[email protected]>
---
ci: fix the vs-build job so that CI builds pass again

Quoted 4 lines

Quoted 1 line
This was noticed first in a Git for Windows PR.
This is probably the same breakage I've been seeing, e.g.
https://github.com/git/git/runs/1494253517

Yes.

Quoted 3 lines

I guess we should just apply directly to 'master' (or 'maint' and merge up), but I can queue it just like all other topics and have it traverse through 'seen'->'next'->'master'.

Whatever is easier for you.

Thanks,
Dscho