Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libevent] add features #8349

Merged

Conversation

NancyLi1013
Copy link
Contributor

@NancyLi1013 NancyLi1013 commented Sep 26, 2019

Add openssl and thread features for libevent.
libevhtp needs to depend on this two features.
Related issue #4793.

@NancyLi1013 NancyLi1013 added the info:internal This PR or Issue was filed by the vcpkg team. label Sep 26, 2019
@NancyLi1013
Copy link
Contributor Author

/azp run

@vicroms
Copy link
Member

vicroms commented Oct 7, 2019

/azp run

@NancyLi1013
Copy link
Contributor Author

The failure on Linux is due to:

/ci/myagent/_work/1/s/installed/x64-linux/debug/lib/libevent.a /ci/myagent/_work/1/s/installed/x64-linux/debug/lib/libevent_core.a /ci/myagent/_work/1/s/installed/x64-linux/debug/lib/libevent_extra.a -levent_openssl_static /ci/myagent/_work/1/s/installed/x64-linux/debug/lib/libglog.a -lpthread /ci/myagent/_work/1/s/installed/x64-linux/debug/lib/libgflags_debug.a -lpthread && :
/usr/bin/ld: cannot find -levent_openssl_static
collect2: error: ld returned 1 exit status

It seems that there is no event_openssl installed here.
I try to checkout this branch to my local and do as the following steps:

  1. vcpkg install libevent[openssl] .
  2. vcpkg install evpp

Then the failure disappeared.
So I guess we need to install openssl feature manually to test this issue.

@NancyLi1013 NancyLi1013 marked this pull request as ready for review October 16, 2019 07:28
@NancyLi1013 NancyLi1013 requested a review from JackBoosY October 21, 2019 03:01
@NancyLi1013
Copy link
Contributor Author

NancyLi1013 commented Nov 7, 2019

All features test pass with below triplets:

  • x86-windows

  • x64-windows-static

  • arm64-windows

  • x64-linux

Note: This port currently doesn't support UWP.

@ras0219-msft ras0219-msft merged commit 058f6e2 into microsoft:master Nov 19, 2019
@ras0219-msft
Copy link
Contributor

LGTM!

@sipsorcery
Copy link
Contributor

The thread EVENT__DISABLE_THREAD_SUPPORT change in this PR has broken the Bitcoin Core msvc build.

I can work around it by using a custom port but was wondering if that change was required for a particular feature? It's not clear from the PR description.

@vicroms
Copy link
Member

vicroms commented Nov 25, 2019

Hi @sipsorcery

We usually disable optional library features by default and make them accessible through package features.

In this case, you might not need a custom port and can just simply re-enable EVENT__DISABLE_THREAD_SUPPORT by doing:

./vcpkg install libevent[threads]

@vicroms
Copy link
Member

vicroms commented Nov 25, 2019

@NancyLi1013
@ras0219-msft

Shouldn't threads be a default feature of this port?

@sipsorcery
Copy link
Contributor

@vicroms thx for the response. Somehow I missed "Feature Packages" being added to vcpkg. Once it got pointed out to me we were able to update the vcpkg list to use libevent[threads] and our build is working again.

+1 for threads being the default.

@NancyLi1013 NancyLi1013 deleted the dev/NancyLi/add_feature_for_libevent branch November 26, 2019 02:17
@NancyLi1013
Copy link
Contributor Author

Hi @sipsorcery thanks for reporting this issue.
@vicroms thanks for your detail explanation to this issue.

I will add thread as the default feature of this port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants