ci: Add workaround for vcpkg's `libevent` package

hebasto commented at 2:05 pm on April 1, 2025: member

This PR is necessary for Windows GHA images, which provide CMake >= 4.0.

The idea has been taken from https://github.com/microsoft/vcpkg/pull/44273#discussion_r2009140242.

ci: Add workaround for vcpkg's libevent package

This change is necessary for Windows GHA images, which provide
CMake >= 4.0.

ef00a28414

hebasto added the label Windows on Apr 1, 2025

hebasto added the label Tests on Apr 1, 2025

DrahtBot commented at 2:05 pm on April 1, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32184.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, pinheadmz
Concept ACK	laanwj

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32182 (ci: Switch to dynamic library linkage in native Windows job by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto commented at 2:05 pm on April 1, 2025: member

cc @laanwj @pinheadmz

hebasto renamed this:
~~ci: Add workaround for vcpkg's libevent package~~
ci: Add workaround for vcpkg's `libevent` package
on Apr 1, 2025

laanwj commented at 2:11 pm on April 1, 2025: member

Concept ACK.

Do we need to mention this in build-windows.md as well, for people manually building using MSVC?

Or is this so temporary and likely to be worked around upstream.

ci: Drop confusing comment

GHA updates images anyway.

30c59adda4

hebasto commented at 2:22 pm on April 1, 2025: member

Do we need to mention this in build-windows.md as well, for people manually building using MSVC?

Or is this so temporary and likely to be worked around upstream.

It seems to be close to being fixed upstream.

pinheadmz commented at 5:06 pm on April 1, 2025: member

https://github.com/libevent/libevent/issues/1782 says:

CMake >= 4.0.0-rc1 removed compatibility with versions < 3.5 and errors out with such versions passed to cmake_minimum_required().

Just so I understand, this PR hacks the min required value, but doesn’t handle whatever incompatibility there actually is between cmake 4 and 3.5?

Concept ACK, CI is passing so seems to do the trick. I can test on a windows 2022 server VM later today.

maflcko commented at 5:23 pm on April 1, 2025: member

Shouldn’t upstream just merge the fix in https://github.com/microsoft/vcpkg/pull/44356 ?

laanwj commented at 5:27 pm on April 1, 2025: member

Just so I understand, this PR hacks the min required value, but doesn’t handle whatever incompatibility there actually is between cmake 4 and 3.5?

Yes, that’s what it does. It doesn’t seem that there is an actual incompatibility. My understanding is that as some versioning mechanism for CMake 4.0+ they made it that compatibility needs to be explicitly checked and enabled for each project.

hebasto commented at 7:10 pm on April 1, 2025: member

… CI is passing so seems to do the trick.

Please note that both CI jobs–Windows native and Windows native, fuzz–used the windows-2022 image Version: 20250324.3.0, which provides cmake version 3.31.6. This image works fine without this PR.

This CI job uses the updated image Version: 20250330.1.0, which provides cmake version 4.0.0, and does prove the usefulness of this PR.

hebasto commented at 7:18 pm on April 1, 2025: member

Shouldn’t upstream just merge the fix in microsoft/vcpkg#44356 ?

I don’t think so.

We have pinned the libevent package to the latest release for a reason:https://github.com/bitcoin/bitcoin/blob/1a6fc04d815e45ee895bd7ff0bc1b03685ed96c3/vcpkg.json#L51-L57

However, merging upstream https://github.com/microsoft/vcpkg/pull/44273 would help.

maflcko commented at 7:28 pm on April 1, 2025: member

Thanks for explaining!

I guess this means the temporary workaround here is needed.

lgtm ACK 30c59adda44f86b5e78249f3fb0d2cff88dad285

DrahtBot requested review from laanwj on Apr 1, 2025

DrahtBot requested review from pinheadmz on Apr 1, 2025

hebasto commented at 7:32 pm on April 1, 2025: member

Just so I understand, this PR hacks the min required value, but doesn’t handle whatever incompatibility there actually is between cmake 4 and 3.5?

Yes, that’s what it does. It doesn’t seem that there is an actual incompatibility. My understanding is that as some versioning mechanism for CMake 4.0+ they made it that compatibility needs to be explicitly checked and enabled for each project.

The CMake’s variable CMAKE_POLICY_VERSION_MINIMUM specifies:

a minimum Policy Version for a project without modifying its calls to cmake_minimum_required(VERSION)…

It is safe to bump the policy version to 3.5, as our own patch in depends does the same:https://github.com/bitcoin/bitcoin/blob/1a6fc04d815e45ee895bd7ff0bc1b03685ed96c3/depends/patches/libevent/cmake_fixups.patch#L1-L13

pinheadmz approved

pinheadmz commented at 8:03 pm on April 1, 2025: member

ACK 30c59adda44f86b5e78249f3fb0d2cff88dad285

Tested locally. Was able to reproduce the issue:

0PS C:\Users\matthew\source\repos\bitcoin> cmake --version
1cmake version 4.0.0

error: building libevent:x64-windows failed with: BUILD_FAILED

log file:

0  Compatibility with CMake < 3.5 has been removed from CMake.
1
2  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
3  to tell CMake that the project requires at least <min> but has been updated
4  to work with policies introduced by <max> or earlier.
5
6  Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.

Added this line set(ENV{CMAKE_POLICY_VERSION_MINIMUM} 3.5)

to cmake.ports for a succesfull build

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA256
 2
 3ACK 30c59adda44f86b5e78249f3fb0d2cff88dad285
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmfsRdQACgkQ5+KYS2KJ
 7yTosBBAAkcc1JaVdn/HJHcqYHftS2nFs+eAYHGn05D6m10zqLfmfMezoYGBFwhVv
 8xblnUJirmEUUFECCHayM6fgioO9bFLruttde6F2dUX2J8NDqSTIXcDYOlPdGXDUf
 9ygExfK7OJUUxcKZ0SSyygPdt9/TqaHj0kXE3QpvuPn0T+SQZ6qTyXDlqckthaeie
10QunrT0viZI1FJ3wQSwrZ+aprVNfBAzCYsduWKusLwIv3B2D+sHN2IF2ofemEeWtm
11kD8SsCJUiEst8a2/y2P4S5a+xTLRjQ5x84G7BG6UfnuKZRNNgK2jufmJWvIJUXMa
12i50ILwcPxbOOa5wKJoQoX8kK2CO8NDrCtOUWWn87Ae1J9uTDDUNhQa8lCulD6tuz
13VUGx61VdQWNeoKWwC2LyKdCGIGG2a3xHF41C5ZYDCXaqkQF8C7tPbO3k8aAl/OJ9
14r/bL7dAll51NScz80w+PW30/KymbveoD8j1S/fi3Q2DTrKHtyGR+4xAr7v12zCbb
15rPN+vmJFglMsljQXN8S8a56Xqgt+EzUXH4Zj9yKOF5a+azWBNqafPYVJ1r46B3vD
16HQ98SN4cInT7yJJjmRrnDRRJ92v0JSw47lvLqFSwmzp6A0H64+y7OGEbyGP7OHa+
17iTxYXyiqHmRQ2B6S3TexL5wuITQJgHZqXz+7+r6rPavcqJR2B9c=
18=A7t5
19-----END PGP SIGNATURE-----

pinheadmz’s public key is on openpgp.org

hebasto merged this on Apr 1, 2025

hebasto closed this on Apr 1, 2025

hebasto deleted the branch on Apr 1, 2025

hebasto added the label Needs backport (29.x) on Apr 1, 2025

hebasto commented at 9:59 am on April 10, 2025: member

This PR is necessary for Windows GHA images, which provide CMake >= 4.0.

They downgraded CMake to 3.31.6 in the subsequent image update. Nevertheless, it’s good for us to be prepared.

ci: Add workaround for vcpkg’s libevent package #32184

Code Coverage & Benchmarks

Reviews

Conflicts

ci: Add workaround for vcpkg’s `libevent` package #32184