Page MenuHomePhabricator

[CMAKE] Fix the bench build for windows
ClosedPublic

Authored by Fabien on Fri, Mar 20, 17:00.

Details

Summary

Event requires lib ws2_32 on windows (as well as shell32 and advapi32).
The missing ws2_32 caused the bench to fail the build (shell32 and advapi32 are already added by other dependencies).

Test Plan

Run the Gitian builds.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Fri, Mar 20, 17:00
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Mar 20, 17:00
deadalnix requested changes to this revision.Fri, Mar 20, 17:06
deadalnix added a subscriber: deadalnix.

Event requires lib ws2_32 on windows

Well then, why not do that?

This revision now requires changes to proceed.Fri, Mar 20, 17:06
Fabien requested review of this revision.Fri, Mar 20, 17:21

Because it's not enough, unfortunately. Not only event needs it, but also util for system.cpp:SetupNetworking().

I started to work on a better solution, but a complete fix requires to backport all the wallet/server separation first, which we should do but it's huge.

This fix is simpl and more in line with autotools (which is unaffected). If it is a no-go then I will revert the faulty commit.

deadalnix requested changes to this revision.EditedFri, Mar 20, 17:36

If libevent needs ws2_32 on some plateform, then it needs it on some plateforms. This needs to be part of the fix. I don't see how any backport changes this.

This revision now requires changes to proceed.Fri, Mar 20, 17:36
Fabien planned changes to this revision.Fri, Mar 20, 17:45

For some reason I was focusing on removing the dependency to util at the same time, and that needs to pull more code. Just add adding the event dependency will work for sure.

Fabien edited the summary of this revision. (Show Details)Fri, Mar 20, 17:48
Fabien updated this revision to Diff 17074.Fri, Mar 20, 17:51

Link windows lib as a dependency for event.

Fabien added inline comments.Fri, Mar 20, 17:52
cmake/modules/FindEvent.cmake
53 ↗(On Diff #17074)

Note to reviewers: no need to use a find_library here since they are part of the windows SDK.

deadalnix accepted this revision.Fri, Mar 20, 19:17
This revision is now accepted and ready to land.Fri, Mar 20, 19:17
This revision was automatically updated to reflect the committed changes.