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

[dscho]

This was noticed first in a Git for Windows PR.

[dscho]

/submit

[gitgitgadget]

Submitted as pull.805.git.1607091741254.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-805/dscho/vs-build-and-libiconv-v1

To fetch this version to local tag pr-805/dscho/vs-build-and-libiconv-v1:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-805/dscho/vs-build-and-libiconv-v1

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Dennis Ameling <dennis@dennisameling.com>
>
> 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 <dennis@dennisameling.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>     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'.

[gitgitgadget]

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Junio,

On Fri, 4 Dec 2020, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Dennis Ameling <dennis@dennisameling.com>
> >
> > 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 <dennis@dennisameling.com>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >     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

Yes.

> 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

[gitgitgadget]

This branch is now known as da/vs-build-iconv-fix.

[gitgitgadget]

This patch series was integrated into seen via git@ca0fe8c.

[SibiSiddharthan]

@dscho Looks like GitHub has upgraded to CMake v3.19.1. In this version I have upstreamed few changes to CMake that should change the behavior of how libraries are searched(i.e from all directories per name(old way) to all names per directories). So, we don't need to hard code the library names anymore. Even if the runners install new packages there will be no conflict for the libraries we are using.

[dscho]

@SibiSiddharthan you mean you got changes accepted into CMake? Congratulations! What do we need to do on Git's side to accommodate?

[SibiSiddharthan]

Yes, my changes have been accepted into CMake, and released in version 3.19.
This particular change(finding libraries) does not require a version bump. (yay!)
You do not need to hard code the libraries we need currently anymore in CI script.

[dscho]

You do not need to hard code the libraries we need currently anymore in CI script.

I don't understand. Could you elaborate, please?

[SibiSiddharthan]

I don't understand. Could you elaborate, please?

Before this change, we were using Iconv_INCLUDE_DIR, Iconv_LIBRARY to point to our iconv. We don't need to do this anymore as CMAKE_PREFIX_PATH will suffice.
In the future if git uses say libfoo, and libfoo is not there in the default find_package of CMake, we might need to explicitly specify libfoo
For eg. If we want to support pcre(currently CMake's find_package does not have this by default) we might need to specify it's location explicitly, or write a find_package script ourselves and place it within this repo.

[gitgitgadget]

This patch series was integrated into seen via git@ac56e70.

[gitgitgadget]

This patch series was integrated into next via git@4c6c505.

[gitgitgadget]

This patch series was integrated into seen via git@5f8e890.

[gitgitgadget]

This patch series was integrated into seen via git@ccbde2c.

[gitgitgadget]

This patch series was integrated into next via git@ccbde2c.

[gitgitgadget]

This patch series was integrated into master via git@ccbde2c.

[gitgitgadget]

Closed via ccbde2c.